Mandy,
Thanks for the review....please see in-lined comments,
On Jul 20, 2017, at 11:53 AM, Kumar Srinivasan<kumar.x.sriniva...@oracle.com>
wrote:
Hi,
Please review refactoring and clean up of the java launcher's help/usage
messages.
The highlights of the changes are as follows:
1. Helper.java: is renamed from LauncherHelper.java, simpler name,
containing mostly methods to help with class loading, module assistance etc.
There are tests depending on `sun.launcher.LauncherHelper` class name.
I actually like LauncherHelper better than Helper to make it clear
what it is just by its simple name.
The rationale was to simplify the fullname from
sun.launcher.LauncherHelper -> sun.launcher.Helper
wrt. the test Ugh, Ok renamed Helper.java back to LauncherHelper.java
Webrev:
http://cr.openjdk.java.net/~ksrini/8183579/webrev.00/
launcher.properties
40 # Translators please note do not translate the options themselves
Option names are no longer in the resource bundle.
It would be helpful to add the comment describing the key name
java.launcher.opt.$OPTION_NAME.param
java.launcher.opt.$OPTION_NAME.desc
Added the notes/comments describing what needs to be done wrt.
adding any new keys.
A newline between each option may help readability.
Since the whitespace in description is irrelevant, maybe keep the indentation
to 4 space?
Done
Does the help message show the options in the order listed here?
In the declaration order.
I would hope it uses the order of the Option enums. If so, we could
list them in alphabetical order in launcher.properties.
The keys are alpha sorted now.
80 java.launcher.opt.x.desc = print help on extra options to the error stream
Should $OPTION_NAME match the option name (rather than toLowerCase)?
Ok used the Enum name directly with no translations.
OptionsHelper.java
311 Arrays.asList(Option.values()).stream()
Alternative: Stream(Option.values())
Done.
This class includes the entry point for -XshowSettings but not other options
such as —-list-modules. It may cause confusion to which class a new launcher
option is added. It may be okay. Maybe just move the code for printing the
help messages in Option class and consider the refactoring for -XshowSetting
and other options in the future.
As you suggested, moved only the two usage helper methods to the new
UsageHelper.
This simplifies the changes.
New webrev at:
http://cr.openjdk.java.net/~ksrini/8183579/webrev.01/
Thanks
Kumar
Mandy
