On Wed, 12 Jul 2023 11:55:51 GMT, Pavel Rappo <pra...@openjdk.org> wrote:
>> Jonathan Gibbons has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Improve comments
>> Convert `InheritableTaglet` to just a marker interface.
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/TagletWriterImpl.java
> line 1:
>
>> 1: /*
>
> It's nice to see TagletWriterImpl go! It's not that I didn't like this class,
> it's that I didn't like the fact that the processing logic was haphazardly
> spread between this class and the taglets.
>
> That said, there's more cleanup that needs to be done in TagletWriter, later.
Agreed that `TagletWriter` is not the end of the line, and that more cleanup is
possible.
At some point, it might be worth writing a document that characterizes the
different writers, and what their functionality is. In particular, the split
between `HtmlDocletWriter` and `TagletWriter` would be worth clarifying.
I would also like to reduce/limit accessibility (i.e public/protected/etc) on
some `TagletWriter` members, to constrain unbounded usage.
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/resources/doclets.properties
> line 27:
>
>> 25:
>> 26: doclet.Generating_0=Generating {0}...
>> 27: doclet.Toolkit_Usage_Violation=The Doclet Toolkit can only be used by {0}
>
> Should we remove this key for other locales too?
In general, I assume that updates to other locales will happen automatically,
as part of the L10N process.
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Utils.java
> line 2120:
>
>> 2118:
>> 2119: public List<? extends DocTree> getBlockTags(Element element,
>> String tagName) {
>> 2120: return getBlockTags(element,
>
> For this or a later PR:
>
> 1. Even prior to this PR that `instanceof` seemed redundant. If we don't want
> to change the `getBlockTags(Element, Predicate)` return type to BlockTagTree,
> I suggest to downcast unconditionally:
>
> t -> ((BlockTagTree) t).getTagName().equals(tagName));
>
> 2. After this PR, the only client of the BaseTaglet.accepts method would be
> SimpleTaglet, which we should move that method to.
Good observation.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14793#discussion_r1261444571
PR Review Comment: https://git.openjdk.org/jdk/pull/14793#discussion_r1261445289
PR Review Comment: https://git.openjdk.org/jdk/pull/14793#discussion_r1261446205
