OK, thanks, I guess I understand ... I think you're saying that these
tests are verifying that the search indexes are not affected by any
locale setting and that (presumably) the check for locale-specific
output in the test is just a "control" to verify the test is running in
a different locale, right?
-- Jon
On 02/07/2020 08:42 AM, Pavel Rappo wrote:
Oh, almost forgot. The reason why there's "a somewhat surprising test case in
TestSearch" is quite trivial. That test appeared in
changeset: 52695:99eb43bc3595
user: hannesw
date: Tue Nov 27 13:02:28 2018 +0100
summary: 8213716: javadoc search not working with Japanese and Chinese
locales
The crux of the problem was that the code was comparing hardcoded strings
(written in English) with locale-sensitive ones. That changeset did away with
that and introduced an enum-based comparison instead.
before:
si.setCategory(resources.getText("doclet.Types"));
...
} else if (category.equals("Types")) {
after:
si.setCategory(SearchIndexItem.Category.TYPES);
...
case TYPES:
See, no magic.
-Pavel
On 7 Feb 2020, at 16:29, Pavel Rappo <pavel.ra...@oracle.com> wrote:
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/
