Re: [jdk16] RFR: JDK-8247994: Localize javadoc search [v4]
On Thu, 17 Dec 2020 17:10:10 GMT, Jonathan Gibbons wrote: >> This is for JDK16, as a precursor to fixing JDK-8258002. >> >> While it is good to be using localized strings in the generated output, the >> significance for JDK-8258002 is that the strings are now obtained from a >> resource file, and not hardcoded in JavaScript file itself. >> >> The source file `search.js` is renamed to `search.js.template`, and (unlike >> other template files) is copied as-is into the generated image. The values >> in the template are resolved when javadoc is executed, depending on the >> locale in use at the time. Because of the change in the file's extension, >> two makefiles are updated to accommodate the new extension: one is for the >> "interim" javadoc used to generate the API docs; the other is for the >> primary javadoc in the main JDK image. > > Jonathan Gibbons has updated the pull request incrementally with one > additional commit since the last revision: > > Address review comment Marked as reviewed by hannesw (Reviewer). - PR: https://git.openjdk.java.net/jdk16/pull/16
Re: [jdk16] RFR: JDK-8247994: Localize javadoc search [v4]
> This is for JDK16, as a precursor to fixing JDK-8258002. > > While it is good to be using localized strings in the generated output, the > significance for JDK-8258002 is that the strings are now obtained from a > resource file, and not hardcoded in JavaScript file itself. > > The source file `search.js` is renamed to `search.js.template`, and (unlike > other template files) is copied as-is into the generated image. The values in > the template are resolved when javadoc is executed, depending on the locale > in use at the time. Because of the change in the file's extension, two > makefiles are updated to accommodate the new extension: one is for the > "interim" javadoc used to generate the API docs; the other is for the primary > javadoc in the main JDK image. Jonathan Gibbons has updated the pull request incrementally with one additional commit since the last revision: Address review comment - Changes: - all: https://git.openjdk.java.net/jdk16/pull/16/files - new: https://git.openjdk.java.net/jdk16/pull/16/files/1a406f35..de6802c0 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk16&pr=16&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk16&pr=16&range=02-03 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk16/pull/16.diff Fetch: git fetch https://git.openjdk.java.net/jdk16 pull/16/head:pull/16 PR: https://git.openjdk.java.net/jdk16/pull/16
Re: [jdk16] RFR: JDK-8247994: Localize javadoc search [v3]
On Thu, 17 Dec 2020 16:39:17 GMT, Jonathan Gibbons wrote: >> This is for JDK16, as a precursor to fixing JDK-8258002. >> >> While it is good to be using localized strings in the generated output, the >> significance for JDK-8258002 is that the strings are now obtained from a >> resource file, and not hardcoded in JavaScript file itself. >> >> The source file `search.js` is renamed to `search.js.template`, and (unlike >> other template files) is copied as-is into the generated image. The values >> in the template are resolved when javadoc is executed, depending on the >> locale in use at the time. Because of the change in the file's extension, >> two makefiles are updated to accommodate the new extension: one is for the >> "interim" javadoc used to generate the API docs; the other is for the >> primary javadoc in the main JDK image. > > Jonathan Gibbons has updated the pull request incrementally with one > additional commit since the last revision: > > Address review comment src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/search.js.template line 40: > 38: var MIN_RESULTS = 3; > 39: var MAX_RESULTS = 500; > 40: var UNNAMED = ">"; Oops, an additional closing angle bracket slipped there. - PR: https://git.openjdk.java.net/jdk16/pull/16
Re: [jdk16] RFR: JDK-8247994: Localize javadoc search [v3]
> This is for JDK16, as a precursor to fixing JDK-8258002. > > While it is good to be using localized strings in the generated output, the > significance for JDK-8258002 is that the strings are now obtained from a > resource file, and not hardcoded in JavaScript file itself. > > The source file `search.js` is renamed to `search.js.template`, and (unlike > other template files) is copied as-is into the generated image. The values in > the template are resolved when javadoc is executed, depending on the locale > in use at the time. Because of the change in the file's extension, two > makefiles are updated to accommodate the new extension: one is for the > "interim" javadoc used to generate the API docs; the other is for the primary > javadoc in the main JDK image. Jonathan Gibbons has updated the pull request incrementally with one additional commit since the last revision: Address review comment - Changes: - all: https://git.openjdk.java.net/jdk16/pull/16/files - new: https://git.openjdk.java.net/jdk16/pull/16/files/ec85..1a406f35 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk16&pr=16&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk16&pr=16&range=01-02 Stats: 3 lines in 2 files changed: 0 ins; 1 del; 2 mod Patch: https://git.openjdk.java.net/jdk16/pull/16.diff Fetch: git fetch https://git.openjdk.java.net/jdk16 pull/16/head:pull/16 PR: https://git.openjdk.java.net/jdk16/pull/16
Re: [jdk16] RFR: JDK-8247994: Localize javadoc search [v2]
> This is for JDK16, as a precursor to fixing JDK-8258002. > > While it is good to be using localized strings in the generated output, the > significance for JDK-8258002 is that the strings are now obtained from a > resource file, and not hardcoded in JavaScript file itself. > > The source file `search.js` is renamed to `search.js.template`, and (unlike > other template files) is copied as-is into the generated image. The values in > the template are resolved when javadoc is executed, depending on the locale > in use at the time. Because of the change in the file's extension, two > makefiles are updated to accommodate the new extension: one is for the > "interim" javadoc used to generate the API docs; the other is for the primary > javadoc in the main JDK image. Jonathan Gibbons has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains two additional commits since the last revision: - Merge remote-tracking branch 'upstream/master' into localize.search.js - JDK-8247994: Localize javadoc search - Changes: - all: https://git.openjdk.java.net/jdk16/pull/16/files - new: https://git.openjdk.java.net/jdk16/pull/16/files/8eb7f27b..ec85 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk16&pr=16&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk16&pr=16&range=00-01 Stats: 1532 lines in 52 files changed: 1207 ins; 121 del; 204 mod Patch: https://git.openjdk.java.net/jdk16/pull/16.diff Fetch: git fetch https://git.openjdk.java.net/jdk16 pull/16/head:pull/16 PR: https://git.openjdk.java.net/jdk16/pull/16
Re: [jdk16] RFR: JDK-8247994: Localize javadoc search
On Mon, 14 Dec 2020 09:11:31 GMT, Hannes Wallnöfer wrote: >> This is for JDK16, as a precursor to fixing JDK-8258002. >> >> While it is good to be using localized strings in the generated output, the >> significance for JDK-8258002 is that the strings are now obtained from a >> resource file, and not hardcoded in JavaScript file itself. >> >> The source file `search.js` is renamed to `search.js.template`, and (unlike >> other template files) is copied as-is into the generated image. The values >> in the template are resolved when javadoc is executed, depending on the >> locale in use at the time. Because of the change in the file's extension, >> two makefiles are updated to accommodate the new extension: one is for the >> "interim" javadoc used to generate the API docs; the other is for the >> primary javadoc in the main JDK image. > > src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/search.js.template > line 40: > >> 38: var MIN_RESULTS = 3; >> 39: var MAX_RESULTS = 500; >> 40: var UNNAMED = "##REPLACE:doclet.search.unnamed##"; > > `` is not a string that is ever shown to the user, it is what is > used by javadoc to denote the default package (see > `toolkit.util.DocletConstants.DEFAULT_PACKAGE_NAME`). This variable shouldn't > be localized as that would break behaviour for code in the default package > (and show the localized string as package name, instead of no package name). Noted, thanks - PR: https://git.openjdk.java.net/jdk16/pull/16
Re: [jdk16] RFR: JDK-8247994: Localize javadoc search
On Sun, 13 Dec 2020 00:19:59 GMT, Jonathan Gibbons wrote: > This is for JDK16, as a precursor to fixing JDK-8258002. > > While it is good to be using localized strings in the generated output, the > significance for JDK-8258002 is that the strings are now obtained from a > resource file, and not hardcoded in JavaScript file itself. > > The source file `search.js` is renamed to `search.js.template`, and (unlike > other template files) is copied as-is into the generated image. The values in > the template are resolved when javadoc is executed, depending on the locale > in use at the time. Because of the change in the file's extension, two > makefiles are updated to accommodate the new extension: one is for the > "interim" javadoc used to generate the API docs; the other is for the primary > javadoc in the main JDK image. Build changes look good. - Marked as reviewed by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk16/pull/16
Re: [jdk16] RFR: JDK-8247994: Localize javadoc search
On Sun, 13 Dec 2020 00:22:04 GMT, Jonathan Gibbons wrote: >> This is for JDK16, as a precursor to fixing JDK-8258002. >> >> While it is good to be using localized strings in the generated output, the >> significance for JDK-8258002 is that the strings are now obtained from a >> resource file, and not hardcoded in JavaScript file itself. >> >> The source file `search.js` is renamed to `search.js.template`, and (unlike >> other template files) is copied as-is into the generated image. The values >> in the template are resolved when javadoc is executed, depending on the >> locale in use at the time. Because of the change in the file's extension, >> two makefiles are updated to accommodate the new extension: one is for the >> "interim" javadoc used to generate the API docs; the other is for the >> primary javadoc in the main JDK image. > > make/CompileInterimLangtools.gmk line 77: > >> 75: Standard.java, \ >> 76: EXTRA_FILES := >> $(BUILDTOOLS_OUTPUTDIR)/gensrc/$1.interim/module-info.java, \ >> 77: COPY := .gif .png .xml .css .js .js.template .txt >> javax.tools.JavaCompilerTool, \ > > Build-folk: it would be nice if this macro could use `$(jdk.javadoc_COPY)` > instead of having to duplicate entries. > (Future RFE?) I agree. The entire design of CompileJavaModules.gmk needs to be updated. I've been procrastinating on cleaning this up, maybe it's time to get going on it... - PR: https://git.openjdk.java.net/jdk16/pull/16
Re: [jdk16] RFR: JDK-8247994: Localize javadoc search
On Mon, 14 Dec 2020 09:34:31 GMT, Hannes Wallnöfer wrote:
>> This is for JDK16, as a precursor to fixing JDK-8258002.
>>
>> While it is good to be using localized strings in the generated output, the
>> significance for JDK-8258002 is that the strings are now obtained from a
>> resource file, and not hardcoded in JavaScript file itself.
>>
>> The source file `search.js` is renamed to `search.js.template`, and (unlike
>> other template files) is copied as-is into the generated image. The values
>> in the template are resolved when javadoc is executed, depending on the
>> locale in use at the time. Because of the change in the file's extension,
>> two makefiles are updated to accommodate the new extension: one is for the
>> "interim" javadoc used to generate the API docs; the other is for the
>> primary javadoc in the main JDK image.
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/DocFile.java
> line 256:
>
>> 254: sb.append(resources.getText(m.group("key")));
>> 255: } catch (MissingResourceException e) {
>> 256: sb.append(m.group());
>
> Shouldn't we propagate an error here, or issue a warning? If a key is missing
> localized properties the value from default properties should be used, so
> this should never happen, right?
I see the added check for "##REPLACE:" in TestSearch.java will catch that case,
so I guess it's ok.
-
PR: https://git.openjdk.java.net/jdk16/pull/16
Re: [jdk16] RFR: JDK-8247994: Localize javadoc search
On Sun, 13 Dec 2020 00:19:59 GMT, Jonathan Gibbons wrote:
> This is for JDK16, as a precursor to fixing JDK-8258002.
>
> While it is good to be using localized strings in the generated output, the
> significance for JDK-8258002 is that the strings are now obtained from a
> resource file, and not hardcoded in JavaScript file itself.
>
> The source file `search.js` is renamed to `search.js.template`, and (unlike
> other template files) is copied as-is into the generated image. The values in
> the template are resolved when javadoc is executed, depending on the locale
> in use at the time. Because of the change in the file's extension, two
> makefiles are updated to accommodate the new extension: one is for the
> "interim" javadoc used to generate the API docs; the other is for the primary
> javadoc in the main JDK image.
Looks good in general. `` shouldn't be localized as it is an internal
tag (see inline comment).
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/search.js.template
line 40:
> 38: var MIN_RESULTS = 3;
> 39: var MAX_RESULTS = 500;
> 40: var UNNAMED = "##REPLACE:doclet.search.unnamed##";
`` is not a string that is ever shown to the user, it is what is used
by javadoc to denote the default package (see
`toolkit.util.DocletConstants.DEFAULT_PACKAGE_NAME`). This variable shouldn't
be localized as that would break behaviour for code in the default package (and
show the localized string as package name, instead of no package name).
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/DocFile.java
line 256:
> 254: sb.append(resources.getText(m.group("key")));
> 255: } catch (MissingResourceException e) {
> 256: sb.append(m.group());
Shouldn't we propagate an error here, or issue a warning? If a key is missing
localized properties the value from default properties should be used, so this
should never happen, right?
-
Changes requested by hannesw (Reviewer).
PR: https://git.openjdk.java.net/jdk16/pull/16
[jdk16] RFR: JDK-8247994: Localize javadoc search
This is for JDK16, as a precursor to fixing JDK-8258002. While it is good to be using localized strings in the generated output, the significance for JDK-8258002 is that the strings are now obtained from a resource file, and not hardcoded in JavaScript file itself. The source file `search.js` is renamed to `search.js.template`, and (unlike other template files) is copied as-is into the generated image. The values in the template are resolved when javadoc is executed, depending on the locale in use at the time. Because of the change in the file's extension, two makefiles are updated to accommodate the new extension: one is for the "interim" javadoc used to generate the API docs; the other is for the primary javadoc in the main JDK image. - Commit messages: - JDK-8247994: Localize javadoc search Changes: https://git.openjdk.java.net/jdk16/pull/16/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk16&pr=16&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8247994 Stats: 122 lines in 9 files changed: 88 ins; 6 del; 28 mod Patch: https://git.openjdk.java.net/jdk16/pull/16.diff Fetch: git fetch https://git.openjdk.java.net/jdk16 pull/16/head:pull/16 PR: https://git.openjdk.java.net/jdk16/pull/16
Re: [jdk16] RFR: JDK-8247994: Localize javadoc search
On Sun, 13 Dec 2020 00:19:59 GMT, Jonathan Gibbons wrote: > This is for JDK16, as a precursor to fixing JDK-8258002. > > While it is good to be using localized strings in the generated output, the > significance for JDK-8258002 is that the strings are now obtained from a > resource file, and not hardcoded in JavaScript file itself. > > The source file `search.js` is renamed to `search.js.template`, and (unlike > other template files) is copied as-is into the generated image. The values in > the template are resolved when javadoc is executed, depending on the locale > in use at the time. Because of the change in the file's extension, two > makefiles are updated to accommodate the new extension: one is for the > "interim" javadoc used to generate the API docs; the other is for the primary > javadoc in the main JDK image. make/CompileInterimLangtools.gmk line 77: > 75: Standard.java, \ > 76: EXTRA_FILES := > $(BUILDTOOLS_OUTPUTDIR)/gensrc/$1.interim/module-info.java, \ > 77: COPY := .gif .png .xml .css .js .js.template .txt > javax.tools.JavaCompilerTool, \ Build-folk: it would be nice if this macro could use `$(jdk.javadoc_COPY)` instead of having to duplicate entries. (Future RFE?) - PR: https://git.openjdk.java.net/jdk16/pull/16
