Re: RFR: JDK-8324774: Add DejaVu web fonts [v2]
On Wed, 20 Mar 2024 15:54:12 GMT, Hannes Wallnöfer wrote: >> This change adds the DejaVu web fonts that were previously maintained >> externally to the open repository so they are available both in JDK API >> documentation and any API documentation generated with the `javadoc` tool. >> All files added in this PR are the same as the ones previously maintained >> externally, with the exception of added license and name/version comments in >> `dejavu.css`. >> >> Copying of font files to the generated documentation is done by looking for >> font file names in `dejavu.css`, so font file names can be changed without >> changing the code. However, the font file list is hard-coded in >> `APITest.java`. `CheckLibraryVersions.java` is updated to make sure the name >> and version in the legal file matches the one in the stylesheet. Of course I >> also performed manual tests to make sure the font and legal files are copied >> to the output tree and used correctly in browsers. >> >> Once #17411 is integrated, `dejavu.css` should also be added to the list of >> files checked by the new "pass-through" test. > > Hannes Wallnöfer has updated the pull request incrementally with two > additional commits since the last revision: > > - JDK-8327385: Add JavaDoc option to exclude web fonts from generated > documentation > - Merge try-with-resource statements Marked as reviewed by jjg (Reviewer). I like the new stuff for `JavadocTester` wrapped up in this work. - PR Review: https://git.openjdk.org/jdk/pull/17633#pullrequestreview-1962024918 PR Comment: https://git.openjdk.org/jdk/pull/17633#issuecomment-2021719299
Re: RFR: JDK-8324774: Add DejaVu web fonts [v2]
> This change adds the DejaVu web fonts that were previously maintained > externally to the open repository so they are available both in JDK API > documentation and any API documentation generated with the `javadoc` tool. > All files added in this PR are the same as the ones previously maintained > externally, with the exception of added license and name/version comments in > `dejavu.css`. > > Copying of font files to the generated documentation is done by looking for > font file names in `dejavu.css`, so font file names can be changed without > changing the code. However, the font file list is hard-coded in > `APITest.java`. `CheckLibraryVersions.java` is updated to make sure the name > and version in the legal file matches the one in the stylesheet. Of course I > also performed manual tests to make sure the font and legal files are copied > to the output tree and used correctly in browsers. > > Once #17411 is integrated, `dejavu.css` should also be added to the list of > files checked by the new "pass-through" test. Hannes Wallnöfer has updated the pull request incrementally with two additional commits since the last revision: - JDK-8327385: Add JavaDoc option to exclude web fonts from generated documentation - Merge try-with-resource statements - Changes: - all: https://git.openjdk.org/jdk/pull/17633/files - new: https://git.openjdk.org/jdk/pull/17633/files/2c6e2163..9f77de97 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=17633&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=17633&range=00-01 Stats: 188 lines in 7 files changed: 174 ins; 2 del; 12 mod Patch: https://git.openjdk.org/jdk/pull/17633.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17633/head:pull/17633 PR: https://git.openjdk.org/jdk/pull/17633
Re: RFR: JDK-8324774: Add DejaVu web fonts
On Tue, 30 Jan 2024 16:13:42 GMT, Hannes Wallnöfer wrote:
> This change adds the DejaVu web fonts that were previously maintained
> externally to the open repository so they are available both in JDK API
> documentation and any API documentation generated with the `javadoc` tool.
> All files added in this PR are the same as the ones previously maintained
> externally, with the exception of added license and name/version comments in
> `dejavu.css`.
>
> Copying of font files to the generated documentation is done by looking for
> font file names in `dejavu.css`, so font file names can be changed without
> changing the code. However, the font file list is hard-coded in
> `APITest.java`. `CheckLibraryVersions.java` is updated to make sure the name
> and version in the legal file matches the one in the stylesheet. Of course I
> also performed manual tests to make sure the font and legal files are copied
> to the output tree and used correctly in browsers.
>
> Once #17411 is integrated, `dejavu.css` should also be added to the list of
> files checked by the new "pass-through" test.
Some suggestions.
I think there should be an option to opt-out of using these fonts.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDoclet.java
line 470:
> 468: // Extract font file names from CSS file
> 469: URL cssURL =
> HtmlConfiguration.class.getResource(DocPaths.RESOURCES.resolve(cssPath).getPath());
> 470: InputStream in = cssURL.openStream();
Put this in try-with-resources
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDoclet.java
line 476:
> 474: try (BufferedReader reader = new BufferedReader(new
> InputStreamReader(in))) {
> 475: String line;
> 476: while ((line = reader.readLine()) != null) {
Consider using `BufferedReader.lines`
https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/io/BufferedReader.html#lines()
-
PR Review: https://git.openjdk.org/jdk/pull/17633#pullrequestreview-1891318432
PR Review Comment: https://git.openjdk.org/jdk/pull/17633#discussion_r1496355484
PR Review Comment: https://git.openjdk.org/jdk/pull/17633#discussion_r1496357502
Re: RFR: JDK-8324774: Add DejaVu web fonts
On Tue, 30 Jan 2024 16:13:42 GMT, Hannes Wallnöfer wrote: > This change adds the DejaVu web fonts that were previously maintained > externally to the open repository so they are available both in JDK API > documentation and any API documentation generated with the `javadoc` tool. > All files added in this PR are the same as the ones previously maintained > externally, with the exception of added license and name/version comments in > `dejavu.css`. > > Copying of font files to the generated documentation is done by looking for > font file names in `dejavu.css`, so font file names can be changed without > changing the code. However, the font file list is hard-coded in > `APITest.java`. `CheckLibraryVersions.java` is updated to make sure the name > and version in the legal file matches the one in the stylesheet. Of course I > also performed manual tests to make sure the font and legal files are copied > to the output tree and used correctly in browsers. > > Once #17411 is integrated, `dejavu.css` should also be added to the list of > files checked by the new "pass-through" test. Build changes look trivially fine - Marked as reviewed by ihse (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/17633#pullrequestreview-1852132288
RFR: JDK-8324774: Add DejaVu web fonts
This change adds the DejaVu web fonts that were previously maintained externally to the open repository so they are available both in JDK API documentation and any API documentation generated with the `javadoc` tool. All files added in this PR are the same as the ones previously maintained externally, with the exception of added license and name/version comments in `dejavu.css`. Copying of font files to the generated documentation is done by looking for font file names in `dejavu.css`, so font file names can be changed without changing the code. However, the font file list is hard-coded in `APITest.java`. `CheckLibraryVersions.java` is updated to make sure the name and version in the legal file matches the one in the stylesheet. Of course I also performed manual tests to make sure the font and legal files are copied to the output tree and used correctly in browsers. Once #17411 is integrated, `dejavu.css` should also be added to the list of files checked by the new "pass-through" test. - Commit messages: - Add license comment - JDK-8324774: Add DejaVu web fonts Changes: https://git.openjdk.org/jdk/pull/17633/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=17633&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8324774 Stats: 373 lines in 32 files changed: 361 ins; 0 del; 12 mod Patch: https://git.openjdk.org/jdk/pull/17633.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17633/head:pull/17633 PR: https://git.openjdk.org/jdk/pull/17633
