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

[Jonathan Gibbons]

On Wed, 24 Jan 2024 22:38:58 GMT, Pavel Rappo <prappo at openjdk.org> 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 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 487:
> 
>> 485:     }
>> 486: 
>> 487:     private static final String AUTOREF_PREFIX = "code:";
> 
> I wish the prefix were such that it could not be forged: for example, automatically assigned, unique within a document comment.

I guess I don't see this as being as big a deal as you seem to think it is. What is it that you are so concerned about?

I also think it is a potential feature to document and use reference links with `code:` URLs, using the reference link syntax to avoid having long method signatures in narrative text.

For example, 

   One of the methods on [List] has [lots of args][List.of10].
   
   [List.of10]code:List.of(E,E,E,E,E,E,E,E,E,E)

> 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
>          */

fixed

> src/jdk.internal.md/share/classes/jdk/internal/markdown/MarkdownTransformer.java line 771:
> 
>> 769:                 copyTo(getStartPos(link));
>> 770:                 // push temporary value for {@code trees} while handling the content of the node
>> 771:                 var saveTrees = trees;
> 
> "saveTrees": I see what you did there :-)

?? Not sure I understand this comment.

> 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 ...}

fixed

> 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.

fixed

> src/jdk.internal.md/share/classes/jdk/internal/markdown/MarkdownTransformer.java line 882:
> 
>> 880:         private int getStartPos(Node node) {
>> 881:             var spans = node.getSourceSpans();
>> 882:             var firstSpan = spans.get(0);
> 
> Suggestion:
> 
>             var firstSpan = spans.getFirst();

done

> src/jdk.internal.md/share/classes/jdk/internal/markdown/MarkdownTransformer.java line 894:
> 
>> 892:         private int getEndPos(Node node) {
>> 893:             var spans = node.getSourceSpans();
>> 894:             var lastSpan = spans.get(spans.size() - 1);
> 
> Suggestion:
> 
>             var lastSpan = spans.getLast();

Done

> src/jdk.internal.md/share/classes/jdk/internal/markdown/MarkdownTransformer.java line 950:
> 
>> 948:     }
>> 949: 
>> 950: 
> 
> Suggestion:

fixed

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1472122002
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1472129024
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1472125185
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1472129294
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1472129971
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1472123360
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1472124337
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1472130370