Hi Jon, This looks good to me.
I think the internal Taglet#isBlockTag method should be documented as well, but no new webrev required for that. Hannes > Am 06.12.2019 um 02:13 schrieb Jonathan Gibbons <[email protected]>: > > Please review the implementation of a change to allow taglets to support tags > that can appear as both inline tags and block tags. > > This is in parallel with the CSR review [1]. > > The core of the change is small, with a very small primary component, and a > related fixup that triggered a bunch of cleanup. > > The API change to the interface is in > src/jdk.javadoc/share/classes/jdk/javadoc/doclet/Taglet.java, which just > corresponds to the changes in the CSR. > > The core of the change is in > src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/taglets/TagletManager.java, > in the poorly-named initBlockTags() (which also initializes inline tags!). > Lines 588-598. > > Structurally, the method used to be: > > if (current.isInlineTag()) { > ... > } else { > ... > } > > > Now, because inline tags and block tags are no longer mutually exclusive, > this becomes > > if (current.isInlineTag()) { > ... > } > if (current.isBlockTag()) { > ... > } > > There is one other significant piece of crufty code that needed to be cleaned > up. The code in UserTaglet for methods like inField, inMethod, etc used to > force a return value of true if the tag was an inline tag. That doesn't work > if a taglet can support both inline and block tags. The fix is to remove the > override for isInline and to compensate in TagletManager at line 395 by only > invoking these checks if the tag is a block tag. This means there is (and > has always been) an asymmetry between inline tags (which can appear anywhere) > and block tags (which can only appear in specified locations.) This is > particularly observable in the use of the declarations for "inlineTags" > (TagletManager:88) and blockTagsBySite (TagletManager:83) This changeset does > not attempt to address/fix the asymmetry. > > In terms of other cleanup, first note that there are two Taglet interfaces > (since "forever"). There's the public interface, > jdk/javadoc/doclet/Taglet.java, and there's an internal version with a > slightly richer API that is used for all built-in > taglets,jdk/javadoc/internal/doclets/toolkit/taglets/Taglet.java. (One of > these days, we should fix that name clash!) The internal Taglet has a > subtype, BaseTaglet, which contains an enum called "BaseTaglet.Site" which > 100% duplicates the enum "Taglet.Location" in the public API. This changeset > removes the internal BaseTaglet.Site, replacing all usages with > Taglet.Location. Regrettably this affects all standard taglets, but in all > cases it's a simple rename from Site to Location. In time, we might want to > also remove the collection of methods BaseTaglet.inXYZ in favor of just > checking if the set contains the desired member: it's not clear the predicate > methods pull their weight. > > There's additional extra cleanup, such as cleaning up incorrect javadoc > comments and expanding wildcard imports in affected files. > > There's a new test that exercises a taglet that supports a tag that can be > used either inline or as a block tag. > > -- Jon > > > [1] CSR Review: > https://mail.openjdk.java.net/pipermail/javadoc-dev/2019-December/001217.html > JBS: https://bugs.openjdk.java.net/browse/JDK-8235306 > Webrev: http://cr.openjdk.java.net/~jjg/8235306/webrev/ >
