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

[Pavel Rappo]

On Fri, 19 Jan 2024 18:37:48 GMT, Jonathan Gibbons <jjg at openjdk.org> 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 eight commits:
> 
>  - Merge with upstream/master
>  - Merge with upstream/master
>  - Merge remote-tracking branch 'upstream/master' into 8298405.doclet-markdown-v3
>  - Address review comments
>  - Fix whitespace
>  - Improve handling of embedded inline taglets
>  - Customize support for Markdown headings
>  - JDK-8298405: Support Markdown in Documentation Comments

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

> 223:         while (ch == ' ' && bp < buflen) {
> 224:             nextChar();
> 225:         }

Why do we specifically care about `\s` symbols here rather than about broader whitespace?

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

> 1163:     }
> 1164: 
> 1165: 

Please delete this blank line.

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

> 1313:     }
> 1314: 
> 1315:     void skipLine() {

Like `peekLike()`, this method also does not seem to be consistent with `newline` in `nextChar()`.

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

> 1380:          *  @see <a href="https://spec.commonmark.org/0.30/#setext-headings">Setext Headings</a>
> 1381:          */
> 1382:         SETEXT_UNDERLINE(Pattern.compile("[=-]+[ \t]*")),

This should be more accurate:
Suggestion:

        SETEXT_UNDERLINE(Pattern.compile("=+|-+[ \t]*")),

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

> 1386:          * @see <a href="https://spec.commonmark.org/0.30/#thematic-breaks">Thematic Break</a>
> 1387:          */
> 1388:         THEMATIC_BREAK(Pattern.compile("((\\+[ \t]*+){3,})|((-[ \t]*+){3,})|((_[ \t]*+){3,})")),

Suggestion:

        /**
         * Thematic break: a line of * - _ interspersed with optional spaces and tabs
         * @see <a href="https://spec.commonmark.org/0.30/#thematic-breaks">Thematic Break</a>
         */
        THEMATIC_BREAK(Pattern.compile("((\*[ \t]*+){3,})|((-[ \t]*+){3,})|((_[ \t]*+){3,})")),

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

> 1394:          * @see <a href="https://spec.commonmark.org/0.30/#code-fence">Code Fence</a>
> 1395:          */
> 1396:         CODE_FENCE(Pattern.compile("(`{3,}[^`]*+)|(~{3,}.*+)")),

Why are this and the previous patterns possessive (`+`), while others aren't?

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

> 1417:     LineKind peekLineKind() {
> 1418:         switch (ch) {
> 1419:             case '#', '=', '-', '+', '_', '`', '~' -> {

This change is to match that of `THEMATIC_BREAK`:
Suggestion:

            case '#', '=', '-', '*', '_', '`', '~' -> {

src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavaTokenizer.java line 1065:

> 1063: 
> 1064:                     if (accept('/')) { // (Spec. 3.7)
> 1065:                         if (accept('/')) { // Markdown comment

Here and elsewhere in this file: do we need to mention Markdown?

src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavaTokenizer.java line 1553:

> 1551: 
> 1552:         /**
> 1553:          * Determine how much indent to remove from markdown comment.

Suggestion:

         * Determine how much indent to remove from Markdown comment.

src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavaTokenizer.java line 1585:

> 1583:          */
> 1584:         UnicodeReader trimMarkdownComment(UnicodeReader line, int indent) {
> 1585:             int pos = line.position();

Unused.

src/jdk.compiler/share/classes/com/sun/tools/javac/parser/ParserFactory.java line 30:

> 28: import java.util.Locale;
> 29: 
> 30: import com.sun.source.util.DocTrees;

Unused.

src/jdk.compiler/share/classes/com/sun/tools/javac/tree/DocPretty.java line 35:

> 33: import com.sun.source.doctree.*;
> 34: import com.sun.source.doctree.AttributeTree.ValueKind;
> 35: import com.sun.source.util.DocTreeScanner;

Unused.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1463169245
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1463155900
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1463252506
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1463181845
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1463202143
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1463237327
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1463239667
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1463422663
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1463422992
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1463424623
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1463437420
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1463441675