<Swing Dev> Review request for 7093156: NLS: Please change the mnemonic assignment system to avoid translation issue (Swing files)

[Alexander Scherbatiy]

On 3/29/2012 1:19 PM, Pavel Porvatov wrote:
> Hi Alexander,
>
> I have several questions about the patch.
>
> File TextAndMnemonicHashMap:
> 1. MNEMONIC_SUFFIX[] = {"Mnemonic", "mnemonic", "Mnemonic", "Mnemonic",}
> what's the reason have three "Mnemonic" values?
>
> 2. TEXT_SUFFIX[] = {"NameText", "nameText", "Text", "Title"}
> What is the reason to have "NameText", "nameText". It looks there is 
> no need in that because that's a particular case of "Text"

      The reason to have two arrays is because there are several 
patterns that are used for the text and mnemonic declaration.
      They are (xxxNameText, xxxMnemonic), (xxx.nameText, 
xxx.mnemonic),(xxxText, xxxMnemonic) and (xxxTitle, xxxMnemonic).

>
> 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.

    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

>
> 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.

    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.
>>
>