On Tue, 11 Jun 2024 11:37:27 GMT, Nizar Benalla <nbena...@openjdk.org> wrote:
> Can I please get a review for this change, that aims to add support for > Global HTML tags. > Here is the > [link](https://cr.openjdk.org/~nbenalla/javadocGlobalAttrs/pkg1/package-summary.html) > to the generated docs. > Thanks in advance. src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/HtmlTag.java line 556: > 554: } > 555: > 556: private final EnumSet<Attr> GLOBAL_ATTRS = EnumSet.of( I get it, you cannot make it static because it's used in an enum ctor. But attaching it to every single enum constant seems wasteful. test/langtools/jdk/javadoc/doclet/TestGlobalHtml/TestGlobalHtml.java line 41: > 39: var tester = new TestGlobalHtml(); > 40: tester.runTests(); > 41: tester.printSummary(); Why is this summary needed? Usually, we don't print it. test/langtools/jdk/javadoc/doclet/TestGlobalHtml/pkg1/C1.java line 36: > 34: * > 35: * <p accesskey="C" autocapitalize="sentences" dir="ltr" lang="en" > data-purpose="documentation"> > 36: * This class is used to demonstrate the use of various HTML global > attributes in Javadoc comments. JavaDoc test/langtools/jdk/javadoc/doclet/TestGlobalHtml/pkg1/C1.java line 299: > 297: */ > 298: public void setUndecorated(boolean undecorated) { > 299: /* Make sure we don't run in the middle of peer creation.*/ I would delete this method-internal comment. While being immaterial, it draws too much attention. test/langtools/jdk/javadoc/doclet/TestGlobalHtml/pkg1/C2.java line 8: > 6: * under the terms of the GNU General Public License version 2 only, as > 7: * published by the Free Software Foundation. Oracle designates this > 8: * particular file as subject to the "Classpath" exception as provided No "classpath" exception is required for a test. test/langtools/jdk/javadoc/doclet/TestGlobalHtml/pkg1/C2.java line 104: > 102: * </p> > 103: * > 104: * <ul> Since you use `ol`, `ul`, and `li`elements, you could add global attributes to them too. Otherwise, you only have such attributes in `p` elements. test/langtools/jdk/javadoc/doclet/TestGlobalHtml/pkg1/C2.java line 115: > 113: */ > 114: public boolean performAction(String action) { > 115: // Placeholder implementation Delete this and the similar comments below. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/19652#discussion_r1634822550 PR Review Comment: https://git.openjdk.org/jdk/pull/19652#discussion_r1634847516 PR Review Comment: https://git.openjdk.org/jdk/pull/19652#discussion_r1634837845 PR Review Comment: https://git.openjdk.org/jdk/pull/19652#discussion_r1634841367 PR Review Comment: https://git.openjdk.org/jdk/pull/19652#discussion_r1634845398 PR Review Comment: https://git.openjdk.org/jdk/pull/19652#discussion_r1634851759 PR Review Comment: https://git.openjdk.org/jdk/pull/19652#discussion_r1634852871
