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