<Swing Dev> Review request for 7093156: NLS: Please change the mnemonic assignment system to avoid translation issue (Swing files)
Alexander Scherbatiy
alexandr.scherbatiy at oracle.com
Wed Apr 4 10:12:55 UTC 2012
webrev: http://cr.openjdk.java.net/~alexsch/7093156/webrev.03/
test: http://cr.openjdk.java.net/~alexsch/7093156/test/02/
Now all properties are generated with the following suffixes:
xxx.textAndMnemonic and xxx.titleAndMnemonic
- patterns which are generated to properties with the
xxx.textAndMnemonic suffix:
(xxxNameText, xxxNameMnemonic)
(xxxNameText, xxxMnemonic)
(xxx.nameText, xxx.mnemonic)
(xxxText, xxxMnemonic)
- pattern which is generated to properties with the xxx.titleAndMnemonic
suffix:
(xxxTitle, xxxMnemonic)
After that the extended hashmap check a key suffix and generate a
compositeKey which allows to get a property in a new format and extract
a text, mnemonic or mnemonic index from it.
The test gets a path to the old properties and L&F class. If L&F is
not null than it is set. After that the test gets all properties and
it's values and check that values are equal to the values which are got
from the UIManager.get(key, locale).
There are some exceptions which are included into the exclude list in
the end of the test.
For example a mnemonic can be set to zero or to a character instead of
using the character integer key value.
Or there can be defined just mnemonics without a text.
Thanks,
Alexandr.
On 3/30/2012 12:56 PM, Alexander Scherbatiy wrote:
>
> webrev: http://cr.openjdk.java.net/~alexsch/7093156/webrev.01/
>
> There is the updated webrev which is used the TextAndMnemonicPattern
> class and separate getTextProperty and getMnemonicProperty methods.
> The getMnemonicFromProperty returns null in case if a mnemonic has not
> been found.
>
> See other comments inline...
>
> On 3/29/2012 5:28 PM, Pavel Porvatov wrote:
>> I'll try explain what I meant below...
>>
>> Lets take the first loop:
>> + for (int i = 0; i < TEXT_SUFFIX.length; i++) {
>> + String suffix = TEXT_SUFFIX[i];
>> + if (stringKey.endsWith(suffix)) {
>> + value = super.get(stringKey + AND_MNEMONIC);
>> + if (value != null) {
>> + return getText(value.toString());
>> + }
>> + }
>> + }
>> and lets stringKey = BlaBlaBlaNameText. Then you'll invoke
>> super.get() twice and the loop will have 4 iterations. So if you will
>> use for the first loop only "Text", "Title" you will have only 2 loop
>> iterations and no unnecessary super.get() invocations.
>
> I have added the TextAndMnemonicPattern class and updated the code
> to use the following patterns:
> ( texts: { "NameText", "Text", "Title"} , mnemonic: "Mnemonic")
> ( texts: { "nameText" } , mnemonic: "mnemonic")
>
> Now the "for each" loop is used and for each mnemonic there is only
> one iteration of the text.
>
>>
>> About "They are (xxxNameText, xxxMnemonic), (xxx.nameText,
>> xxx.mnemonic),(xxxText, xxxMnemonic) and (xxxTitle, xxxMnemonic)":
>> can we don't use "Name" for pair (xxxNameText, xxxMnemonic), like
>> xxxTextAndMnemonic? For consistency we could use for pair
>> (xxx.nameText, xxx.mnemonic) xxx.textAndMnemonic...
>
> 1) There are real collisions if we will use the same suffix like
> xxx.textAndMnemonic for all properties. For example see the
> gtk.properties:
> FileChooser.renameFileErrorTitle=Error
> FileChooser.renameFileErrorText=Error renaming file "{0}" to "{1}"
>
> Changing it to the same suffix will lead to:
> FileChooser.renameFileError.textAndMnemonic=Error
> FileChooser.renameFileError.textAndMnemonic=Error renaming file
> "{0}" to "{1}"
>
> So one value will be lost.
>
> 2) We really need to check all these text suffixes.
>
> There are 3 examples from the swing property files:
> ColorChooser.hsvNameText=HSV
> ColorChooser.hsvMnemonic=72
> FileChooser.helpButtonText=Help
> FileChooser.helpButtonMnemonic=72
> GTKColorChooserPanel.nameText=GTK Color Chooser
> GTKColorChooserPanel.mnemonic=71
>
> which are converted to the new format:
> FileChooser.helpButtonTextAndMnemonic=&Help
> ColorChooser.hsvNameTextAndMnemonic=&HSV
> GTKColorChooserPanel.nameTextAndMnemonic=>K Color Chooser
>
> Now on the left side there are requests and on the right side are
> our actions:
> ColorChooser.hsvNameText | check that it is request to the
> text (has NameText suffix) and convert to new format
> ColorChooser.hsvNameTextAndMnemonic (add AndMnemonic suffix)
> GTKColorChooserPanel.nameText | check that it is request to
> the text (has nameText) and convert to new format
> GTKColorChooserPanel.nameTextAndMnemonicadd AndMnemonic suffix)
> FileChooser.helpButtonMnemonic | check that it is a request to
> the mnemonic (has Mnemonic suffix) and convert to new format (remove
> Mnemonic suffix and add TextAndMnemonic suffix)
> GTKColorChooserPanel.mnemonic | check that it is a request to
> the mnemonic (has mnemonic suffix) and convert to new format (remove
> mnemonic suffix and add nameTextAndMnemonic suffix)
>
> The suffixes for the text can be Text and Title as well.
> So the "NameText", "Text", "Title" and "nameText" suffixes are
> already used in swing components and their unification requires to
> updating a lot of java code.
>>
>>>
>>>>
>>>> 3. Could you pleas use "for each" loops (if possible), they are
>>>> more compact and readable
>>>
>>> In the second loop the i index is used to add a TEXT_SUFFIX
>>> after cutting a MNEMONIC_SUFFIX.
>>> So the first loop can use the "for each" loop.
>> Yep! That's what I meant...
>>>
>>> May be it is better to have a TextAndMnemonic class that holds
>>> values for the corresponded text and mnemonic suffixes?
>>>
>>>>
>>>> 4. Integer.toString((int) Character.toUpperCase(c)) looks a little
>>>> bit tricky. Can we use something like this:
>>>> String.valueOf(Character.toUpperCase(c)) ?
>>> For example there is the property:
>>> ColorChooser.rgbRedTextAndMnemonic=Re&d
>>> The code that needs a mnemonic expect that it gets the integer
>>> value |68| in the string instead of char D.
>>> So the right mnemonic value should be
>>> ColorChooser.rgbRedMnemonic=68
>>>
>> Ok. BTW: I think "return null" will be more correct in the
>> getMnemonic method
>
> Updated the method to return null if a mnemonic has not been found.
>
> Thanks,
> Alexandr.
>
>>>>
>>>> 5. What is the reason to have the second loop?
>>> The first loop checks is it a request to a text value. The
>>> second loop checks is it a request to a mnemonic value.
>>>
>>>> Can we just check
>>>> stringKey.endsWith("mnemonic") || stringKey.endsWith("Mnemonic")?
>>> There might be collisions. It is better to explicitly check
>>> that a text suffix from a pattern have the certain suffix from the
>>> mnemonic pattern.
>>
>> Regards, Pavel
>>>
>>> Thanks,
>>> Alexandr.
>>>
>>>>
>>>> Regards, Pavel
>>>>
>>>>> bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7093156
>>>>> webrev: http://cr.openjdk.java.net/~alexsch/7093156/webrev.00/
>>>>>
>>>>> The properties in the swing resources files are changed from the
>>>>> xxxText=ABC
>>>>> xxxMnemonic=B
>>>>> to
>>>>> xxxTextAndMnemonic=A&BC
>>>>>
>>>>> where Text suffix can be one of the following: "NameText",
>>>>> "nameText", "Text" and "Title"
>>>>>
>>>>>
>>>>> The hashmap in UIDefaults class is extended to return the correct
>>>>> values for the keys xxxText and xxxMnemonic from the stored
>>>>> xxxTextAndMnemonic string.
>>>>>
>>>>> There is no html text in the swing resource files so this case is
>>>>> not considered.
>>>>> The fix covers only the swing resource files in the JDK:
>>>>> src/share/classes/com/sun/swing/internal/plaf
>>>>> src/share/classes/com/sun/java/swing/plaf
>>>>>
>>>>> The demo resources will be fixed in the separated issue.
>>>>>
>>>>> Thanks,
>>>>> Alexandr.
>>>>>
>>>>
>>>
>>
>
>
More information about the swing-dev
mailing list