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.
