On Thu, 10 Aug 2023 07:39:06 GMT, Tejesh R <[email protected]> wrote:
>> On `NewFolderAction`, plain String is added `Action.ACTION_COMMAND_KEY`.
>> Converting the `String `to `locale` before adding as command key fix the
>> issue.
>> I have verified the test in all other platforms and Look and Feel which has
>> option to create New Folder, results were fine. No regressions found on CI
>> system with the fix. Added manual test to verify the fix.
>
> Tejesh R has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Test update
Changes requested by aivanov (Reviewer).
src/java.desktop/share/classes/javax/swing/filechooser/FileSystemView.java line
1:
> 1: /*
Update the copyright year.
src/java.desktop/share/classes/javax/swing/filechooser/FileSystemView.java line
785:
> 783: String newFolderString =
> 784: UIManager.getString("FileChooser.other.newFolder");
> 785: String newFolderNextString =
Suggestion:
String newFolderNextString =
test/jdk/javax/swing/JFileChooser/NewFolderLocale/FileChooserNewFolderLocaleTest.java
line 36:
> 34: * @summary Test to check if created newFolder is updated with
> 35: * changing Locale.
> 36: * @run main FileChooserNewFolderLocaleTest
Suggestion:
* @run main/othervm -Duser.language=en -Duser.country=US
FileChooserNewFolderLocaleTest
To ensure the test starts with US English locale.
test/jdk/javax/swing/JFileChooser/NewFolderLocale/FileChooserNewFolderLocaleTest.java
line 42:
> 40: static ResourceBundle res;
> 41: static String newFolderKey;
> 42: static String newFolderSubKey;
These could be local variables if you inline the `setLocale` method.
test/jdk/javax/swing/JFileChooser/NewFolderLocale/FileChooserNewFolderLocaleTest.java
line 46:
> 44: public static void main(String[] args) throws Exception {
> 45: File newFolderEnglish = null;
> 46: File newFolderFrench = null;
~~Declare variables where they're used for the first time.~~
I see, you use them for cleaning up in the finally block.
test/jdk/javax/swing/JFileChooser/NewFolderLocale/FileChooserNewFolderLocaleTest.java
line 64:
> 62:
> 63: setLocale("fr");
> 64: fileChooser = new JFileChooser();
You should be able to re-use the same instance.
test/jdk/javax/swing/JFileChooser/NewFolderLocale/FileChooserNewFolderLocaleTest.java
line 68:
> 66:
> fileChooser.getFileSystemView().createNewFolder(currentDir);
> 67: if( !newFolderFrench.getName().contains
> 68: (UIManager.getString(newFolderKey))) {
Suggestion:
if
(!newFolderFrench.getName().endsWith(UIManager.getString(newFolderKey))) {
Better yet, create a constant for `FRENCH_NEW_FOLDER = "Nouveau dossier"` and
use it to put the value to `UIManager` as well as to check it's used.
You may want to ensure, `newFolderEnglish` ends with `"New Folder"`.
test/jdk/javax/swing/JFileChooser/NewFolderLocale/FileChooserNewFolderLocaleTest.java
line 78:
> 76: System.out.println("Failed to delete file : " +
> 77: newFolderEnglish.getName());
> 78: }
Perhaps, you don't need to print anything here at all or report an error only.
test/jdk/javax/swing/JFileChooser/NewFolderLocale/FileChooserNewFolderLocaleTest.java
line 114:
> 112: res.getString("new_folder"));
> 113: UIManager.put(newFolderSubKey,
> 114: res.getString("new_folder") + " ({0})");
The only thing needed here is:
UIManager.put(newFolderKey, "Nouveau dossier");
UIManager.put(newFolderSubKey, "Nouveau dossier ({0})");
And this could be inlined into the test method code.
-------------
PR Review: https://git.openjdk.org/jdk/pull/15069#pullrequestreview-1602811647
PR Review Comment: https://git.openjdk.org/jdk/pull/15069#discussion_r1310386196
PR Review Comment: https://git.openjdk.org/jdk/pull/15069#discussion_r1310348256
PR Review Comment: https://git.openjdk.org/jdk/pull/15069#discussion_r1310379187
PR Review Comment: https://git.openjdk.org/jdk/pull/15069#discussion_r1310395036
PR Review Comment: https://git.openjdk.org/jdk/pull/15069#discussion_r1310360687
PR Review Comment: https://git.openjdk.org/jdk/pull/15069#discussion_r1310361661
PR Review Comment: https://git.openjdk.org/jdk/pull/15069#discussion_r1310377988
PR Review Comment: https://git.openjdk.org/jdk/pull/15069#discussion_r1310397046
PR Review Comment: https://git.openjdk.org/jdk/pull/15069#discussion_r1310371568
