On Fri, 14 Jun 2024 12:12:51 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/javadocGlobalPR/pkg1/package-summary.html) >> to the generated docs. >> Thanks in advance. > > Nizar Benalla has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains ten additional > commits since the last revision: > > - Null safety > - Merge branch 'master' into html-attributes > - remove trailing whitespace > - -Add a boolean attribute to the enum type > -Simply regex in `visitAttribute` > -Simplified the Test > -Added a negative test > - no longer print summary > - no longer print summary > - Add small comment > - Remove classpath exception > - Allow global variables I've updated the PR based on everyone's suggestions, here are the main changes. Currently running tier 1-3 src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/Checker.java line 695: > 693: // custom "data-*" attributes are also accepted > 694: var attrName = name.toString(); > 695: if (!attrName.startsWith("on") && > !attrName.startsWith("data-")) { I've removed the regex here. src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/HtmlTag.java line 441: > 439: ALINK, > 440: ALT, > 441: ARIA_ACTIVEDESCENDANT(true), The enum `Attr` holds two values now, the second representing whether it's a global attribute. src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/HtmlTag.java line 670: > 668: public AttrKind getAttrKind(Name attrName) { > 669: Attr attr= getAttr(attrName); > 670: AttrKind k = attr.isGlobal()? AttrKind.OK: attrs.get(attr); // > null-safe As hannes suggested, `getAttrKind` now returns `AttrKind.OK` if we are dealing with a global attribute. Edit: It's no longer null safe so I will slightly change it src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/HtmlTag.java line 673: > 671: > 672: if (attr != null) { > 673: kind = attr.isGlobal() ? AttrKind.OK : attrs.get(attr); // > get is null-safe As hannes suggested, `getAttrKind` now returns `AttrKind.OK` if we are dealing with a global attribute. I've added a null check here because of `attr.isGlobal()` test/langtools/jdk/javadoc/doclet/TestGlobalHtml/TestGlobalHtml.java line 70: > 68: public class C { > 69: /** > 70: * <form> Added a negative test, as `<a>` and `<form>` tags aren't allowed . test/langtools/jdk/javadoc/doclet/TestGlobalHtml/pkg1/C1.java line 23: > 21: * questions. > 22: */ > 23: I've simplified the `C1` class, it's smaller now and has many smaller examples. ------------- PR Review: https://git.openjdk.org/jdk/pull/19652#pullrequestreview-2117318623 PR Review Comment: https://git.openjdk.org/jdk/pull/19652#discussion_r1639144111 PR Review Comment: https://git.openjdk.org/jdk/pull/19652#discussion_r1639145147 PR Review Comment: https://git.openjdk.org/jdk/pull/19652#discussion_r1639147374 PR Review Comment: https://git.openjdk.org/jdk/pull/19652#discussion_r1639726374 PR Review Comment: https://git.openjdk.org/jdk/pull/19652#discussion_r1639145878 PR Review Comment: https://git.openjdk.org/jdk/pull/19652#discussion_r1639146304
