RFR: 8325168: JShell should support Markdown comments [v5]

[Jonathan Gibbons]

On Fri, 31 May 2024 06:28:28 GMT, Jan Lahoda <jlahoda at openjdk.org> wrote:

>> This is an attempt to add Markdown support in documentation comments to JShell.
>> 
>> It works by converting the Markdown text to HTML during the process of resolving `{@inheritDoc}` tags.
>
> Jan Lahoda has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 119 commits:
> 
>  - Merge branch 'master' into JDK-8298405
>  - Merge remote-tracking branch 'origin/JDK-8332858' into JDK-8298405
>  - Fixing prefix offset.
>  - Fixing handling of synthetic/rewritten links.
>  - Merge remote-tracking branch 'origin/JDK-8332858' into JDK-8298405
>  - 8332858: References with escapes have broken positions after they are transformed
>  - Fixing test.
>  - Merge branch 'master' into JDK-8298405
>  - Merge branch '8298405.doclet-markdown-v3' into JDK-8298405
>  - address review feedback
>  - ... and 109 more: https://git.openjdk.org/jdk/compare/2ab8ab56...d85a2b8a

I'll approve it for what it is.

There are some minor comments about imports and tests; these can either be fixed here in this PR, or in cleanup tasks before RDP2.

It is disappointing but not surprising to see duplication of somewhat similar code in the `jdk.javadoc` module, especially the handling of Markdown code. This just adds to the duplication of effort interpreting other aspects of doc comments. I understand how we got to this point, but down the road, we should look to share more code, translating doc comments into some intermediate representation (such as the HTML classes currently in  `jdk.javadoc.internal.doclets.formats.html.markup`) such that we only need minimal subsequent conversion to a near-text form., like ANSI-styled text. That would be a good layer of abstraction to have in the `jdk.javadoc` module anyway, and if we can expose it (internally?) to other JDK modules, to support their use and display of doc comments, so much the better: we all win.

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

> 36: import com.sun.source.doctree.DocTree;
> 37: import com.sun.source.doctree.DocTreeVisitor;
> 38: import com.sun.source.doctree.EscapeTree;

unused import

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

> 63: import static com.sun.tools.javac.util.Position.NOPOS;
> 64: import java.util.regex.Matcher;
> 65: import jdk.internal.org.commonmark.internal.util.Escaping;

unused (and out of order) imports

src/jdk.jshell/share/classes/jdk/internal/shellsupport/doc/JavadocHelper.java line 103:

> 101: import jdk.internal.org.commonmark.parser.IncludeSourceSpans;
> 102: import jdk.internal.org.commonmark.parser.Parser;
> 103: import jdk.internal.org.commonmark.renderer.html.HtmlRenderer;

weirdly ordered imports

src/jdk.jshell/share/classes/jdk/internal/shellsupport/doc/JavadocHelper.java line 518:

> 516:                 public Void visitLink(LinkTree node, Void p) {
> 517:                     if (sp.isRewrittenTree(null, dcTree, node)) {
> 518:                         //this links is a synthetic rewritten link, replace

spelling `this link`

test/langtools/jdk/internal/shellsupport/doc/JavadocHelperTest.java line 346:

> 344:                       "@throws java.lang.IllegalAccessException exc3\n" +
> 345:                       "@return value\n" +
> 346:                       "@since snc");

Is there a good reason to not use text blocks before/after `getSubTest`
or a single text block with an old-fashioned `.replace` for a marker string?

Comment applies to all similar instances in the test. I won't bother to note them individually.

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

Marked as reviewed by jjg (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/17686#pullrequestreview-2091610752
PR Review Comment: https://git.openjdk.org/jdk/pull/17686#discussion_r1622890367
PR Review Comment: https://git.openjdk.org/jdk/pull/17686#discussion_r1622890430
PR Review Comment: https://git.openjdk.org/jdk/pull/17686#discussion_r1622895867
PR Review Comment: https://git.openjdk.org/jdk/pull/17686#discussion_r1622899268
PR Review Comment: https://git.openjdk.org/jdk/pull/17686#discussion_r1622893871