RFR: 8338133: Cleanup direct use of `new HtmlTree`
Jonathan Gibbons
jjg at openjdk.org
Tue Sep 3 18:01:22 UTC 2024
On Mon, 2 Sep 2024 10:50:30 GMT, Hannes Wallnöfer <hannesw at openjdk.org> wrote:
>> Please review an update to "clean up" the direct use of HtmlTree constructors.
>>
>> Hitherto, many/most instances of `HtmlTree` were created by static factory methods. This update extends that convention.
>> In most cases, this is by providing either simple no-arg factory methods or commonly used overloads that take an `HtmlId` or `HtmlStyle`.
>>
>> For some tags, (`br`, `hr`, `wbr`) this allows a singleton instance to be used.
>> For some of the more obscure cases, a more generic `HtmlTree.of(HtmlTag)` method was used.
>>
>> Notes:
>> * some significant block-level nodes, like `pre`, should probably always set a style, which could be enforced by suitable factory methods. That is currently not the case and could be a future cleanup.
>> * some lists put the same style info on each list item, but might be better placed on the enclosing list. That could be a future cleanup
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/IndexRedirectWriter.java line 107:
>
>> 105:
>> 106: var body = HtmlTree.BODY(HtmlStyles.indexRedirectPage)
>> 107: .add(bodyContent);
>
> I assume the `<main>` element is omitted intentially here? If so I'm fine with that, but I just want to make sure it's not an accidental change.
Good catch; that is an accident. I'll verify and fix.
Curiously, it did not cause any test to fail, or the overall JDK API output to be different.
For a redirect page, it's not clear there should be a `<main>` element, but that should be a separate possibly stand-alone change.
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/SearchWriter.java line 114:
>
>> 112: .add(HtmlTree.of(HtmlTag.P)
>> 113: .setId(HtmlId.of("page-search-notify"))
>> 114: .add(contents.getContent("doclet.search.loading")))
>
> We could combine `.of` and `.add` into `HtmlTree.P(Content)` here.
will look at that
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20778#discussion_r1742461238
PR Review Comment: https://git.openjdk.org/jdk/pull/20778#discussion_r1742461857
More information about the javadoc-dev
mailing list