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/tree/DocTreeMaker.java line 238:

> 236:                         && postamble.isEmpty()
> 237:                         && fullBody.stream().anyMatch(t -> t.getKind() == Kind.MARKDOWN)
> 238:                         ? CommentStyle.JAVADOC_LINE : CommentStyle.JAVADOC_BLOCK;

While clever, it seems to be prone to false positive `JAVADOC_LINE`. Also, it is inconsistent with `null` and `Position.NOPOS` returned from the `getText` and `getSourcePos(int)` methods respectively.

src/jdk.compiler/share/classes/com/sun/tools/javac/tree/DocTreeMaker.java line 572:

> 570:                         }
> 571: 
> 572:                         case TEXT -> {

I haven't looked at `SentenceBreaker` in detail, but one thing that bothers me is that it sees a comment before that comment has been transformed. This means that `///` comments might not "feel" like Markdown to authors.

src/jdk.compiler/share/classes/com/sun/tools/javac/tree/DocTreeMaker.java line 704:

> 702:             }
> 703: 
> 704:             // If the break is well within the span of the string i.e. not

Oh irony! Sentence segmentation in javadoc has some problems with abbreviations like that.

src/jdk.compiler/share/classes/com/sun/tools/javac/tree/DocTreeMaker.java line 790:

> 788: 
> 789:         // end of paragraph is newline, followed by a blank line or the beginning of the next block
> 790:         private static final Pattern endPara = Pattern.compile("\n(([ \t]*\n)|( {0,3}[-+*#=]))");

So DocTreeMaker now also knows about Markdown. I wonder if we can avoid that. Also, I assume you mean this (`+` is not a part of "thematic break"):
Suggestion:

        private static final Pattern endPara = Pattern.compile("\n(([ \t]*\n)|( {0,3}[-_*#=]))");

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1464830924
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1465191542
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1465201174
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1465204965