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.gibb...@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
