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.
