Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v36]

2024-02-15 Thread Hannes Wallnöfer
On Fri, 16 Feb 2024 00:46:34 GMT, Jonathan Gibbons  wrote:

>> Please review a patch to add support for Markdown syntax in documentation 
>> comments, as described in the associated JEP.
>> 
>> Notable features:
>> 
>> * support for `///` documentation comments in `JavaTokenizer`
>> * new module `jdk.internal.md` -- a private copy of the `commonmark-java` 
>> library
>> * updates to `DocCommentParser` to treat `///` comments as Markdown
>> * updates to the standard doclet to render Markdown comments in HTML
>
> Jonathan Gibbons has updated the pull request incrementally with two 
> additional commits since the last revision:
> 
>  - Support for Table Of Contents in Markdown headings
>  - fix typo

src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java 
line 286:

> 284: lineKind = (ch == '\n' || ch == '\r') ? 
> LineKind.BLANK
> 285: : (indent <= 3) ? peekLineKind()
> 286: : lineKind != LineKind.OTHER ? 
> LineKind.INDENTED_CODE_BLOCK

Doesn't this cause false positives for indented code blocks? In my 
understanding, indented lines in a list context and directly following a 
blockquote are not interpreted as code blocks, but as part of the list or 
blockquote. So my guess would be that `not OTHER` should be something like 
`BLANK or INDENTED_CODE_BLOCK or HEADER`, and that still leaves the problem of 
a [blank line in a list context](https://spec.commonmark.org/0.30/#example-108).

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1492065799


Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v36]

2024-02-15 Thread Jonathan Gibbons
> Please review a patch to add support for Markdown syntax in documentation 
> comments, as described in the associated JEP.
> 
> Notable features:
> 
> * support for `///` documentation comments in `JavaTokenizer`
> * new module `jdk.internal.md` -- a private copy of the `commonmark-java` 
> library
> * updates to `DocCommentParser` to treat `///` comments as Markdown
> * updates to the standard doclet to render Markdown comments in HTML

Jonathan Gibbons has updated the pull request incrementally with two additional 
commits since the last revision:

 - Support for Table Of Contents in Markdown headings
 - fix typo

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/16388/files
  - new: https://git.openjdk.org/jdk/pull/16388/files/da8752c8..53afd804

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=16388&range=35
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16388&range=34-35

  Stats: 102 lines in 2 files changed: 99 ins; 1 del; 2 mod
  Patch: https://git.openjdk.org/jdk/pull/16388.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388

PR: https://git.openjdk.org/jdk/pull/16388


Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v35]

2024-02-15 Thread Jonathan Gibbons
> Please review a patch to add support for Markdown syntax in documentation 
> comments, as described in the associated JEP.
> 
> Notable features:
> 
> * support for `///` documentation comments in `JavaTokenizer`
> * new module `jdk.internal.md` -- a private copy of the `commonmark-java` 
> library
> * updates to `DocCommentParser` to treat `///` comments as Markdown
> * updates to the standard doclet to render Markdown comments in HTML

Jonathan Gibbons has updated the pull request incrementally with three 
additional commits since the last revision:

 - fix handling of `@' in a Markdown comment
 - improve comments for some LineKind members
 - update LineKind members and test

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/16388/files
  - new: https://git.openjdk.org/jdk/pull/16388/files/393d3a9c..da8752c8

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=16388&range=34
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16388&range=33-34

  Stats: 55 lines in 3 files changed: 49 ins; 0 del; 6 mod
  Patch: https://git.openjdk.org/jdk/pull/16388.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388

PR: https://git.openjdk.org/jdk/pull/16388


Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v32]

2024-02-15 Thread Pavel Rappo
On Thu, 15 Feb 2024 19:39:07 GMT, Pavel Rappo  wrote:

>> whitespace is handled separately, on line 280 (`readIndent`) and285 (`: 
>> (indent <= 3) ? peekLineKind()`)
>
> Correct, but I believe the ordered list marker should be like this:
> 
> ORDERED_LIST_ITEM(Pattern.compile("[0-9]{1,9}[.)] .*"))
>  ^
> 
> Note extra whitespace character. I think we should really add this to our 
> tests, since lists are foundational Markdown structures, probably right after 
> italics and bold.

Then we should add `[ \t]` to both constants, right:

BULLETED_LIST_ITEM(Pattern.compile("[-+*][ \t].*"))
ORDERED_LIST_ITEM(Pattern.compile("[0-9]{1,9}[.)][ \t].*"))

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1491564839


Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v32]

2024-02-15 Thread Jonathan Gibbons
On Thu, 15 Feb 2024 19:39:07 GMT, Pavel Rappo  wrote:

>> whitespace is handled separately, on line 280 (`readIndent`) and285 (`: 
>> (indent <= 3) ? peekLineKind()`)
>
> Correct, but I believe the ordered list marker should be like this:
> 
> ORDERED_LIST_ITEM(Pattern.compile("[0-9]{1,9}[.)] .*"))
>  ^
> 
> Note extra whitespace character. I think we should really add this to our 
> tests, since lists are foundational Markdown structures, probably right after 
> italics and bold.

My recollection is that the space is required.   I will double check.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1491552312


Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v32]

2024-02-15 Thread Pavel Rappo
On Thu, 15 Feb 2024 19:20:23 GMT, Jonathan Gibbons  wrote:

>> src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java
>>  line 1401:
>> 
>>> 1399:  */
>>> 1400: enum LineKind {
>>> 1401: BLANK(Pattern.compile("[ \t]*")),
>> 
>> `BLANK` is a pseudo kind, because it is set manually, but never returned 
>> from `peekLine()`. I wonder if we can change it somehow.
>
> Even if it is set manually, it is still appropriate to have it as a member in 
> the `LineKind` enum class.

Not that it invalidates your reply; clarification: I meant `peekLineKind()`, 
not `peekLine()`.

>> src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java
>>  line 1433:
>> 
>>> 1431:  * @see >> href="https://spec.commonmark.org/0.30/#list-items";>List items
>>> 1432:  */
>>> 1433: BULLETED_LIST_ITEM(Pattern.compile("[-+*] .*")),
>> 
>> This comment is about `BULLETED_LIST_ITEM` and `ORDERED_LIST_ITEM` 
>> constants. I know that we don't need to be very precise, but perhaps in this 
>> case we should. While the CommonMark spec is a vague on that, from my 
>> experiments with [dingus](https://spec.commonmark.org/dingus/), it appears 
>> that a list marker can be preceded and followed by some number of whitespace 
>> characters.
>
> whitespace is handled separately, on line 280 (`readIndent`) and285 (`: 
> (indent <= 3) ? peekLineKind()`)

Correct, but I believe the ordered list marker should be like this:

ORDERED_LIST_ITEM(Pattern.compile("[0-9]{1,9}[.)] .*"))
 ^

Note extra whitespace character. I think we should really add this to our 
tests, since lists are foundational Markdown structures, probably right after 
italics and bold.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1491550784
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1491545609


Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v31]

2024-02-15 Thread Jonathan Gibbons
On Thu, 15 Feb 2024 19:36:36 GMT, Jonathan Gibbons  wrote:

>> 1.  Since forever, and still true, the way to specify a doclet is by its 
>> name, and the tool will create the instance for you.  This goes back to the 
>> original old days before any API, when the only entry point was the command 
>> line.
>> This implies that (a) the doclet has a no-args constructor and (b) that all 
>> doclet config is done through command line options.  A better solution would 
>> be to add new functionality to the various javadoc tool API (some or all of 
>> `Main`/`Start`/`DocumentationTool`) so that a client can initialize an 
>> instance of a doclet and pass that instance into the API.
>> 
>> 2. If I understand the question correctly, the code is invoked by using the 
>> command-line option `-XDaccessInternalAPI` which is a preexisting hidden 
>> feature already supported by `javac`.  It is used in `TestTransformer.java` 
>> on line 161.
>> 
>> javadoc("-d", base.resolve("api").toString(),
>> "-Xdoclint:none",
>> "-XDaccessInternalAPI", // required to access JavacTrees
>> "-doclet", "TestTransformer$MyDoclet",
>> "-docletpath", System.getProperty("test.classes"),
>> "-sourcepath", src.toString(),
>> "p");
>
> I confirm that `TestTransformer.java` fails as expected with an 
> `IllegalAccessError` if the option is not used.
> 
> java.lang.IllegalAccessError: class TestTransformer$MyDoclet (in unnamed 
> module @0x355de863) cannot access class com.sun.tools.javac.api.JavacTrees 
> (in module jdk.compiler) because module jdk.compiler does not export 
> com.sun.tools.javac.api to unnamed module @0x355de863
> at TestTransformer$MyDoclet.run(TestTransformer.java:139)
> at 
> jdk.javadoc/jdk.javadoc.internal.tool.Start.parseAndExecute(Start.java:589)

Generally, the error occurs because the `MyDoclet` class is run in a different 
class loader than that used for the test.  The class loader for the test 
already has the necessary access permissions given, from the lines in 
`@modules` in the `jtreg` test description.  Ideally, we could call `new 
MyDoclet()` in the main test code, and pas the instance in to the `javadoc` 
call and from there to the javadoc `Start` method.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1491547571


Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v31]

2024-02-15 Thread Jonathan Gibbons
On Thu, 15 Feb 2024 19:15:25 GMT, Jonathan Gibbons  wrote:

>> src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/Start.java line 571:
>> 
>>> 569: // of a doclet to be specified instead of the name of the
>>> 570: // doclet class and optional doclet path.
>>> 571: // See https://bugs.openjdk.org/browse/JDK-8263219
>> 
>> It's hard to understand this:
>> 
>>> to permit an instance of an appropriately configured instance of a doclet
>> 
>> Also: how is that piece of code used? When I commented it out, no 
>> test/langtools:langtools_javadoc tests failed.
>
> 1.  Since forever, and still true, the way to specify a doclet is by its 
> name, and the tool will create the instance for you.  This goes back to the 
> original old days before any API, when the only entry point was the command 
> line.
> This implies that (a) the doclet has a no-args constructor and (b) that all 
> doclet config is done through command line options.  A better solution would 
> be to add new functionality to the various javadoc tool API (some or all of 
> `Main`/`Start`/`DocumentationTool`) so that a client can initialize an 
> instance of a doclet and pass that instance into the API.
> 
> 2. If I understand the question correctly, the code is invoked by using the 
> command-line option `-XDaccessInternalAPI` which is a preexisting hidden 
> feature already supported by `javac`.  It is used in `TestTransformer.java` 
> on line 161.
> 
> javadoc("-d", base.resolve("api").toString(),
> "-Xdoclint:none",
> "-XDaccessInternalAPI", // required to access JavacTrees
> "-doclet", "TestTransformer$MyDoclet",
> "-docletpath", System.getProperty("test.classes"),
> "-sourcepath", src.toString(),
> "p");

I confirm that `TestTransformer.java` fails as expected with an 
`IllegalAccessError` if the option is not used.

java.lang.IllegalAccessError: class TestTransformer$MyDoclet (in unnamed module 
@0x355de863) cannot access class com.sun.tools.javac.api.JavacTrees (in module 
jdk.compiler) because module jdk.compiler does not export 
com.sun.tools.javac.api to unnamed module @0x355de863
at TestTransformer$MyDoclet.run(TestTransformer.java:139)
at 
jdk.javadoc/jdk.javadoc.internal.tool.Start.parseAndExecute(Start.java:589)

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1491538120


Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v32]

2024-02-15 Thread Jonathan Gibbons
On Thu, 15 Feb 2024 19:27:12 GMT, Jonathan Gibbons  wrote:

>> src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java
>>  line 422:
>> 
>>> 420: defaultContentCharacter();
>>> 421: }
>>> 422: }
>> 
>> Is it to pass `` through to Markdown, to allow it to deal with Markdown 
>> escapes?
>
> It is more about supporting the sequence `` ` `` so that the backtick is 
> treated as a literal character and not the beginning of a code span.   This 
> is the "backstop" preferred way to ensure that a single backtick is treated 
> literally, without relying on detected that it is unbalanced when the end of 
> the current block is reached.  The alternative workaround would be a single 
> backtick enclosed in multiple backticks, such as this ``` `` ` `` ```. (I 
> leave you to figure out what I actually typed there!!!)

You might also need to use `` ` `` when there are two literal backticks in a 
sentence.

After the first backtick (`), another backtick (`) can be used to delimit a 
code span.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1491528153


Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v34]

2024-02-15 Thread Jonathan Gibbons
> Please review a patch to add support for Markdown syntax in documentation 
> comments, as described in the associated JEP.
> 
> Notable features:
> 
> * support for `///` documentation comments in `JavaTokenizer`
> * new module `jdk.internal.md` -- a private copy of the `commonmark-java` 
> library
> * updates to `DocCommentParser` to treat `///` comments as Markdown
> * updates to the standard doclet to render Markdown comments in HTML

Jonathan Gibbons has updated the pull request incrementally with two additional 
commits since the last revision:

 - fix return tag name
 - remove debug print

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/16388/files
  - new: https://git.openjdk.org/jdk/pull/16388/files/f6d08db9..393d3a9c

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=16388&range=33
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16388&range=32-33

  Stats: 2 lines in 2 files changed: 0 ins; 1 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/16388.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388

PR: https://git.openjdk.org/jdk/pull/16388


Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v32]

2024-02-15 Thread Jonathan Gibbons
On Thu, 15 Feb 2024 17:17:46 GMT, Pavel Rappo  wrote:

>> Jonathan Gibbons has updated the pull request with a new target base due to 
>> a merge or a rebase. The pull request now contains 44 commits:
>> 
>>  - fill in `visitRawText` in `CommentHelper.getTags` visitor
>>  - fixes for the "New API" page
>>  - change "standard" to "traditional" when referring to a comment
>>  - Merge remote-tracking branch 'upstream/master' into 
>> 8298405.doclet-markdown-v3
>>  - Merge remote-tracking branch 'upstream/master' into 
>> 8298405.doclet-markdown-v3
>>  - improve support for DocCommentParser.LineKind
>>  - Merge remote-tracking branch 'upstream/master' into 
>> 8298405.doclet-markdown-v3 # Please enter a commit message to explain why 
>> this merge is necessary, # especially if it merges an updated upstream into 
>> a topic branch. # # Lines starting with '#' will be ignored, and an empty 
>> message aborts # the
>>commit.
>>  - update copyright year on test
>>  - refactor recent new test case in TestMarkdown.java
>>  - address review feedback
>>  - ... and 34 more: https://git.openjdk.org/jdk/compare/8765b176...2801c2e1
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java
>  line 422:
> 
>> 420: defaultContentCharacter();
>> 421: }
>> 422: }
> 
> Is it to pass `` through to Markdown, to allow it to deal with Markdown 
> escapes?

It is more about supporting the sequence `` ` `` so that the backtick is 
treated as a literal character and not the beginning of a code span.   This is 
the "backstop" preferred way to ensure that a single backtick is treated 
literally, without relying on detected that it is unbalanced when the end of 
the current block is reached.  The alternative workaround would be a single 
backtick enclosed in multiple backticks, such as this ``` `` ` `` ```. (I leave 
you to figure out what I actually typed there!!!)

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1491519707


Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v32]

2024-02-15 Thread Jonathan Gibbons
On Thu, 15 Feb 2024 17:03:09 GMT, Pavel Rappo  wrote:

>> Jonathan Gibbons has updated the pull request with a new target base due to 
>> a merge or a rebase. The pull request now contains 44 commits:
>> 
>>  - fill in `visitRawText` in `CommentHelper.getTags` visitor
>>  - fixes for the "New API" page
>>  - change "standard" to "traditional" when referring to a comment
>>  - Merge remote-tracking branch 'upstream/master' into 
>> 8298405.doclet-markdown-v3
>>  - Merge remote-tracking branch 'upstream/master' into 
>> 8298405.doclet-markdown-v3
>>  - improve support for DocCommentParser.LineKind
>>  - Merge remote-tracking branch 'upstream/master' into 
>> 8298405.doclet-markdown-v3 # Please enter a commit message to explain why 
>> this merge is necessary, # especially if it merges an updated upstream into 
>> a topic branch. # # Lines starting with '#' will be ignored, and an empty 
>> message aborts # the
>>commit.
>>  - update copyright year on test
>>  - refactor recent new test case in TestMarkdown.java
>>  - address review feedback
>>  - ... and 34 more: https://git.openjdk.org/jdk/compare/8765b176...2801c2e1
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java
>  line 1401:
> 
>> 1399:  */
>> 1400: enum LineKind {
>> 1401: BLANK(Pattern.compile("[ \t]*")),
> 
> `BLANK` is a pseudo kind, because it is set manually, but never returned from 
> `peekLine()`. I wonder if we can change it somehow.

Even if it is set manually, it is still appropriate to have it as a member in 
the `LineKind` enum class.

> src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java
>  line 1433:
> 
>> 1431:  * @see > href="https://spec.commonmark.org/0.30/#list-items";>List items
>> 1432:  */
>> 1433: BULLETED_LIST_ITEM(Pattern.compile("[-+*] .*")),
> 
> This comment is about `BULLETED_LIST_ITEM` and `ORDERED_LIST_ITEM` constants. 
> I know that we don't need to be very precise, but perhaps in this case we 
> should. While the CommonMark spec is a vague on that, from my experiments 
> with [dingus](https://spec.commonmark.org/dingus/), it appears that a list 
> marker can be preceded and followed by some number of whitespace characters.

whitespace is handled separately, on line 280 (`readIndent`) and285 (`: (indent 
<= 3) ? peekLineKind()`)

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1491508303
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1491512611


Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v31]

2024-02-15 Thread Jonathan Gibbons
On Tue, 13 Feb 2024 11:15:55 GMT, Pavel Rappo  wrote:

>> Jonathan Gibbons has updated the pull request with a new target base due to 
>> a merge or a rebase. The pull request now contains 40 commits:
>> 
>>  - Merge remote-tracking branch 'upstream/master' into 
>> 8298405.doclet-markdown-v3
>>  - improve support for DocCommentParser.LineKind
>>  - Merge remote-tracking branch 'upstream/master' into 
>> 8298405.doclet-markdown-v3 # Please enter a commit message to explain why 
>> this merge is necessary, # especially if it merges an updated upstream into 
>> a topic branch. # # Lines starting with '#' will be ignored, and an empty 
>> message aborts # the
>>commit.
>>  - update copyright year on test
>>  - refactor recent new test case in TestMarkdown.java
>>  - address review feedback
>>  - address review feedback
>>  - added test case in TestMarkdown.java for handling of `@deprecated` tag
>>  - amend comment in test
>>  - improve comments on negative test case
>>  - ... and 30 more: https://git.openjdk.org/jdk/compare/2ed889b7...1c64a6e0
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/Checker.java line 
> 1021:
> 
>> 1019: .findFirst();
>> 1020: if (first.isEmpty() || first.get() != tree) {
>> 1021: dct.getFirstSentence().forEach(t -> 
>> System.err.println(t.getKind() + ": >>|" + t + "|<<"));
> 
> Is it leftover debug output?

Oops, yes.  Thanks.

> src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/Start.java line 571:
> 
>> 569: // of a doclet to be specified instead of the name of the
>> 570: // doclet class and optional doclet path.
>> 571: // See https://bugs.openjdk.org/browse/JDK-8263219
> 
> It's hard to understand this:
> 
>> to permit an instance of an appropriately configured instance of a doclet
> 
> Also: how is that piece of code used? When I commented it out, no 
> test/langtools:langtools_javadoc tests failed.

1.  Since forever, and still true, the way to specify a doclet is by its name, 
and the tool will create the instance for you.  This goes back to the original 
old days before any API, when the only entry point was the command line.
This implies that (a) the doclet has a no-args constructor and (b) that all 
doclet config is done through command line options.  A better solution would be 
to add new functionality to the various javadoc tool API (some or all of 
`Main`/`Start`/`DocumentationTool`) so that a client can initialize an instance 
of a doclet and pass that instance into the API.

2. If I understand the question correctly, the code is invoked by using the 
command-line option `-XDaccessInternalAPI` which is a preexisting hidden 
feature already supported by `javac`.  It is used in `TestTransformer.java` on 
line 161.

javadoc("-d", base.resolve("api").toString(),
"-Xdoclint:none",
"-XDaccessInternalAPI", // required to access JavacTrees
"-doclet", "TestTransformer$MyDoclet",
"-docletpath", System.getProperty("test.classes"),
"-sourcepath", src.toString(),
"p");

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1491504223
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1491502389


Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v33]

2024-02-15 Thread Jonathan Gibbons
> Please review a patch to add support for Markdown syntax in documentation 
> comments, as described in the associated JEP.
> 
> Notable features:
> 
> * support for `///` documentation comments in `JavaTokenizer`
> * new module `jdk.internal.md` -- a private copy of the `commonmark-java` 
> library
> * updates to `DocCommentParser` to treat `///` comments as Markdown
> * updates to the standard doclet to render Markdown comments in HTML

Jonathan Gibbons has updated the pull request incrementally with one additional 
commit since the last revision:

  fix compilation and whitespace issues

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/16388/files
  - new: https://git.openjdk.org/jdk/pull/16388/files/2801c2e1..f6d08db9

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=16388&range=32
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16388&range=31-32

  Stats: 4 lines in 2 files changed: 0 ins; 0 del; 4 mod
  Patch: https://git.openjdk.org/jdk/pull/16388.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388

PR: https://git.openjdk.org/jdk/pull/16388


Re: Linking to anchors in the same class

2024-02-15 Thread Jonathan Gibbons

Nir,

Thank you for your question.  It may be that there is a bug, or at 
least, a lack of tests, in this area.


In general, you need to give text for the link, since (unlike for 
program elements) javadoc cannot infer the string for itself. Iit 
doesn't go looking for headings and ids.


You might try  `@see Chronus##time-heading my-text` or failing that `href="Chronus.html##time-heading">my-text


For our part, we should make sure that we have tests for this situation 
and/or clarify the spec if that is needed.


-- Jon

On 2/15/24 9:22 AM, Nir Lisker wrote:

Hi,

I have a question after reading 
https://bugs.openjdk.org/browse/JDK-8294195. If I have a class 
Chronus.java with a heading in its class javadoc:


Time

then an anchor Chronus.html#time-heading is auto-generated. How do I 
link to this anchor from within the class using @see and @link? I have 
tried

@see ##time-heading
@see Chronus.html##time-heading
@see Chronus##Chronus.html#time-heading

None of which work. What is the correct syntax?

Thanks,
Nir


Linking to anchors in the same class

2024-02-15 Thread Nir Lisker
Hi,

I have a question after reading https://bugs.openjdk.org/browse/JDK-8294195.
If I have a class Chronus.java with a heading in its class javadoc:

Time

then an anchor Chronus.html#time-heading is auto-generated. How do I link
to this anchor from within the class using @see and @link? I have tried
@see ##time-heading
@see Chronus.html##time-heading
@see Chronus##Chronus.html#time-heading

None of which work. What is the correct syntax?

Thanks,
Nir


Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v32]

2024-02-15 Thread Pavel Rappo
On Thu, 15 Feb 2024 00:30:25 GMT, Jonathan Gibbons  wrote:

>> Please review a patch to add support for Markdown syntax in documentation 
>> comments, as described in the associated JEP.
>> 
>> Notable features:
>> 
>> * support for `///` documentation comments in `JavaTokenizer`
>> * new module `jdk.internal.md` -- a private copy of the `commonmark-java` 
>> library
>> * updates to `DocCommentParser` to treat `///` comments as Markdown
>> * updates to the standard doclet to render Markdown comments in HTML
>
> Jonathan Gibbons has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains 44 commits:
> 
>  - fill in `visitRawText` in `CommentHelper.getTags` visitor
>  - fixes for the "New API" page
>  - change "standard" to "traditional" when referring to a comment
>  - Merge remote-tracking branch 'upstream/master' into 
> 8298405.doclet-markdown-v3
>  - Merge remote-tracking branch 'upstream/master' into 
> 8298405.doclet-markdown-v3
>  - improve support for DocCommentParser.LineKind
>  - Merge remote-tracking branch 'upstream/master' into 
> 8298405.doclet-markdown-v3 # Please enter a commit message to explain why 
> this merge is necessary, # especially if it merges an updated upstream into a 
> topic branch. # # Lines starting with '#' will be ignored, and an empty 
> message aborts # the
>commit.
>  - update copyright year on test
>  - refactor recent new test case in TestMarkdown.java
>  - address review feedback
>  - ... and 34 more: https://git.openjdk.org/jdk/compare/8765b176...2801c2e1

I'm again looking `LineKind`. This time because of 9eaf84e5dd6.

src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java 
line 422:

> 420: defaultContentCharacter();
> 421: }
> 422: }

Is it to pass `` through to Markdown, to allow it to deal with Markdown escapes?

src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java 
line 1401:

> 1399:  */
> 1400: enum LineKind {
> 1401: BLANK(Pattern.compile("[ \t]*")),

`BLANK` is a pseudo kind, because it is set manually, but never returned from 
`peekLine()`. I wonder if we can change it somehow.

src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java 
line 1433:

> 1431:  * @see  href="https://spec.commonmark.org/0.30/#list-items";>List items
> 1432:  */
> 1433: BULLETED_LIST_ITEM(Pattern.compile("[-+*] .*")),

This comment is about `BULLETED_LIST_ITEM` and `ORDERED_LIST_ITEM` constants. I 
know that we don't need to be very precise, but perhaps in this case we should. 
While the CommonMark spec is a vague on that, from my experiments with 
[dingus](https://spec.commonmark.org/dingus/), it appears that a list marker 
can be preceded and followed by some number of whitespace characters.

-

PR Review: https://git.openjdk.org/jdk/pull/16388#pullrequestreview-1883374712
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1491362821
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1491344667
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1491354450