On Fri, 19 Jan 2024 18:37:48 GMT, Jonathan Gibbons <j...@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
