On Fri, 5 Feb 2021 04:44:07 GMT, Jonathan Gibbons <j...@openjdk.org> wrote:
>> Please review an update to improve the support, where appropriate, for
>> nested inline tags.
>>
>> It has always been the case that certain inline tags allow text and HTML to
>> appear within them. With tags like `{@return}` and `{@summary}` it becomes
>> desirable to also generally allow nested inline tags to appear in those
>> places as well. The work for this was started with the support for
>> `{@return}` [JDK-8075778](https://bugs.openjdk.java.net/browse/JDK-8075778),
>> but applying the work more generally was out of scope at the time. This
>> change completes the work that was started then.
>>
>> The work can be grouped into 4 parts, in 3 commits.
>>
>> ## Commit 1
>>
>> * Update `DocCommentParser` to syntactically allow nested inline tags in
>> situations that permit text and HTML
>> * Update the downstream code to semantically limit nestg where it does not
>> make sense, and provide new tests to verify the behavior.
>>
>> A family of new tests are added, each testing the ability to put an inline
>> tag within another of the same kind, with and without doclint being enabled.
>> In addition, test cases are added placing a simple instance `{@value}` in an
>> enclosing tag: this is a useful case just because the expansion is plain
>> text and therefore valid in all situations. Additional tests and test cases
>> can be added as needed.
>>
>> This commit left the `{@index}` tag generating "bad" code when it was
>> nested. The error was "reference to an undeclared ID". The (temporary)
>> solution was to disable the automatic link checking for this specific
>> subtest.
>>
>> ## Commit 2
>>
>> * `HtmlDocletWriter` and `TagletWriterImpl ` pass around a pair of booleans
>> `isFirstSentence` and `inSummary` to help determine the output to be
>> generated. Conceptually, a third value is added to that group: a set
>> containing the set of nested tag kinds, so that it is possible to determine
>> the enclosing tags for a tag. But, rather than add a third parameter to be
>> passed around, the 3 are grouped into a new class `TagletWriterImpl.Context`
>> which encapsulates the two booleans and the new set. The new class is added
>> in a way to minimize code churn. No tests are affected by this change: all
>> continue to pass.
>>
>> ## Commit 3
>>
>> * The new `Context#inTags` field is used to help improve the behavior of
>> nested `{@index}` tags even when used incorrectly, with warnings disabled.
>> As a result, the temporary change in the first commit to disable automatic
>> link checking in one of the test cases is reverted.
>>
>> <hr>
>>
>> The introduction of the new Context class is arguably more general than we
>> need at this time, but it clears up some erratic and inconsistent use of the
>> `isFirstSentence` and `inSummary` booleans. The new class also provides a
>> better framework for any complex new inline tags we may add in future. We
>> might want to change the `Set<DocTree.Kind>` to some other collection at
>> some point, if needs be (a stack, for example.) We might also want to move
>> more information into the `Context`, such as the related `Element` that is
>> otherwise ubiquitously passed around.
>>
>> The overall cleanup also revealed some latent bugs in the code, that were
>> hidden in some of the tests. Most notable was that there were still some
>> cases were `<` and `>` were not being correctly escaped as `<` and `>`
>> leading to output in some tests of the form `List<String>` ! This triggered
>> a minor cleanup/rewrite of the beginning of
>> `HtmlDocletWriter.seeTagsToContent` which was previously a bit too liberal
>> with the use of `new RawHtml`! The other minor cleanup was more consistent
>> handling of whitespace at the end of the first sentence, as will be seen in
>> a couple of places in one of the tests that was updated.
>
> Jonathan Gibbons has updated the pull request incrementally with one
> additional commit since the last revision:
>
> improve handling of nested links
Thanks for fixing the nested `<a>` issue. With your last commit (96bfeb5) the
output from
/** First sentence. {@link #m1() ABC {@link #m2() DEF} GHI} */
looks like this
First sentence. <a href="#m1()"><code>ABC DEF GHI</code></a>
and if doclint is enabled, javadoc issues a warning
warning: nested tag: @link
/** First sentence. {@link #m1() ABC {@link #m2() DEF} GHI} */
^
I would recommend comparing JDK API documentation before and after, if only to
bring any uncovered surprises to maintainers' attention early.
-------------
Marked as reviewed by prappo (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/2369
