> 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. > > 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 A newline between each option may help readability. Since the whitespace in description is irrelevant, maybe keep the indentation to 4 space? Does the help message show the options in the order listed here? I would hope it uses the order of the Option enums. If so, we could list them in alphabetical order in launcher.properties. 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)? OptionsHelper.java 311 Arrays.asList(Option.values()).stream() Alternative: Stream(Option.values()) 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. Mandy
