On Wed, 24 Jan 2024 22:38:58 GMT, Pavel Rappo <pra...@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
