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

Pavel Rappo prappo at openjdk.org
Fri Jan 26 17:39:43 UTC 2024


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.internal.md/share/classes/jdk/internal/markdown/MarkdownTransformer.java line 668:

> 666:          * U+FFFC characters that were found in the original document.
> 667:          */
> 668:         Iterator<?> replaceIter;

Suggestion:

        final Iterator<?> replaceIter;

src/jdk.internal.md/share/classes/jdk/internal/markdown/MarkdownTransformer.java line 732:

> 730:                 offsets.add(m.end());
> 731:             }
> 732:             sourceLineOffsets = offsets.stream().mapToInt(Integer::intValue).toArray();

Here's an alternative, which you might find better (or not):
Suggestion:

            sourceLineOffsets = Stream.concat(Stream.of(0), lineBreak.matcher(source).results()
                            .map(MatchResult::end)).mapToInt(Integer::intValue).toArray();

src/jdk.internal.md/share/classes/jdk/internal/markdown/MarkdownTransformer.java line 763:

> 761:          *
> 762:          * @param link the link node
> 763:          */

Suggestion:

        /**
         * Visits a {@code Link} commonmark node.
         *
         * If the destination for the link begins with {@code code:}
         * convert it to {@code {@link ...}} or {@code {@linkplain ...}} doc tree node.
         * {@code {@link ...}} will be used if the content (label) for
         * the link is the same as the reference found after the {@code code:};
         * otherwise, {@code {@linkplain ...}} will be used.
         *
         * The label will be left blank for {@code {@link ...}} nodes,
         * implying that a default label should be used, based on the
         * program element that was referenced.
         *
         * @param link the link node
         */

src/jdk.internal.md/share/classes/jdk/internal/markdown/MarkdownTransformer.java line 778:

> 776:                     copyTo(getEndPos(link.getLastChild()));
> 777: 
> 778:                     // determine whether to use {@link... } or {@linkplain ...}

Nit:
Suggestion:

                    // determine whether to use {@link ... } or {@linkplain ...}

src/jdk.internal.md/share/classes/jdk/internal/markdown/MarkdownTransformer.java line 831:

> 829:         /**
> 830:          * {@return the position in the original comment for a position in {@code source},
> 831:          * using {@link #replaceAdjustPos}}.

Suggestion:

         * using {@link #replaceAdjustPos}}

src/jdk.internal.md/share/classes/jdk/internal/markdown/MarkdownTransformer.java line 840:

> 838: 
> 839:         /**
> 840:          * Process a node and any children.

Suggestion:

         * Processes a node and any children.

src/jdk.internal.md/share/classes/jdk/internal/markdown/MarkdownTransformer.java line 939:

> 937: 
> 938:         /**
> 939:          * Flush any text in the {@code text} buffer, by creating a new

Suggestion:

         * Flushes any text in the {@code text} buffer, by creating a new

src/jdk.internal.md/share/classes/jdk/internal/markdown/MarkdownTransformer.java line 950:

> 948:     }
> 949: 
> 950: 

Suggestion:

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1467870392
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1467870182
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1467643549
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1467871256
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1467876796
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1467871714
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1467872096
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1467872597


More information about the build-dev mailing list