Jon, 1. The patch has become a tad bit outdated after to your recent push. Just a heads-up.
2. 145 if (locale == Locale.getDefault()) { That looks very conservative, I would expect `equals()`. Since it's about a one-off optimization, it is not a big deal. I just stumbled upon that. Thanks for small cleanups along the way. Looks good to me. -Pavel > On 6 Feb 2020, at 19:26, Jonathan Gibbons <jonathan.gibb...@oracle.com> wrote: > > Please review a change to use separate locales for messages written to the > console and for content written in the generated HTML pages. > > Background: although the command-line help is not very informative about how > the -locale option is used, the man page is clear that it is clear that the > option affects the generated documentation. > > Command-line help: > > -locale <name> > Locale to be used, e.g. en_US or en_US_WIN > > Man page: > > `-locale` *name* > : Specifies the locale that the `javadoc` tool uses when it generates > documentation. The argument is the name of the locale, as described in > `java.util.Locale` documentation, such as `en_US` (English, United States) > or `en_US_WIN` (Windows variant). > > With the proposed change, the -locale option only affects the generated > documentation; it does not affect console output, which will use the default > locale, in line with other JDK tools. > > > > The changes ... > > The core of the change is in HtmlConfiguration, which creates the resource > bundles for the Messages object (used to generate console messages) and for > Configuration.getResources(), which is used to get the resources for content > in the generated output. To clarify the distinction, > BaseConfiguration.getResources is renamed to > BaseConfiguration.getDocResources, to emphasise that it is to be used for > resources in the generated documentation. This rename percolates through most > of the other affected files. The rename also highlighted some issues in > HtmlOptions, which were using the low-level reporter and resources API to > generate messaages, instead of using the Messages API. These have now been > fixed. > > The tests ... > > There's a somewhat surprising test case in TestSearch, which was testing the > search feature when using Japanese and Chinese locales. The primary part of > these test cases seems to be that the locale setting does *not* affect the > generated search index files, but perhaps as a control to verify the locale > option is taking effect, the test cases also verify some of the output > generated by javadoc. It's not clear how useful the test cases are, but > rather than simply remove them, each one is split in two: one to set the > default locale, and one to use the locale option. > > The primary test update is the recently-new TestLocaleOption.java, which is > updated to be able to test the use of the default locale and the locale > option. The test is refactored a bit to share more of the common code for the > different test cases. > > > > Additional work ... > > The command-line help and man page should be improved. The man page states > that the -locale option should be first on the command line, which is no > longer the case. > The change should probably have a release note. > > > > -- Jon > > > > JBS: https://bugs.openjdk.java.net/browse/JDK-8238437 > Webrev: http://cr.openjdk.java.net/~jjg/8238437/webrev/ > > > > >