RFR: JDK-8289332: Auto-generate ids for user-defined headings [v2]
Hannes Wallnöfer
hannesw at openjdk.org
Wed Aug 17 14:27:10 UTC 2022
On Tue, 16 Aug 2022 19:58:52 GMT, Jonathan Gibbons <jjg at openjdk.org> wrote:
>> Hannes Wallnöfer has updated the pull request incrementally with two additional commits since the last revision:
>>
>> - Remove trailing whitespace
>> - Address review feedback
>>
>> - Make sure hasIdAttribute is safe
>> - Move id generating code to HtmlIds
>> - Rename -hdr suffix to -heading
>> - Rename header to heading in code
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java line 1762:
>
>> 1760: private boolean hasIdAttribute(StartElementTree node) {
>> 1761: return node.getAttributes().stream().anyMatch(
>> 1762: dt -> equalsIgnoreCase(((AttributeTree)dt).getName(), "id"));
>
> Do you need to filter/map the stream to ensure no class cast exceptions?
> Can the attributes include `ErroneousTree`?
Cood catch, I added a pattern matching instanceof which should be the easiest way to fix the problem.
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java line 1791:
>
>> 1789: if (!idValue.isEmpty()) {
>> 1790: // Make id unique
>> 1791: idValue = idValue + "-hdr";
>
> Would it be better to use `header` rather than just `hdr` ?
> Better still, `heading` seems to be the term in the HTML spec; `header` is also used, but not for headings.
I renamed the `-hdr` suffix to `-heading` and also renamed code occurrences of "header" to "heading" (except for the test name).
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java line 1800:
>
>> 1798: }
>> 1799: content.add("id=\"").add(idValue).add("\"");
>> 1800: }
>
> This part of the algorithm, to create an `id` from the text contents of a header, still feels like it belongs in `HtmlIds`. It feels like we should have
>
> HtmlIds.getIdForHeader(CharSequence headerText)
>
> or something like that
I moved the code to a new `HtmlIds.forHeading(CharSequence, Set<String>)` method. The `forHeading` follows the naming scheme of `HtmlIds`. The second argument of type `Set<String>` is to make the returned id unique within its page.
-------------
PR: https://git.openjdk.org/jdk/pull/9627
More information about the javadoc-dev
mailing list