RFR: JDK-8241780 Allow \n@ inside inline tags.

Pavel Rappo pavel.rappo at oracle.com
Thu Apr 23 18:38:31 UTC 2020


Looks good.

> On 23 Apr 2020, at 00:54, Jonathan Gibbons <jonathan.gibbons at oracle.com> wrote:
> 
> Updated webrev: no change to src/, all tests now pass.
> 
> The HTML docs compare OK before/after the change, ignoring version info in the header for each page, and ignoring one file which compares differently because of an invalid doc comment (with an unterminated `{@code` tag!)
> 
> Webrev: http://cr.openjdk.java.net/~jjg/8241780/webrev.01/index.html
> 
> -- Jon
> 
> On 4/22/20 2:45 PM, Jonathan Gibbons wrote:
>> Pavel,
>> 
>> You're right, the loop label can go away, and Mach5 found a test failure, so there will be a followup webrev.
>> 
>> While I agree with the sentiment of checking the visual appearance in a browser, this is a previously unsolved problem, nor does this changeset attempt to address that problem, so it remains unsolved, for now.  I have no suggestions for anything we can use, other than to manually inspect pages.
>> 
>> -- Jon
>> 
>> On 4/22/20 2:35 PM, Pavel Rappo wrote:
>>> Hi Jon,
>>> 
>>> That's really good news for doc-comment writers! Many thanks for doing that.
>>> 
>>> I'm not an expert in com.sun.tools.javac.parser.DocCommentParser, but that change looks pretty straightforward. I have a couple of comments though.
>>> 
>>> 1. We should check that this change passes the tests yet leaves the OpenJDK API documentation unaffected. The documentation shouldn't change, nor do I expect it to do so.
>>> 
>>> 2. If all goes well in #1, I'm willing to volunteer a followup task of removing workarounds for the previous behaviour in OpenJDK.
>>> 
>>> To accomplish what is described in #1 and #2 we need an agreed way of testing the visual appearance of javadoc HTML output. Whatever the tool or process we choose, it must be capable of ignoring variance in markup as long as it *looks* the same in a browser. What would you suggest we should use?
>>> 
>>> Nit. I think that the "loop" label can go away now.
>>> 
>>> -Pavel
>>> 
>>>> On 22 Apr 2020, at 18:27, Jonathan Gibbons <jonathan.gibbons at oracle.com> wrote:
>>>> 
>>>> Please review a change that will permit the use of a previously illegal construction, to allow <newline> <spaces> `@` inside inline tags. This will allow "<pre>{@code..." constructions that contain annotations, such as the following:
>>>> 
>>>>     /**
>>>>      * This is a method.
>>>>      * <pre>{@code
>>>>      *    @Override
>>>>      *    void m() { }
>>>>      * }</pre>
>>>>      */
>>>> 
>>>> Previously, the text was first parsed into the main body and subsequent block tags, and only then were those analyzed for inline tags. That meant that the example just given was invalid, for having an incomplete inline tag between `<pre>` and an apparent block tag named `@Override`. With the change, `@` at the beginning of a line inside an inline tag will not be treated as the beginning of a block tag, and so the comment will parse as might be expected.
>>>> 
>>>> The change to the code is as simple as deleting the code that detected block tags inside inline tags.
>>>> 
>>>> There are some minor compatibility effects.
>>>> 
>>>> 1. Some comments that were previously invalid may become valid. This is the desired effect.
>>>> 
>>>> 2. Some comments that previously invalid may now be parsed differently. In particular, in the case of a genuinely missing '}', the parse will now swallow up any block tags that might follow.  For example, consider the following:
>>>> 
>>>>      /**
>>>>       * This is a method.
>>>>       * @param p1 this has an unbalanced {@code description
>>>>       * @param p2 this is the second parameter
>>>>       */
>>>>      void m(int p1, int p2) { }
>>>> 
>>>> As a result of the change, the description for parameter p2 will be swallowed up as part of the invalid description for p1. This will only be visible in error messages and clients of the API that analyze erroneous comments.
>>>> 
>>>> The tests are updated to accommodate the change. A specific test for the `<pre>{@code...}</pre>` construction is added.  The biggest change is to the test code that "predicts" the output of the AST pretty printer, which is now updated to better handle the new behavior of `{@code}` and `{@literal}` tags.
>>>> 
>>>> -- Jon
>>>> 
>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8241780
>>>> Webrev: http://cr.openjdk.java.net/~jjg/8241780/webrev.00/index.html
>>>> 
>>>> 
>>>> 



More information about the javadoc-dev mailing list