Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v45]
> 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 63 commits: - fix test after merge - Merge remote-tracking branch 'upstream/master' into 8298405.doclet-markdown-v3 - Revise `Markdown.update` to better handle container blocks. - Refactor most of TestMarkdown.java into separate tests, grouped by functionality - add support for (top-level) trailing code blocks add simple test case with tabs add TestMarkdownCodeBlocks.testTypical - fix whitespace - fix Markdown.update for new POST_LIST_INDENT test case given in review feedback - refactor tests for Markdown headings into a separate test class. - update DocCommentParser and tests to improve handling of code blocks and code spans in Markdown documentation comments - fix indentation, for consistency - ... and 53 more: https://git.openjdk.org/jdk/compare/6f8d351e...292ff0f1 - Changes: https://git.openjdk.org/jdk/pull/16388/files Webrev: https://webrevs.openjdk.org/?repo=jdk=16388=44 Stats: 23548 lines in 205 files changed: 22863 ins; 252 del; 433 mod Patch: https://git.openjdk.org/jdk/pull/16388.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388 PR: https://git.openjdk.org/jdk/pull/16388
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v44]
On Mon, 4 Mar 2024 14:40:06 GMT, Pavel Rappo wrote:
> I have a test case to report. The following results in no `@param`
> information being rendered, which I think is a bug:
>
> ```
> /// Hello, _Markdown_ world!
> ///
> ///
> /// @param hello
> ///
> ///
> public class C { }
> ```
Scratch that. After experimenting a bit more with **traditional** javadoc
comments, I realised that it might be expected that `@param` would be lost in
that `...` block; okay.
Still, I find it surprising that the description of the `@param` tag in the
above comment, hosted in `///` or `/**...*/`, is a single node of type
`RawText` with the following content:
hello
(Note, the content includes ``.)
-
PR Comment: https://git.openjdk.org/jdk/pull/16388#issuecomment-1976818803
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v44]
On Fri, 1 Mar 2024 21:49:29 GMT, Jonathan Gibbons 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 incrementally with one
> additional commit since the last revision:
>
> Revise `Markdown.update` to better handle container blocks.
I have a test case to report. The following results in no `@param` information
being rendered, which I think is a bug:
/// Hello, _Markdown_ world!
///
///
/// @param hello
///
///
public class C { }
-
PR Comment: https://git.openjdk.org/jdk/pull/16388#issuecomment-1976734010
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v44]
> 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 incrementally with one additional commit since the last revision: Revise `Markdown.update` to better handle container blocks. - Changes: - all: https://git.openjdk.org/jdk/pull/16388/files - new: https://git.openjdk.org/jdk/pull/16388/files/398f93fc..0b4d9b3c Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16388=43 - incr: https://webrevs.openjdk.org/?repo=jdk=16388=42-43 Stats: 348 lines in 3 files changed: 226 ins; 43 del; 79 mod Patch: https://git.openjdk.org/jdk/pull/16388.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388 PR: https://git.openjdk.org/jdk/pull/16388
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v43]
On Wed, 28 Feb 2024 17:12:04 GMT, Jonathan Gibbons wrote: >> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java >> line 1601: >> >>> 1599: : eKind != ElementKind.OTHER ? 1 // module, >>> package, class, interface >>> 1600: : 0; // doc file >>> 1601: return "h" + Math.min(heading.getLevel() + offset, 6); >> >> I really like that we adapt the heading level to the current context, but I >> notice that the code kind of expects `h1` headings to be used everywhere, >> and "punishes" use of contextually correct headings by generating smaller >> headings. Maybe it would be more educational to only adjust the level if it >> needs adjusting? > > Setext headings only come in "level 1" and "level 2" flavors. > And, at the time the renderer sees the AST, the difference between ATX and > setext headings is erased. They're just "headings". > > I also think it is better to have a simple rule than an "adaptive" rule. If > you start doing a more complex rule, you have to consider the effect on > subheadings as well. I suspected it was about the limited range of Setext headings. Yesterday my proposal was intentionally vague, but thinking about this again I think we should actually do the simplest and least intrusive thing possible: // minLevel is 4 for members, 2 for page-level elements, 1 for doc files "h" + Math.max(heading.getLevel(), minLevel); This arguably generates the correct heading for most simple use cases. What it doesn't do is to translate whole hierarchies of headings, but I would argue that few people people need this and those who do should figure out the rules and use the correct ATX headings. - PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1507439797
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v43]
On Wed, 28 Feb 2024 15:54:38 GMT, Hannes Wallnöfer wrote: >> Jonathan Gibbons has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Refactor most of TestMarkdown.java into separate tests, grouped by >> functionality > > src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java > line 1601: > >> 1599: : eKind != ElementKind.OTHER ? 1 // module, >> package, class, interface >> 1600: : 0; // doc file >> 1601: return "h" + Math.min(heading.getLevel() + offset, 6); > > I really like that we adapt the heading level to the current context, but I > notice that the code kind of expects `h1` headings to be used everywhere, and > "punishes" use of contextually correct headings by generating smaller > headings. Maybe it would be more educational to only adjust the level if it > needs adjusting? Setext headings only come in "level 1" and "level 2" flavors. And, at the time the renderer sees the AST, the difference between ATX and setext headings is erased. They're just "headings". - PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1506307115
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v43]
On Fri, 23 Feb 2024 22:27:43 GMT, Jonathan Gibbons 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 incrementally with one > additional commit since the last revision: > > Refactor most of TestMarkdown.java into separate tests, grouped by > functionality src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java line 1601: > 1599: : eKind != ElementKind.OTHER ? 1 // module, > package, class, interface > 1600: : 0; // doc file > 1601: return "h" + Math.min(heading.getLevel() + offset, 6); I really like that we adapt the heading level to the current context, but I notice that the code kind of expects `h1` headings to be used everywhere, and "punishes" use of contextually correct headings by generating smaller headings. Maybe it would be more educational to only adjust the level if it needs adjusting? - PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1506190847
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v43]
On Fri, 23 Feb 2024 22:27:43 GMT, Jonathan Gibbons 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 incrementally with one > additional commit since the last revision: > > Refactor most of TestMarkdown.java into separate tests, grouped by > functionality src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java line 1733: > 1731: return false; > 1732: } > 1733: The two methods below and some other methods in this class have too much indentation. - PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1506145051
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v43]
On Fri, 23 Feb 2024 22:27:43 GMT, Jonathan Gibbons 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 incrementally with one > additional commit since the last revision: > > Refactor most of TestMarkdown.java into separate tests, grouped by > functionality src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java line 1380: > 1378: > 1379: var useMarkdown = trees.stream().anyMatch(t -> t.getKind() == > Kind.MARKDOWN); > 1380: var markdownHandler = useMarkdown ? new > MarkdownHandler(element) : null; The `MarkdownHandler` and `HeadingNodeRenderer` classes must become aware of the current `TagletWriter.Context`. That's because headings and other block tags should only be generated if `TagletWriter.Context.isFirstSentence` is `false`. - PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1506084275
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v43]
> 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 incrementally with one additional commit since the last revision: Refactor most of TestMarkdown.java into separate tests, grouped by functionality - Changes: - all: https://git.openjdk.org/jdk/pull/16388/files - new: https://git.openjdk.org/jdk/pull/16388/files/d460ee33..398f93fc Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16388=42 - incr: https://webrevs.openjdk.org/?repo=jdk=16388=41-42 Stats: 2001 lines in 9 files changed: 1143 ins; 857 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/16388.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388 PR: https://git.openjdk.org/jdk/pull/16388
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v42]
> 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 incrementally with one additional commit since the last revision: add support for (top-level) trailing code blocks add simple test case with tabs add TestMarkdownCodeBlocks.testTypical - Changes: - all: https://git.openjdk.org/jdk/pull/16388/files - new: https://git.openjdk.org/jdk/pull/16388/files/40616865..d460ee33 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16388=41 - incr: https://webrevs.openjdk.org/?repo=jdk=16388=40-41 Stats: 143 lines in 3 files changed: 135 ins; 2 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/16388.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388 PR: https://git.openjdk.org/jdk/pull/16388
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v41]
> 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 incrementally with one additional commit since the last revision: fix whitespace - Changes: - all: https://git.openjdk.org/jdk/pull/16388/files - new: https://git.openjdk.org/jdk/pull/16388/files/27bc0b9d..40616865 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16388=40 - incr: https://webrevs.openjdk.org/?repo=jdk=16388=39-40 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/16388.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388 PR: https://git.openjdk.org/jdk/pull/16388
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v38]
On Thu, 22 Feb 2024 19:17:09 GMT, Jonathan Gibbons wrote: >>>I think in this place we also need to check for indented code blocks, since >>>we reduced currIndent and may have 4 or more character indentation compared >>>to the new currIndent value. >> >> This seems wrong. Surely, we should not reduce the indentation and then for >> the same line in the comment check for an extra-indented line. Or else I am >> missing something. >> >> That being said, thanks for the test case. I will investigate it. > > Update: I see and understand your test case. Update: I found that more elegant solution you hinted at, by refactoring the method from nested `if` statements to a series of checks with early returns. The test case you provided now works as expected. - PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1499918026
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v40]
> 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 incrementally with one additional commit since the last revision: fix Markdown.update for new POST_LIST_INDENT test case given in review feedback - Changes: - all: https://git.openjdk.org/jdk/pull/16388/files - new: https://git.openjdk.org/jdk/pull/16388/files/72420419..27bc0b9d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16388=39 - incr: https://webrevs.openjdk.org/?repo=jdk=16388=38-39 Stats: 94 lines in 4 files changed: 53 ins; 16 del; 25 mod Patch: https://git.openjdk.org/jdk/pull/16388.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388 PR: https://git.openjdk.org/jdk/pull/16388
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v38]
On Thu, 22 Feb 2024 19:13:57 GMT, Jonathan Gibbons wrote: >> Thanks. I will investigate your hint to look for a more elegant solution. >> Thanks for the new test case. > >>I think in this place we also need to check for indented code blocks, since >>we reduced currIndent and may have 4 or more character indentation compared >>to the new currIndent value. > > This seems wrong. Surely, we should not reduce the indentation and then for > the same line in the comment check for an extra-indented line. Or else I am > missing something. > > That being said, thanks for the test case. I will investigate it. Update: I see and understand your test case. - PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1499787391
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v38]
On Thu, 22 Feb 2024 15:14:32 GMT, Jonathan Gibbons wrote:
>> src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java
>> line 1373:
>>
>>> 1371: while (indent < currIndent) {
>>> 1372: currIndent = indentStack.pop();
>>> 1373: }
>>
>> This new Markdown class looks very good!
>>
>> I think in this place we also need to check for indented code blocks, since
>> we reduced `currIndent` and may have 4 or more character indentation
>> compared to the new `currIndent` value.
>>
>> My tentative fix is to add the following code here, but maybe a more elegant
>> solution can be found by restructuring the method to first check for `indent
>> < currIndent` and then only having to check for new indented code blocks
>> once.
>>
>>
>> if (indent >= currIndent + 4 && !isParagraph(prevLineKind)) {
>> blockId++;
>> lineKind = LineKind.INDENTED_CODE_BLOCK;
>> return;
>> }
>>
>>
>> The following could be added to `TestMarkdownCodeBlocks.java` to test this
>> case:
>>
>>
>> POST_LIST_INDENT(
>> """
>> 1. list item
>>
>> second paragraph
>>
>> {@code CODE}
>> @Anno
>>
>> end""",
>> """
>>
>>
>> list item
>> second paragraph
>>
>>
>> {@code CODE}
>> @Anno
>>
>> end"""),
>
> Thanks. I will investigate your hint to look for a more elegant solution.
> Thanks for the new test case.
>I think in this place we also need to check for indented code blocks, since we
>reduced currIndent and may have 4 or more character indentation compared to
>the new currIndent value.
This seems wrong. Surely, we should not reduce the indentation and then for the
same line in the comment check for an extra-indented line. Or else I am missing
something.
That being said, thanks for the test case. I will investigate it.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1499779864
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v39]
> 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 incrementally with one additional commit since the last revision: refactor tests for Markdown headings into a separate test class. - Changes: - all: https://git.openjdk.org/jdk/pull/16388/files - new: https://git.openjdk.org/jdk/pull/16388/files/5fc9c84a..72420419 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16388=38 - incr: https://webrevs.openjdk.org/?repo=jdk=16388=37-38 Stats: 630 lines in 2 files changed: 345 ins; 285 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/16388.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388 PR: https://git.openjdk.org/jdk/pull/16388
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v38]
On Thu, 22 Feb 2024 14:53:11 GMT, Hannes Wallnöfer wrote:
>> Jonathan Gibbons has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - update DocCommentParser and tests to improve handling of code blocks and
>> code spans in Markdown documentation comments
>> - fix indentation, for consistency
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java
> line 1373:
>
>> 1371: while (indent < currIndent) {
>> 1372: currIndent = indentStack.pop();
>> 1373: }
>
> This new Markdown class looks very good!
>
> I think in this place we also need to check for indented code blocks, since
> we reduced `currIndent` and may have 4 or more character indentation compared
> to the new `currIndent` value.
>
> My tentative fix is to add the following code here, but maybe a more elegant
> solution can be found by restructuring the method to first check for `indent
> < currIndent` and then only having to check for new indented code blocks once.
>
>
> if (indent >= currIndent + 4 && !isParagraph(prevLineKind)) {
> blockId++;
> lineKind = LineKind.INDENTED_CODE_BLOCK;
> return;
> }
>
>
> The following could be added to `TestMarkdownCodeBlocks.java` to test this
> case:
>
>
> POST_LIST_INDENT(
> """
> 1. list item
>
> second paragraph
>
> {@code CODE}
> @Anno
>
> end""",
> """
>
>
> list item
> second paragraph
>
>
> {@code CODE}
> @Anno
>
> end"""),
Thanks. I will investigate your hint to look for a more elegant solution.
Thanks for the new test case.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1499408829
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v38]
On Thu, 22 Feb 2024 00:17:29 GMT, Jonathan Gibbons 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 incrementally with two
> additional commits since the last revision:
>
> - update DocCommentParser and tests to improve handling of code blocks and
> code spans in Markdown documentation comments
> - fix indentation, for consistency
src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java
line 1373:
> 1371: while (indent < currIndent) {
> 1372: currIndent = indentStack.pop();
> 1373: }
This new Markdown class looks very good!
I think in this place we also need to check for indented code blocks, since we
reduced `currIndent` and may have 4 or more character indentation compared to
the new `currIndent` value.
My tentative fix is to add the following code here, but maybe there's a more
elegant solution by restructuring the method to first check for `indent <
currIndent` and then for new indented code blocks.
if (indent >= currIndent + 4 && !isParagraph(prevLineKind)) {
blockId++;
lineKind = LineKind.INDENTED_CODE_BLOCK;
return;
}
The following could be added to `TestMarkdownCodeBlocks.java` to test this case:
POST_LIST_INDENT(
"""
1. list item
second paragraph
{@code CODE}
@Anno
end""",
"""
list item
second paragraph
{@code CODE}
@Anno
end"""),
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1499375866
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v38]
> 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 incrementally with two additional commits since the last revision: - update DocCommentParser and tests to improve handling of code blocks and code spans in Markdown documentation comments - fix indentation, for consistency - Changes: - all: https://git.openjdk.org/jdk/pull/16388/files - new: https://git.openjdk.org/jdk/pull/16388/files/25921fd0..5fc9c84a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16388=37 - incr: https://webrevs.openjdk.org/?repo=jdk=16388=36-37 Stats: 1240 lines in 4 files changed: 918 ins; 275 del; 47 mod Patch: https://git.openjdk.org/jdk/pull/16388.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388 PR: https://git.openjdk.org/jdk/pull/16388
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v36]
On Fri, 16 Feb 2024 07:42:47 GMT, Hannes Wallnöfer wrote: >> Jonathan Gibbons has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - Support for Table Of Contents in Markdown headings >> - fix typo > > src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java > line 286: > >> 284: lineKind = (ch == '\n' || ch == '\r') ? >> LineKind.BLANK >> 285: : (indent <= 3) ? peekLineKind() >> 286: : lineKind != LineKind.OTHER ? >> LineKind.INDENTED_CODE_BLOCK > > Doesn't this cause false positives for indented code blocks? In my > understanding, indented lines [in a list > context](https://spec.commonmark.org/0.30/#example-258) and [directly > following a > blockquote](https://spec.commonmark.org/dingus/?text=%3E%20%20%20%20foo%0A%20%20%20%20%20bar%0A) > are not interpreted as code blocks (always in the case of blockquote, and > depending on the offset of text after the list marker in list items). > > Note: edited because my initial suggestion was incorrect. @hns @pavelrappo noted; working on it ... - PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1494909524
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v37]
> 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 incrementally with one additional commit since the last revision: remove obsolete comment - Changes: - all: https://git.openjdk.org/jdk/pull/16388/files - new: https://git.openjdk.org/jdk/pull/16388/files/53afd804..25921fd0 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16388=36 - incr: https://webrevs.openjdk.org/?repo=jdk=16388=35-36 Stats: 2 lines in 1 file changed: 0 ins; 2 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/16388.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388 PR: https://git.openjdk.org/jdk/pull/16388
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v36]
On Fri, 16 Feb 2024 07:42:47 GMT, Hannes Wallnöfer wrote:
>> Jonathan Gibbons has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Support for Table Of Contents in Markdown headings
>> - fix typo
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java
> line 286:
>
>> 284: lineKind = (ch == '\n' || ch == '\r') ?
>> LineKind.BLANK
>> 285: : (indent <= 3) ? peekLineKind()
>> 286: : lineKind != LineKind.OTHER ?
>> LineKind.INDENTED_CODE_BLOCK
>
> Doesn't this cause false positives for indented code blocks? In my
> understanding, indented lines [in a list
> context](https://spec.commonmark.org/0.30/#example-258) and [directly
> following a
> blockquote](https://spec.commonmark.org/dingus/?text=%3E%20%20%20%20foo%0A%20%20%20%20%20bar%0A)
> are not interpreted as code blocks, but as part of the list or blockquote.
> So my guess would be that `not OTHER` should be something like `BLANK or
> INDENTED_CODE_BLOCK or HEADER`, and that still leaves the problem of a [blank
> line in a list context](https://spec.commonmark.org/0.30/#example-108).
Sigh. I remember when we all hoped that we wouldn't need to go in the weeds of
Markdown parsing, but here we are. Hannes is right. Here are a coupe of
problematic examples:
- /// List
///
/// - item
///
/// @param
- /// > Quote
/// >
/// > {@link java.lang.Object}
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1492430822
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v36]
On Fri, 16 Feb 2024 00:46:34 GMT, Jonathan Gibbons 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 incrementally with two > additional commits since the last revision: > > - Support for Table Of Contents in Markdown headings > - fix typo src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java line 286: > 284: lineKind = (ch == '\n' || ch == '\r') ? > LineKind.BLANK > 285: : (indent <= 3) ? peekLineKind() > 286: : lineKind != LineKind.OTHER ? > LineKind.INDENTED_CODE_BLOCK Doesn't this cause false positives for indented code blocks? In my understanding, indented lines in a list context and directly following a blockquote are not interpreted as code blocks, but as part of the list or blockquote. So my guess would be that `not OTHER` should be something like `BLANK or INDENTED_CODE_BLOCK or HEADER`, and that still leaves the problem of a [blank line in a list context](https://spec.commonmark.org/0.30/#example-108). - PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1492065799
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v36]
> 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 incrementally with two additional commits since the last revision: - Support for Table Of Contents in Markdown headings - fix typo - Changes: - all: https://git.openjdk.org/jdk/pull/16388/files - new: https://git.openjdk.org/jdk/pull/16388/files/da8752c8..53afd804 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16388=35 - incr: https://webrevs.openjdk.org/?repo=jdk=16388=34-35 Stats: 102 lines in 2 files changed: 99 ins; 1 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/16388.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388 PR: https://git.openjdk.org/jdk/pull/16388
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v35]
> 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 incrementally with three additional commits since the last revision: - fix handling of `@' in a Markdown comment - improve comments for some LineKind members - update LineKind members and test - Changes: - all: https://git.openjdk.org/jdk/pull/16388/files - new: https://git.openjdk.org/jdk/pull/16388/files/393d3a9c..da8752c8 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16388=34 - incr: https://webrevs.openjdk.org/?repo=jdk=16388=33-34 Stats: 55 lines in 3 files changed: 49 ins; 0 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/16388.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388 PR: https://git.openjdk.org/jdk/pull/16388
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v32]
On Thu, 15 Feb 2024 19:39:07 GMT, Pavel Rappo wrote:
>> whitespace is handled separately, on line 280 (`readIndent`) and285 (`:
>> (indent <= 3) ? peekLineKind()`)
>
> Correct, but I believe the ordered list marker should be like this:
>
> ORDERED_LIST_ITEM(Pattern.compile("[0-9]{1,9}[.)] .*"))
> ^
>
> Note extra whitespace character. I think we should really add this to our
> tests, since lists are foundational Markdown structures, probably right after
> italics and bold.
Then we should add `[ \t]` to both constants, right:
BULLETED_LIST_ITEM(Pattern.compile("[-+*][ \t].*"))
ORDERED_LIST_ITEM(Pattern.compile("[0-9]{1,9}[.)][ \t].*"))
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1491564839
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v31]
On Thu, 15 Feb 2024 19:36:36 GMT, Jonathan Gibbons wrote:
>> 1. Since forever, and still true, the way to specify a doclet is by its
>> name, and the tool will create the instance for you. This goes back to the
>> original old days before any API, when the only entry point was the command
>> line.
>> This implies that (a) the doclet has a no-args constructor and (b) that all
>> doclet config is done through command line options. A better solution would
>> be to add new functionality to the various javadoc tool API (some or all of
>> `Main`/`Start`/`DocumentationTool`) so that a client can initialize an
>> instance of a doclet and pass that instance into the API.
>>
>> 2. If I understand the question correctly, the code is invoked by using the
>> command-line option `-XDaccessInternalAPI` which is a preexisting hidden
>> feature already supported by `javac`. It is used in `TestTransformer.java`
>> on line 161.
>>
>> javadoc("-d", base.resolve("api").toString(),
>> "-Xdoclint:none",
>> "-XDaccessInternalAPI", // required to access JavacTrees
>> "-doclet", "TestTransformer$MyDoclet",
>> "-docletpath", System.getProperty("test.classes"),
>> "-sourcepath", src.toString(),
>> "p");
>
> I confirm that `TestTransformer.java` fails as expected with an
> `IllegalAccessError` if the option is not used.
>
> java.lang.IllegalAccessError: class TestTransformer$MyDoclet (in unnamed
> module @0x355de863) cannot access class com.sun.tools.javac.api.JavacTrees
> (in module jdk.compiler) because module jdk.compiler does not export
> com.sun.tools.javac.api to unnamed module @0x355de863
> at TestTransformer$MyDoclet.run(TestTransformer.java:139)
> at
> jdk.javadoc/jdk.javadoc.internal.tool.Start.parseAndExecute(Start.java:589)
Generally, the error occurs because the `MyDoclet` class is run in a different
class loader than that used for the test. The class loader for the test
already has the necessary access permissions given, from the lines in
`@modules` in the `jtreg` test description. Ideally, we could call `new
MyDoclet()` in the main test code, and pas the instance in to the `javadoc`
call and from there to the javadoc `Start` method.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1491547571
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v32]
On Thu, 15 Feb 2024 19:39:07 GMT, Pavel Rappo wrote:
>> whitespace is handled separately, on line 280 (`readIndent`) and285 (`:
>> (indent <= 3) ? peekLineKind()`)
>
> Correct, but I believe the ordered list marker should be like this:
>
> ORDERED_LIST_ITEM(Pattern.compile("[0-9]{1,9}[.)] .*"))
> ^
>
> Note extra whitespace character. I think we should really add this to our
> tests, since lists are foundational Markdown structures, probably right after
> italics and bold.
My recollection is that the space is required. I will double check.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1491552312
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v32]
On Thu, 15 Feb 2024 19:20:23 GMT, Jonathan Gibbons wrote:
>> src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java
>> line 1401:
>>
>>> 1399: */
>>> 1400: enum LineKind {
>>> 1401: BLANK(Pattern.compile("[ \t]*")),
>>
>> `BLANK` is a pseudo kind, because it is set manually, but never returned
>> from `peekLine()`. I wonder if we can change it somehow.
>
> Even if it is set manually, it is still appropriate to have it as a member in
> the `LineKind` enum class.
Not that it invalidates your reply; clarification: I meant `peekLineKind()`,
not `peekLine()`.
>> src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java
>> line 1433:
>>
>>> 1431: * @see >> href="https://spec.commonmark.org/0.30/#list-items;>List items
>>> 1432: */
>>> 1433: BULLETED_LIST_ITEM(Pattern.compile("[-+*] .*")),
>>
>> This comment is about `BULLETED_LIST_ITEM` and `ORDERED_LIST_ITEM`
>> constants. I know that we don't need to be very precise, but perhaps in this
>> case we should. While the CommonMark spec is a vague on that, from my
>> experiments with [dingus](https://spec.commonmark.org/dingus/), it appears
>> that a list marker can be preceded and followed by some number of whitespace
>> characters.
>
> whitespace is handled separately, on line 280 (`readIndent`) and285 (`:
> (indent <= 3) ? peekLineKind()`)
Correct, but I believe the ordered list marker should be like this:
ORDERED_LIST_ITEM(Pattern.compile("[0-9]{1,9}[.)] .*"))
^
Note extra whitespace character. I think we should really add this to our
tests, since lists are foundational Markdown structures, probably right after
italics and bold.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1491550784
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1491545609
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v31]
On Thu, 15 Feb 2024 19:15:25 GMT, Jonathan Gibbons wrote:
>> src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/Start.java line 571:
>>
>>> 569: // of a doclet to be specified instead of the name of the
>>> 570: // doclet class and optional doclet path.
>>> 571: // See https://bugs.openjdk.org/browse/JDK-8263219
>>
>> It's hard to understand this:
>>
>>> to permit an instance of an appropriately configured instance of a doclet
>>
>> Also: how is that piece of code used? When I commented it out, no
>> test/langtools:langtools_javadoc tests failed.
>
> 1. Since forever, and still true, the way to specify a doclet is by its
> name, and the tool will create the instance for you. This goes back to the
> original old days before any API, when the only entry point was the command
> line.
> This implies that (a) the doclet has a no-args constructor and (b) that all
> doclet config is done through command line options. A better solution would
> be to add new functionality to the various javadoc tool API (some or all of
> `Main`/`Start`/`DocumentationTool`) so that a client can initialize an
> instance of a doclet and pass that instance into the API.
>
> 2. If I understand the question correctly, the code is invoked by using the
> command-line option `-XDaccessInternalAPI` which is a preexisting hidden
> feature already supported by `javac`. It is used in `TestTransformer.java`
> on line 161.
>
> javadoc("-d", base.resolve("api").toString(),
> "-Xdoclint:none",
> "-XDaccessInternalAPI", // required to access JavacTrees
> "-doclet", "TestTransformer$MyDoclet",
> "-docletpath", System.getProperty("test.classes"),
> "-sourcepath", src.toString(),
> "p");
I confirm that `TestTransformer.java` fails as expected with an
`IllegalAccessError` if the option is not used.
java.lang.IllegalAccessError: class TestTransformer$MyDoclet (in unnamed module
@0x355de863) cannot access class com.sun.tools.javac.api.JavacTrees (in module
jdk.compiler) because module jdk.compiler does not export
com.sun.tools.javac.api to unnamed module @0x355de863
at TestTransformer$MyDoclet.run(TestTransformer.java:139)
at
jdk.javadoc/jdk.javadoc.internal.tool.Start.parseAndExecute(Start.java:589)
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1491538120
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v34]
> 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 incrementally with two additional commits since the last revision: - fix return tag name - remove debug print - Changes: - all: https://git.openjdk.org/jdk/pull/16388/files - new: https://git.openjdk.org/jdk/pull/16388/files/f6d08db9..393d3a9c Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16388=33 - incr: https://webrevs.openjdk.org/?repo=jdk=16388=32-33 Stats: 2 lines in 2 files changed: 0 ins; 1 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/16388.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388 PR: https://git.openjdk.org/jdk/pull/16388
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v32]
On Thu, 15 Feb 2024 19:27:12 GMT, Jonathan Gibbons wrote: >> src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java >> line 422: >> >>> 420: defaultContentCharacter(); >>> 421: } >>> 422: } >> >> Is it to pass `` through to Markdown, to allow it to deal with Markdown >> escapes? > > It is more about supporting the sequence `` ` `` so that the backtick is > treated as a literal character and not the beginning of a code span. This > is the "backstop" preferred way to ensure that a single backtick is treated > literally, without relying on detected that it is unbalanced when the end of > the current block is reached. The alternative workaround would be a single > backtick enclosed in multiple backticks, such as this ``` `` ` `` ```. (I > leave you to figure out what I actually typed there!!!) You might also need to use `` ` `` when there are two literal backticks in a sentence. After the first backtick (`), another backtick (`) can be used to delimit a code span. - PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1491528153
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v32]
On Thu, 15 Feb 2024 17:17:46 GMT, Pavel Rappo 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 44 commits: >> >> - fill in `visitRawText` in `CommentHelper.getTags` visitor >> - fixes for the "New API" page >> - change "standard" to "traditional" when referring to a comment >> - Merge remote-tracking branch 'upstream/master' into >> 8298405.doclet-markdown-v3 >> - Merge remote-tracking branch 'upstream/master' into >> 8298405.doclet-markdown-v3 >> - improve support for DocCommentParser.LineKind >> - Merge remote-tracking branch 'upstream/master' into >> 8298405.doclet-markdown-v3 # Please enter a commit message to explain why >> this merge is necessary, # especially if it merges an updated upstream into >> a topic branch. # # Lines starting with '#' will be ignored, and an empty >> message aborts # the >>commit. >> - update copyright year on test >> - refactor recent new test case in TestMarkdown.java >> - address review feedback >> - ... and 34 more: https://git.openjdk.org/jdk/compare/8765b176...2801c2e1 > > src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java > line 422: > >> 420: defaultContentCharacter(); >> 421: } >> 422: } > > Is it to pass `` through to Markdown, to allow it to deal with Markdown > escapes? It is more about supporting the sequence `` ` `` so that the backtick is treated as a literal character and not the beginning of a code span. This is the "backstop" preferred way to ensure that a single backtick is treated literally, without relying on detected that it is unbalanced when the end of the current block is reached. The alternative workaround would be a single backtick enclosed in multiple backticks, such as this ``` `` ` `` ```. (I leave you to figure out what I actually typed there!!!) - PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1491519707
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v32]
On Thu, 15 Feb 2024 17:03:09 GMT, Pavel Rappo 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 44 commits:
>>
>> - fill in `visitRawText` in `CommentHelper.getTags` visitor
>> - fixes for the "New API" page
>> - change "standard" to "traditional" when referring to a comment
>> - Merge remote-tracking branch 'upstream/master' into
>> 8298405.doclet-markdown-v3
>> - Merge remote-tracking branch 'upstream/master' into
>> 8298405.doclet-markdown-v3
>> - improve support for DocCommentParser.LineKind
>> - Merge remote-tracking branch 'upstream/master' into
>> 8298405.doclet-markdown-v3 # Please enter a commit message to explain why
>> this merge is necessary, # especially if it merges an updated upstream into
>> a topic branch. # # Lines starting with '#' will be ignored, and an empty
>> message aborts # the
>>commit.
>> - update copyright year on test
>> - refactor recent new test case in TestMarkdown.java
>> - address review feedback
>> - ... and 34 more: https://git.openjdk.org/jdk/compare/8765b176...2801c2e1
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java
> line 1401:
>
>> 1399: */
>> 1400: enum LineKind {
>> 1401: BLANK(Pattern.compile("[ \t]*")),
>
> `BLANK` is a pseudo kind, because it is set manually, but never returned from
> `peekLine()`. I wonder if we can change it somehow.
Even if it is set manually, it is still appropriate to have it as a member in
the `LineKind` enum class.
> src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java
> line 1433:
>
>> 1431: * @see > href="https://spec.commonmark.org/0.30/#list-items;>List items
>> 1432: */
>> 1433: BULLETED_LIST_ITEM(Pattern.compile("[-+*] .*")),
>
> This comment is about `BULLETED_LIST_ITEM` and `ORDERED_LIST_ITEM` constants.
> I know that we don't need to be very precise, but perhaps in this case we
> should. While the CommonMark spec is a vague on that, from my experiments
> with [dingus](https://spec.commonmark.org/dingus/), it appears that a list
> marker can be preceded and followed by some number of whitespace characters.
whitespace is handled separately, on line 280 (`readIndent`) and285 (`: (indent
<= 3) ? peekLineKind()`)
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1491508303
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1491512611
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v31]
On Tue, 13 Feb 2024 11:15:55 GMT, Pavel Rappo 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 40 commits:
>>
>> - Merge remote-tracking branch 'upstream/master' into
>> 8298405.doclet-markdown-v3
>> - improve support for DocCommentParser.LineKind
>> - Merge remote-tracking branch 'upstream/master' into
>> 8298405.doclet-markdown-v3 # Please enter a commit message to explain why
>> this merge is necessary, # especially if it merges an updated upstream into
>> a topic branch. # # Lines starting with '#' will be ignored, and an empty
>> message aborts # the
>>commit.
>> - update copyright year on test
>> - refactor recent new test case in TestMarkdown.java
>> - address review feedback
>> - address review feedback
>> - added test case in TestMarkdown.java for handling of `@deprecated` tag
>> - amend comment in test
>> - improve comments on negative test case
>> - ... and 30 more: https://git.openjdk.org/jdk/compare/2ed889b7...1c64a6e0
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/Checker.java line
> 1021:
>
>> 1019: .findFirst();
>> 1020: if (first.isEmpty() || first.get() != tree) {
>> 1021: dct.getFirstSentence().forEach(t ->
>> System.err.println(t.getKind() + ": >>|" + t + "|<<"));
>
> Is it leftover debug output?
Oops, yes. Thanks.
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/Start.java line 571:
>
>> 569: // of a doclet to be specified instead of the name of the
>> 570: // doclet class and optional doclet path.
>> 571: // See https://bugs.openjdk.org/browse/JDK-8263219
>
> It's hard to understand this:
>
>> to permit an instance of an appropriately configured instance of a doclet
>
> Also: how is that piece of code used? When I commented it out, no
> test/langtools:langtools_javadoc tests failed.
1. Since forever, and still true, the way to specify a doclet is by its name,
and the tool will create the instance for you. This goes back to the original
old days before any API, when the only entry point was the command line.
This implies that (a) the doclet has a no-args constructor and (b) that all
doclet config is done through command line options. A better solution would be
to add new functionality to the various javadoc tool API (some or all of
`Main`/`Start`/`DocumentationTool`) so that a client can initialize an instance
of a doclet and pass that instance into the API.
2. If I understand the question correctly, the code is invoked by using the
command-line option `-XDaccessInternalAPI` which is a preexisting hidden
feature already supported by `javac`. It is used in `TestTransformer.java` on
line 161.
javadoc("-d", base.resolve("api").toString(),
"-Xdoclint:none",
"-XDaccessInternalAPI", // required to access JavacTrees
"-doclet", "TestTransformer$MyDoclet",
"-docletpath", System.getProperty("test.classes"),
"-sourcepath", src.toString(),
"p");
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1491504223
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1491502389
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v33]
> 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 incrementally with one additional commit since the last revision: fix compilation and whitespace issues - Changes: - all: https://git.openjdk.org/jdk/pull/16388/files - new: https://git.openjdk.org/jdk/pull/16388/files/2801c2e1..f6d08db9 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16388=32 - incr: https://webrevs.openjdk.org/?repo=jdk=16388=31-32 Stats: 4 lines in 2 files changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/16388.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388 PR: https://git.openjdk.org/jdk/pull/16388
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v32]
On Thu, 15 Feb 2024 00:30:25 GMT, Jonathan Gibbons 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 44 commits:
>
> - fill in `visitRawText` in `CommentHelper.getTags` visitor
> - fixes for the "New API" page
> - change "standard" to "traditional" when referring to a comment
> - Merge remote-tracking branch 'upstream/master' into
> 8298405.doclet-markdown-v3
> - Merge remote-tracking branch 'upstream/master' into
> 8298405.doclet-markdown-v3
> - improve support for DocCommentParser.LineKind
> - Merge remote-tracking branch 'upstream/master' into
> 8298405.doclet-markdown-v3 # Please enter a commit message to explain why
> this merge is necessary, # especially if it merges an updated upstream into a
> topic branch. # # Lines starting with '#' will be ignored, and an empty
> message aborts # the
>commit.
> - update copyright year on test
> - refactor recent new test case in TestMarkdown.java
> - address review feedback
> - ... and 34 more: https://git.openjdk.org/jdk/compare/8765b176...2801c2e1
I'm again looking `LineKind`. This time because of 9eaf84e5dd6.
src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java
line 422:
> 420: defaultContentCharacter();
> 421: }
> 422: }
Is it to pass `` through to Markdown, to allow it to deal with Markdown escapes?
src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java
line 1401:
> 1399: */
> 1400: enum LineKind {
> 1401: BLANK(Pattern.compile("[ \t]*")),
`BLANK` is a pseudo kind, because it is set manually, but never returned from
`peekLine()`. I wonder if we can change it somehow.
src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java
line 1433:
> 1431: * @see href="https://spec.commonmark.org/0.30/#list-items;>List items
> 1432: */
> 1433: BULLETED_LIST_ITEM(Pattern.compile("[-+*] .*")),
This comment is about `BULLETED_LIST_ITEM` and `ORDERED_LIST_ITEM` constants. I
know that we don't need to be very precise, but perhaps in this case we should.
While the CommonMark spec is a vague on that, from my experiments with
[dingus](https://spec.commonmark.org/dingus/), it appears that a list marker
can be preceded and followed by some number of whitespace characters.
-
PR Review: https://git.openjdk.org/jdk/pull/16388#pullrequestreview-1883374712
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1491362821
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1491344667
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1491354450
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v32]
> 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 44 commits: - fill in `visitRawText` in `CommentHelper.getTags` visitor - fixes for the "New API" page - change "standard" to "traditional" when referring to a comment - Merge remote-tracking branch 'upstream/master' into 8298405.doclet-markdown-v3 - Merge remote-tracking branch 'upstream/master' into 8298405.doclet-markdown-v3 - improve support for DocCommentParser.LineKind - Merge remote-tracking branch 'upstream/master' into 8298405.doclet-markdown-v3 # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit. - update copyright year on test - refactor recent new test case in TestMarkdown.java - address review feedback - ... and 34 more: https://git.openjdk.org/jdk/compare/8765b176...2801c2e1 - Changes: https://git.openjdk.org/jdk/pull/16388/files Webrev: https://webrevs.openjdk.org/?repo=jdk=16388=31 Stats: 22094 lines in 197 files changed: 21411 ins; 286 del; 397 mod Patch: https://git.openjdk.org/jdk/pull/16388.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388 PR: https://git.openjdk.org/jdk/pull/16388
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v31]
On Mon, 12 Feb 2024 23:52:35 GMT, Jonathan Gibbons 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 40 commits:
>
> - Merge remote-tracking branch 'upstream/master' into
> 8298405.doclet-markdown-v3
> - improve support for DocCommentParser.LineKind
> - Merge remote-tracking branch 'upstream/master' into
> 8298405.doclet-markdown-v3 # Please enter a commit message to explain why
> this merge is necessary, # especially if it merges an updated upstream into a
> topic branch. # # Lines starting with '#' will be ignored, and an empty
> message aborts # the
>commit.
> - update copyright year on test
> - refactor recent new test case in TestMarkdown.java
> - address review feedback
> - address review feedback
> - added test case in TestMarkdown.java for handling of `@deprecated` tag
> - amend comment in test
> - improve comments on negative test case
> - ... and 30 more: https://git.openjdk.org/jdk/compare/2ed889b7...1c64a6e0
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java
line 563:
> 561:
> 562: /**
> 563: * {@returns the plain-text content of a named HTML element in a
> list of content}
Suggestion:
* {@return the plain-text content of a named HTML element in a list of
content}
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/Checker.java line
1021:
> 1019: .findFirst();
> 1020: if (first.isEmpty() || first.get() != tree) {
> 1021: dct.getFirstSentence().forEach(t ->
> System.err.println(t.getKind() + ": >>|" + t + "|<<"));
Is it leftover debug output?
src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/Start.java line 571:
> 569: // of a doclet to be specified instead of the name of the
> 570: // doclet class and optional doclet path.
> 571: // See https://bugs.openjdk.org/browse/JDK-8263219
It's hard to understand this:
> to permit an instance of an appropriately configured instance of a doclet
Also: how is that piece of code used? When I commented it out, no
test/langtools:langtools_javadoc tests failed.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1487629102
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1487659843
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1487642764
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v31]
> 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 40 commits: - Merge remote-tracking branch 'upstream/master' into 8298405.doclet-markdown-v3 - improve support for DocCommentParser.LineKind - Merge remote-tracking branch 'upstream/master' into 8298405.doclet-markdown-v3 # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit. - update copyright year on test - refactor recent new test case in TestMarkdown.java - address review feedback - address review feedback - added test case in TestMarkdown.java for handling of `@deprecated` tag - amend comment in test - improve comments on negative test case - ... and 30 more: https://git.openjdk.org/jdk/compare/2ed889b7...1c64a6e0 - Changes: https://git.openjdk.org/jdk/pull/16388/files Webrev: https://webrevs.openjdk.org/?repo=jdk=16388=30 Stats: 22058 lines in 194 files changed: 21390 ins; 271 del; 397 mod Patch: https://git.openjdk.org/jdk/pull/16388.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388 PR: https://git.openjdk.org/jdk/pull/16388
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v22]
On Mon, 12 Feb 2024 17:27:42 GMT, Jonathan Gibbons wrote: >>> I guess it depends how much we want to import the code as-is, without >>> knowing any lint-warts. >> >> I think it would be nice not to have to modify code beyond superficial >> edits: addition of headers, renaming of packages, and converting of >> non-ASCII symbols. > > I think adding `@SuppressWarnings` is a superficial edit, but I see the > writing on the wall. Based on additional private conversations, and because the annotation is added automatically by the same utility used to address other issues in the imported code (package declarations, import statements, non-ASCII characters, etc), I will leave the annotation in at this time. - PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1486674245
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v30]
On Mon, 12 Feb 2024 17:44:29 GMT, Pavel Rappo wrote:
>> Jonathan Gibbons has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> refactor recent new test case in TestMarkdown.java
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java
> line 1465:
>
>> 1463: markdownInput.append('\n');
>> 1464: }
>> 1465: markdownInput.append(PLACEHOLDER_BLOCK);
>
> There's quite a lot of overlap with the recently simplified
> https://github.com/openjdk/jdk/blob/7c6316886d1ae86a663d996dae373c42281622fd/src/jdk.internal.md/share/classes/jdk/internal/markdown/MarkdownTransformer.java#L220-L238
>
> If it's impractical to factor out the common bits, maybe we could at least
> make the respective parts of HtmlDocletWriter as close as possible to those
> of MarkdownTransformer.
I think it is impractical to factor out the common bits, but I will look at the
possibility of making the code more similar -- but no promises.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1486609585
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v30]
On Fri, 9 Feb 2024 22:17:43 GMT, Jonathan Gibbons 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 incrementally with one
> additional commit since the last revision:
>
> refactor recent new test case in TestMarkdown.java
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java
line 1465:
> 1463: markdownInput.append('\n');
> 1464: }
> 1465: markdownInput.append(PLACEHOLDER_BLOCK);
There's quite a lot of overlap with the recently simplified
https://github.com/openjdk/jdk/blob/7c6316886d1ae86a663d996dae373c42281622fd/src/jdk.internal.md/share/classes/jdk/internal/markdown/MarkdownTransformer.java#L220-L238
If it's impractical to factor out the common bits, maybe we could at least make
the respective parts of HtmlDocletWriter as close as possible to those of
MarkdownTransformer.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1486548869
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v22]
On Mon, 12 Feb 2024 17:23:56 GMT, Pavel Rappo wrote: >> It's possible, but that would be a more global setting, whereas this is a >> very targeted modification. >> >> I guess it depends how much we want to import the code as-is, without >> knowing any lint-warts. >> >> FWIW, any module can be compiled with module-specific lint settings, if we >> choose to do so. > >> I guess it depends how much we want to import the code as-is, without >> knowing any lint-warts. > > I think it would be nice not to have to modify code beyond superficial edits: > addition of headers, renaming of packages, and converting of non-ASCII > symbols. I think adding `@SuppressWarnings` is a superficial edit, but I see the writing on the wall. - PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1486531142
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v22]
On Mon, 12 Feb 2024 16:29:19 GMT, Jonathan Gibbons wrote: > I guess it depends how much we want to import the code as-is, without knowing > any lint-warts. I think it would be nice not to have to modify code beyond superficial edits: addition of headers, renaming of packages, and converting of non-ASCII symbols. - PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1486526603
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v22]
On Thu, 8 Feb 2024 17:20:23 GMT, Pavel Rappo 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 28 commits:
>>
>> - Merge remote-tracking branch 'upstream/master' into
>> 8298405.doclet-markdown-v3 # Please enter a commit message to explain why
>> this merge is necessary, # especially if it
>>merges an updated upstream into a topic branch. # # Lines starting with
>> '#' will be ignored, and an empty message aborts # the commit.
>> - add test case contributed by @lahodaj
>> - initial fix for source offset problem
>> - remove unused imports
>> - First pass at remove DocCommentTransformer from the public API.
>>
>>It is still declared internally, and installed by default, using the
>> service-provider mechanism.
>>If the standard impl is not available, an identity transformer is used.
>> - updates for "first sentence" issues and tests
>> - add info about provenance of `jdk.internal.md` module
>> - MarkdownTransformer: tweak doc comments
>> - MarkdownTransformer: change `Lower.replaceIter` to be `private final`
>> - MarkdownTransformer: use suggested text for using streams
>> - ... and 18 more: https://git.openjdk.org/jdk/compare/18e24d06...63dd8bf4
>
> src/jdk.internal.md/share/classes/module-info.java line 41:
>
>> 39: // * package and import statements are updated
>> 40: // * characters outside the ASCII range are converted to Unicode escapes
>> 41: // * @SuppressWarnings("fallthrough") is added to getSetextHeadingLevel
>
> I wonder if it would be better to compile this module without (some) lint
> checks. Is it possible?
It's possible, but that would be a more global setting, whereas this is a very
targeted modification.
I guess it depends how much we want to import the code as-is, without knowing
any lint-warts.
FWIW, any module can be compiled with module-specific lint settings, if we
choose to do so.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1486450189
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v7]
On Thu, 8 Feb 2024 18:52:54 GMT, Jonathan Gibbons wrote:
>> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/markup/RawHtml.java
>> line 145:
>>
>>> 143: }
>>> 144:
>>> 145: Pattern tag = Pattern.compile("<(?[A-Za-z0-9]+)(\\s|>)");
>>
>> I'm not sure I grok this pattern; what's up with `\\s`?
>
> The code is looking for HTML tag names, The match for a tag name is one of
> * `<` _tag-name_ `>`
> * `<` _tag-name_ _whitespace_
I see, thanks.
Suggestion:
private final Pattern tag = Pattern.compile("<(?[A-Za-z0-9]+)(\\s|>)");
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1486384478
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v7]
On Tue, 30 Jan 2024 23:47:00 GMT, Jonathan Gibbons wrote:
>> 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.
It's just a funny word play evoking the famous eco slogan, is all.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1486368352
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v7]
On Thu, 1 Feb 2024 00:19:42 GMT, Jonathan Gibbons wrote: >> I'll add a test case. > > Done Thank you. I double-checked: if that `replace` is removed, jdk/javadoc/doclet/testMarkdown/TestMarkdown.java fails. - PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1486361794
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v7]
On Wed, 31 Jan 2024 22:15:23 GMT, Jonathan Gibbons wrote: >> 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) > > I've changed the prefix to be dynamically generated. It's now called > `autorefScheme` and is passed around as needed. It contains a hex hashcode, > so is reasonably unguessable. > > I'm still sort-of sad to lose the ability to use `code:` URLs directly, so > I've left a comment in the code hinting that that is a possibility. Thanks for accepting this; we could revert it later if the fixed, known scheme is deemed more useful. - PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1486364459
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v7]
On Tue, 30 Jan 2024 23:15:32 GMT, Jonathan Gibbons wrote:
>> 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}[-_*#=]))");
>
> The code is doing its best to model the non-Markdown behavior, which is to
> detect paragraph breaks, which terminate the first sentence in the absence of
> any period.
>
> `+` is in the pattern as a list marker; I added `_` for thematic break, and
> added more comments.
I can see that you have fixed it to recognise lists `+` and thematic breaks
`_`; good.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1486343596
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v30]
> 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 incrementally with one additional commit since the last revision: refactor recent new test case in TestMarkdown.java - Changes: - all: https://git.openjdk.org/jdk/pull/16388/files - new: https://git.openjdk.org/jdk/pull/16388/files/d22668da..3f8aa6b5 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16388=29 - incr: https://webrevs.openjdk.org/?repo=jdk=16388=28-29 Stats: 42 lines in 1 file changed: 1 ins; 3 del; 38 mod Patch: https://git.openjdk.org/jdk/pull/16388.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388 PR: https://git.openjdk.org/jdk/pull/16388
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v29]
> 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 incrementally with one additional commit since the last revision: address review feedback - Changes: - all: https://git.openjdk.org/jdk/pull/16388/files - new: https://git.openjdk.org/jdk/pull/16388/files/91588bc3..d22668da Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16388=28 - incr: https://webrevs.openjdk.org/?repo=jdk=16388=27-28 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/16388.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388 PR: https://git.openjdk.org/jdk/pull/16388
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v6]
On Fri, 9 Feb 2024 18:27:59 GMT, Jonathan Gibbons wrote: >> src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java >> line 201: >> >>> 199: } >>> 200: newline = true; >>> 201: } >> >> I can see clean LF and CRLF, but no FF; was the latter intentional? >> >> In any case, we should double-check the generated test output to see if >> there's anything unexpected. > > We've been here, with `\f` before, noting the different handling of `\f` in > `javac` and `javadoc`. > > My recollection is that it was intentional to drop support for `\f`, thus > aligning `javac` and `javadoc` behavior, and to normalize handling of the > newline sequences, translating them to `\n`. > In any case, we should double-check the generated test output to see if > there's anything unexpected. verified tests pass and no change in generated JDK API docs - PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1484812046
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v6]
On Mon, 15 Jan 2024 11:48:23 GMT, Pavel Rappo wrote:
>> Jonathan Gibbons has updated the pull request with a new target base due to
>> a merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains seven additional
>> commits since the last revision:
>>
>> - 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/source/util/DocTreeFactory.java line
> 299:
>
>> 297: * @param code the code
>> 298: * @return a {@code RawTextTree} object
>> 299: * @throws IllegalArgumentException if the kind is not a recognized
>> kind for raw text
>
> This method's implementation also throws `NullPointerException` if kind is
> null, which is unusual for these methods. You can either add `@throws`, or
> workaround it by using `String.valueOf(kind)` instead of `kind.toString()`
> down in the implementation.
It took me a while to understand this comment.
For here and now, I will use `String.valueOf(kind)`
Long term, it would be good to better document `null` behavior in this API.
[JDK-8325577](https://bugs.openjdk.org/browse/JDK-8325577)
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1484798682
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v6]
On Mon, 15 Jan 2024 12:20:57 GMT, Pavel Rappo wrote: >> Jonathan Gibbons has updated the pull request with a new target base due to >> a merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains seven additional >> commits since the last revision: >> >> - 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/parser/DocCommentParser.java > line 130: > >> 128: >> 129: /** >> 130: * Create a parser for a documentation comment. > > Suggestion: > > * Creates a parser for a documentation comment. Done - PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1484737196
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v28]
On Thu, 8 Feb 2024 15:38:26 GMT, Pavel Rappo wrote: >> test/langtools/jdk/javadoc/tool/sampleapi/lib/sampleapi/generator/ModuleGenerator.java >> line 49: >> >>> 47: import sampleapi.util.PoorDocCommentTable; >>> 48: >>> 49: import static >>> com.sun.tools.javac.parser.Tokens.Comment.CommentStyle.JAVADOC; >> >> To clarify, the change to this file is that you removed two unused imports, >> right? > > Ping. Maybe if it is not essential, we could remove that change, to keep PR > more focused. Okay I can see that you tried to revert the change to that file and published a [PR] to optimise imports separately. Sadly, the "revert" introduced even more churn. Here's my proposal, which might not be elegant but does the job: git checkout 8298405.doclet-markdown-v3 git diff -R head --not upstream/master -- test/langtools/jdk/javadoc/tool/sampleapi/lib/sampleapi/generator/ModuleGenerator.java | git apply git commit -am "Revert all changes to ModuleGenerator.java" If you have any issues with that snippet, ping me out of band; I hope the intent of the snippet is clear though. [PR]: https://github.com/openjdk/jdk/pull/17782 - PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1484477033
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v28]
On Thu, 8 Feb 2024 21:46:34 GMT, Jonathan Gibbons wrote:
>> I believe this is substantially covered in line 226-227. See the third call
>> to `test` in the following group of lines:
>>
>>
>> for (String src : sources) {
>> test(src);
>> test(src.replaceAll("@Deprecated", "/** @deprecated */"));
>> test(src.replaceAll("deprecated", "notDeprecated2") //
>> change class name
>> .replaceAll("@Deprecated", "/// @deprecated\n"));
>> }
>>
>>
>> * The first call, `test(src)`, runs all the test cases, using the
>> `@Deprecated` annotation by default. In these test cases, the name of the
>> element encodes whether it is expected that the element is deprecated or not.
>>
>> * The second call, `test(src.replaceAll("deprecated", "notDeprecated2")`,
>> runs the test cases again, after changing the annotation to a traditional
>> (`/** ...*/`) comment containing the `@deprecated` tag. This is a
>> long-standing call, and tests the legacy behavior of `@deprecated` appearing
>> in a traditional doc comment. Effectively, it tests whether a `/**
>> @deprecated */` comment has equivalent behavior to a `@Deprecated` comment.
>>
>> * The third call is new to this PR and the functionality to support Markdown
>> comments. It makes two changes to the source for the test cases, before
>> running them:
>>1. as in the previous test, the annotations are replaced by a comment
>> containing `@deprecated` -- except that this time, the comment is a
>> new-style `/// ... ` comment instead of a traditional `/** ... */` comment,
>> and ...
>>2. because we do not expect `/// @deprecated` to indicate deprecation, we
>> need to change the expected result for each test case, which we do by
>> changing the element name for the test case. The change is the first call
>> to replace to avoid false positives after changing the doc comment. The
>> change uses a new name, `notDeprecated2`, in which `notDeprecated` encodes
>> the expected deprecation status, and the `2` is to differentiate the
>> declarations from other declarations in the case case that already use the
>> name `notDeprecated`.
>
> While the underlying mechanism in javac for indicating deprecation is the
> same for all, I accept this is primarily a test for generated class files. I
> can add a javadoc test for basic behavior of `@deprecated` in Markdown
> comments.
Thanks for confirming that there's a test to check that `/// @deprecated` is a
no-op and for patiently explaining how that test works.
I confirm that
test/langtools/tools/javac/classfiles/attributes/deprecated/DeprecatedTest.java
starts failing if I introduce the following change:
---
a/src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavaTokenizer.java
+++
b/src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavaTokenizer.java
@@ -1638,7 +1638,7 @@ protected void scanDocComment() {
? trimJavadocLineComment(line, indent)
: trimJavadocComment(line);
-if (cs == CommentStyle.JAVADOC_BLOCK) {
+//if (cs == CommentStyle.JAVADOC_BLOCK) {
// If standalone @deprecated tag
int pos = line.position();
line.skipWhitespace();
@@ -1652,7 +1652,7 @@ protected void scanDocComment() {
}
line.reset(pos);
-}
+//}
putLine(line);
}
I can also see that you have introduced a dedicated
test/langtools/jdk/javadoc/doclet/testMarkdown/TestMarkdown.java#testDeprecated
which also starts failing with that change of mine.
Please update the copyright year in
test/langtools/jdk/javadoc/doclet/testMarkdown/TestMarkdown.java.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1484410402
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v7]
On Tue, 23 Jan 2024 13:02:46 GMT, Pavel Rappo 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.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java
> line 1315:
>
>> 1313: }
>> 1314:
>> 1315: void skipLine() {
>
> Like `peekLike()`, this method also does not seem to be consistent with
> `newline` in `nextChar()`.
It's okay. You explained my confusion here:
https://github.com/openjdk/jdk/pull/16388#discussion_r1470367971
> src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java
> line 1419:
>
>> 1417: LineKind peekLineKind() {
>> 1418: switch (ch) {
>> 1419: case '#', '=', '-', '+', '_', '`', '~' -> {
>
> This change is to match that of `THEMATIC_BREAK`:
> Suggestion:
>
> case '#', '=', '-', '*', '_', '`', '~' -> {
While you seem to have forgotten to change it, no tests failed, which is sad.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1484619952
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1484617917
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v28]
On Wed, 15 Nov 2023 00:37:06 GMT, Jonathan Gibbons wrote:
>> test/langtools/jdk/javadoc/tool/testTransformer/TestTransformer.java line 96:
>>
>>> 94: var sl =
>>> ServiceLoader.load(DocTrees.DocCommentTreeTransformer.class);
>>> 95: return StreamSupport.stream(sl.spliterator(), false)
>>> 96: .filter(p -> p.name().equals(name))
>>
>> ServiceLoader specification suggests another way of streaming:
>> Suggestion:
>>
>> return sl.stream()
>> .map(ServiceLoader.Provider::get)
>> .filter(t -> t.name().equals(name))
>>
>> Would it be the same and more readable?
>
> Hmm. It's longer, but arguably simpler to comprehend than using a
> spliterator. I've changed the code.
There's now a leftover `import java.util.stream.StreamSupport;` in that file.
>> test/langtools/tools/javac/doctree/MDPrinter.java line 67:
>>
>>> 65: * Conceptually based on javac's {@code DPrinter}.
>>> 66: */
>>> 67: public class MDPrinter {
>>
>> While DPrinter is used, MDPrinter isn't. (At least, I could't find any
>> usages of it.) If you feel like MDPrinter is important, we should at least
>> add a compile test for it, to protect it from bitrot.
>
>
MDPrinter.java is now a test that compiles itself; thanks.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1484541756
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1484196535
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v28]
On Thu, 8 Feb 2024 15:35:58 GMT, Pavel Rappo wrote: >> Please update the DocComment printout in that commented out test: the actual >> content is different. It would be nice if the test were passing at least at >> the moment of its initial commit. >> >> Here's what I see locally: >> >> >> Expect: >> DocComment[DOC_COMMENT, pos:0 >> firstSentence: 1 >> Summary[SUMMARY, pos:4 >> summary: 1 >> Erroneous[ERRONEOUS, pos:14, prefPos:37 >> code: compiler.err.dc.unterminated.inline.tag >> body: abc_`|_def}|_rest_`more` >> ] >> ] >> body: empty >> block tags: empty >> ] >> >> Found: >> DocComment[DOC_COMMENT, pos:0 >> firstSentence: 1 >> Summary[SUMMARY, pos:1 >> summary: 1 >> Erroneous[ERRONEOUS, pos:11, prefPos:32 >> code: compiler.err.dc.unterminated.inline.tag >> body: abc_`|def}|rest_`more` >> ] >> ] >> body: empty >> block tags: empty >> ] > > Ping. Thanks for fixing the test and clarifying its comment. - PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1484174598
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v6]
On Mon, 29 Jan 2024 23:50:25 GMT, Jonathan Gibbons wrote: >> Probably, yes. > > New class comment added. I can see it and it reads nicely; thanks! - PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1484586216
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v6]
On Tue, 30 Jan 2024 00:47:33 GMT, Jonathan Gibbons wrote: > > Musing on this more. > > Can/should we, without introducing probably unwelcome `Kind.MD` to > > `javax.tools.JavaFileObject.Kind`, teach javac to recognise `package.md` > > similarly to how it recognises legacy `package.html`? If we are aiming for > > Markdown to be a drop in replacement for traditional javadoc comments, we > > might want to go the extra mile. > > I'm pleased to see that Markdown `-overview` files work just fine. > > No. There are times to let go of legacy behavior, and even if this is not the > time to remove support for `package.html`, there is no reason to go backwards > and support `package.md`. The preferred replacement for `package.html` has > long been `package-info.java` and you can put Markdown content in that file > with no issues. > > In similar fashion, remember the recent discussion as to whether we should > support `@deprecated` in Markdown comments as marking the declaration as > _deprecated_, even without the `@Deprecated` annotation. The general > consensus was to not persist with that legacy behavior. Okay. - PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1484693923
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v6]
On Mon, 29 Jan 2024 23:34:24 GMT, Jonathan Gibbons wrote:
>> src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java
>> line 215:
>>
>>> 213: case '\n', '\r' -> {
>>> 214: return newString(bp, p);
>>> 215: }
>>
>> Hm... this does not seem to be consistent with `newline` in `nextChar`;
>> should it be consistent?
>
> I think it is OK, isn't it?
>
> In both cases, a newline sequence can begin with `\r` or `\n`.
>
> In this `peekLine` method, we only want the content up to but not including
> the newline, so there is no need to handle the possibility of `\r\n`. In
> `nextChar`, we do want to detect `r` and `\r\n` and treat both as equivalent
> to `\n`.
>
> Or am I missing something?
You don't seem to be missing anything; it's me who was confused.
>> src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java
>> line 1295:
>>
>>> 1293: switch (ch) {
>>> 1294: case ' ' -> indent++;
>>> 1295: case '\t' -> indent = 4;
>>
>> Shouldn't it be like this or it does not matter?
>> ```suggestion
>> case '\t' -> indent += 4;
>
> I did mean `indent = 4`.
> It would not mean `indent +=4` because you would have to take preceding
> character count into account, to round up to a multiple of 4.
> But anyway, it's enough to know the indent is 4 or more, meaning the code is
> looking at an indented code block.
> https://spec.commonmark.org/0.30/#indented-code-blocks
I'm not sure I understood the _take preceding character count into account_
part, but if `indent = 4` is what you meant and having read that section of the
spec, I'm okay with it. Thanks.
>> src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java
>> line 1421:
>>
>>> 1419: case '#', '=', '-', '+', '_', '`', '~' -> {
>>> 1420: String line = peekLine();
>>> 1421: for (LineKind lk : LineKind.values()) {
>>
>> Nothing wrong here, just noting that this is one more way one can depend on
>> an enum constant order. Change it, and a peeked line kind might change too
>> (e.g. `OTHER` matches everything.)
>
> Like it or not, JLS defines that enum members are ordered, and even has an
> example 8.9.3-1 of using the `values` method in an enhanced `for` loop. Any
> change to the order of the members of any enum has the potential to be a
> breaking change and should never be done lightly. Curiously, JLS 13.4.26
> does not say that reordering enum constants may break clients.
>
> Anyway, I added comments to the LineKind enum declaration.
I think I'm still coming to terms with this feature of enum constants: being
order-sensitive.
https://docs.oracle.com/javase/specs/jls/se21/html/jls-13.html#jls-13.4.26 is
concerned with "binary compatibility". What we are talking about here is more
like "behavioural compatibilty" as defined in
https://wiki.openjdk.org/display/csr/Kinds+of+Compatibility. But we digress.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1484581568
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1484593858
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1484607076
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v22]
On Wed, 7 Feb 2024 18:14:24 GMT, Jonathan Gibbons 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 28 commits:
>
> - Merge remote-tracking branch 'upstream/master' into
> 8298405.doclet-markdown-v3 # Please enter a commit message to explain why
> this merge is necessary, # especially if it
>merges an updated upstream into a topic branch. # # Lines starting with
> '#' will be ignored, and an empty message aborts # the commit.
> - add test case contributed by @lahodaj
> - initial fix for source offset problem
> - remove unused imports
> - First pass at remove DocCommentTransformer from the public API.
>
>It is still declared internally, and installed by default, using the
> service-provider mechanism.
>If the standard impl is not available, an identity transformer is used.
> - updates for "first sentence" issues and tests
> - add info about provenance of `jdk.internal.md` module
> - MarkdownTransformer: tweak doc comments
> - MarkdownTransformer: change `Lower.replaceIter` to be `private final`
> - MarkdownTransformer: use suggested text for using streams
> - ... and 18 more: https://git.openjdk.org/jdk/compare/18e24d06...63dd8bf4
src/jdk.internal.md/share/classes/module-info.java line 41:
> 39: // * package and import statements are updated
> 40: // * characters outside the ASCII range are converted to Unicode escapes
> 41: // * @SuppressWarnings("fallthrough") is added to getSetextHeadingLevel
I wonder if it would be better to compile this module without (some) lint
checks. Is it possible?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1483335356
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v7]
On Tue, 30 Jan 2024 21:37:10 GMT, Jonathan Gibbons wrote:
>> src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavaTokenizer.java
>> line 1065:
>>
>>> 1063:
>>> 1064: if (accept('/')) { // (Spec. 3.7)
>>> 1065: if (accept('/')) { // Markdown comment
>>
>> Here and elsewhere in this file: do we need to mention Markdown?
>
> The "M" word appears 10 times in this file. I'll work to convert them to an
> alternate nomenclature, such as "line comment".
I can see that you've managed to remove all of those occurrences nicely; well
done!
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1484624573
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v28]
> 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 incrementally with one additional commit since the last revision: address review feedback - Changes: - all: https://git.openjdk.org/jdk/pull/16388/files - new: https://git.openjdk.org/jdk/pull/16388/files/3b1350b2..91588bc3 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16388=27 - incr: https://webrevs.openjdk.org/?repo=jdk=16388=26-27 Stats: 23 lines in 2 files changed: 4 ins; 18 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/16388.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388 PR: https://git.openjdk.org/jdk/pull/16388
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v6]
On Mon, 15 Jan 2024 12:26:34 GMT, Pavel Rappo wrote: >> Jonathan Gibbons has updated the pull request with a new target base due to >> a merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains seven additional >> commits since the last revision: >> >> - 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/parser/DocCommentParser.java > line 201: > >> 199: } >> 200: newline = true; >> 201: } > > I can see clean LF and CRLF, but no FF; was the latter intentional? > > In any case, we should double-check the generated test output to see if > there's anything unexpected. We've been here, with `\f` before, noting the different handling of `\f` in `javac` and `javadoc`. My recollection is that it was intentional to drop support for `\f`, thus aligning `javac` and `javadoc` behavior, and to normalize handling of the newline sequences, translating them to `\n`. - PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1484669301
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v6]
On Sat, 13 Jan 2024 21:55:06 GMT, Pavel Rappo wrote:
>> Jonathan Gibbons has updated the pull request with a new target base due to
>> a merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains seven additional
>> commits since the last revision:
>>
>> - 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/source/doctree/DocTreeVisitor.java
> line 257:
>
>> 255: *
>> 256: * @implSpec Visits the provided {@code RawTextTree} node
>> 257: * by calling {@code visitOther(node, p)}.
>
> Nit: for consistency with the rest of the file, reorder `@param`-`@return`
> block with the `@implSpec` tag:
>
> Suggestion:
>
> *
> * @implSpec Visits the provided {@code RawTextTree} node
> * by calling {@code visitOther(node, p)}.
> *
> * @param node the node being visited
> * @param p a parameter value
> * @return a result value
Done
> src/jdk.compiler/share/classes/com/sun/tools/javac/api/JavacTrees.java line
> 1080:
>
>> 1078: }
>> 1079:
>> 1080: private String info(FileObject fo) {
>
> This does not seem to be used; is it for debugging? If so, add your `//
> DEBUG` comment.
It probably was used for debugging at some point. The method can be `static`,
meaning it is not specific to this class and could be elsewhere if we wanted to
retain the functionality. Code like this is easy enough to regenerate, so I
will delete this copy for now.
> src/jdk.compiler/share/classes/com/sun/tools/javac/api/JavacTrees.java line
> 1156:
>
>> 1154:
>> 1155: /**
>> 1156: * {@return the {@linkplain ParserFactory} parser factory}.
>
> Suggestion:
>
> * {@return the {@linkplain ParserFactory} parser factory}
done
> src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavaTokenizer.java
> line 1065:
>
>> 1063:
>> 1064: if (accept('/')) { // (Spec. 3.7)
>> 1065: if (accept('/')) { // Markdown comment
>
> I believe that some of the changes in `com/sun/tools/javac/parser` were
> contributed by @JimLaskey. If so, don't forget to add him as a co-author:
> `/contributor add jlaskey`.
did that
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1484652974
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1484650198
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1484651230
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1484651466
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v27]
On Thu, 8 Feb 2024 23:02:18 GMT, Jonathan Gibbons 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 incrementally with one > additional commit since the last revision: > > added test case in TestMarkdown.java for handling of `@deprecated` tag Re: https://github.com/openjdk/jdk/pull/16388#discussion_r1483182361 See https://github.com/openjdk/jdk/pull/17782 - PR Comment: https://git.openjdk.org/jdk/pull/16388#issuecomment-1935112305
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v27]
> 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 incrementally with one additional commit since the last revision: added test case in TestMarkdown.java for handling of `@deprecated` tag - Changes: - all: https://git.openjdk.org/jdk/pull/16388/files - new: https://git.openjdk.org/jdk/pull/16388/files/cb070451..3b1350b2 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16388=26 - incr: https://webrevs.openjdk.org/?repo=jdk=16388=25-26 Stats: 97 lines in 1 file changed: 91 ins; 2 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/16388.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388 PR: https://git.openjdk.org/jdk/pull/16388
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v26]
On Thu, 8 Feb 2024 20:58:24 GMT, Jonathan Gibbons wrote:
>> Ping.
>
> I believe this is substantially covered in line 226-227. See the third call
> to `test` in the following group of lines:
>
>
> for (String src : sources) {
> test(src);
> test(src.replaceAll("@Deprecated", "/** @deprecated */"));
> test(src.replaceAll("deprecated", "notDeprecated2") // change
> class name
> .replaceAll("@Deprecated", "/// @deprecated\n"));
> }
>
>
> * The first call, `test(src)`, runs all the test cases, using the
> `@Deprecated` annotation by default. In these test cases, the name of the
> element encodes whether it is expected that the element is deprecated or not.
>
> * The second call, `test(src.replaceAll("deprecated", "notDeprecated2")`,
> runs the test cases again, after changing the annotation to a traditional
> (`/** ...*/`) comment containing the `@deprecated` tag. This is a
> long-standing call, and tests the legacy behavior of `@deprecated` appearing
> in a traditional doc comment. Effectively, it tests whether a `/**
> @deprecated */` comment has equivalent behavior to a `@Deprecated` comment.
>
> * The third call is new to this PR and the functionality to support Markdown
> comments. It makes two changes to the source for the test cases, before
> running them:
>1. as in the previous test, the annotations are replaced by a comment
> containing `@deprecated` -- except that this time, the comment is a new-style
> `/// ... ` comment instead of a traditional `/** ... */` comment, and ...
>2. because we do not expect `/// @deprecated` to indicate deprecation, we
> need to change the expected result for each test case, which we do by
> changing the element name for the test case. The change is the first call to
> replace to avoid false positives after changing the doc comment. The change
> uses a new name, `notDeprecated2`, in which `notDeprecated` encodes the
> expected deprecation status, and the `2` is to differentiate the declarations
> from other declarations in the case case that already use the name
> `notDeprecated`.
While the underlying mechanism in javac for indicating deprecation is the same
for all, I accept this is primarily a test for generated class files. I can add
a javadoc test for basic behavior of `@deprecated` in Markdown comments.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1483628179
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v26]
> 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 incrementally with one additional commit since the last revision: amend comment in test - Changes: - all: https://git.openjdk.org/jdk/pull/16388/files - new: https://git.openjdk.org/jdk/pull/16388/files/c891fe9a..cb070451 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16388=25 - incr: https://webrevs.openjdk.org/?repo=jdk=16388=24-25 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/16388.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388 PR: https://git.openjdk.org/jdk/pull/16388
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v25]
On Thu, 8 Feb 2024 15:37:06 GMT, Pavel Rappo wrote:
>> Ping. I do believe that it's important to have such a test.
>
> Ping.
I believe this is substantially covered in line 226-227. See the third call to
`test` in the following group of lines:
for (String src : sources) {
test(src);
test(src.replaceAll("@Deprecated", "/** @deprecated */"));
test(src.replaceAll("deprecated", "notDeprecated2") // change
class name
.replaceAll("@Deprecated", "/// @deprecated\n"));
}
* The first call, `test(src)`, runs all the test cases, using the `@Deprecated`
annotation by default. In these test cases, the name of the element encodes
whether it is expected that the element is deprecated or not.
* The second call, `test(src.replaceAll("deprecated", "notDeprecated2")`, runs
the test cases again, after changing the annotation to a traditional (`/**
...*/`) comment containing the `@deprecated` tag. This is a long-standing call,
and tests the legacy behavior of `@deprecated` appearing in a traditional doc
comment. Effectively, it tests whether a `/** @deprecated */` comment has
equivalent behavior to a `@Deprecated` comment.
* The third call is new to this PR and the functionality to support Markdown
comments. It makes two changes to the source for the test cases, before
running them:
1. as in the previous test, the annotations are replaced by a comment
containing `@deprecated` -- except that this time, the comment is a new-style
`/// ... ` comment instead of a traditional `/** ... */` comment, and ...
2. because we do not expect `/// @deprecated` to indicate deprecation, we
need to change the expected result for each test case, which we do by changing
the element name for the test case. The change is the first call to replace to
avoid false positives after changing the doc comment. The change uses a new
name, `notDeprecated2`, in which `notDeprecated` encodes the expected
deprecation status, and the `2` is to differentiate the declarations from other
declarations in the case case that already use the name `notDeprecated`.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1483582480
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v25]
> 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 incrementally with one additional commit since the last revision: improve comments on negative test case - Changes: - all: https://git.openjdk.org/jdk/pull/16388/files - new: https://git.openjdk.org/jdk/pull/16388/files/5fc415b6..c891fe9a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16388=24 - incr: https://webrevs.openjdk.org/?repo=jdk=16388=23-24 Stats: 12 lines in 1 file changed: 6 ins; 0 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/16388.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388 PR: https://git.openjdk.org/jdk/pull/16388
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v24]
> 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 incrementally with one additional commit since the last revision: remove commented-out code from DocCommentTester - Changes: - all: https://git.openjdk.org/jdk/pull/16388/files - new: https://git.openjdk.org/jdk/pull/16388/files/92b5e7a5..5fc415b6 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16388=23 - incr: https://webrevs.openjdk.org/?repo=jdk=16388=22-23 Stats: 11 lines in 1 file changed: 0 ins; 11 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/16388.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388 PR: https://git.openjdk.org/jdk/pull/16388
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v23]
> 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 incrementally with one additional commit since the last revision: clean up imports in ModuleGenerator test file - Changes: - all: https://git.openjdk.org/jdk/pull/16388/files - new: https://git.openjdk.org/jdk/pull/16388/files/63dd8bf4..92b5e7a5 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16388=22 - incr: https://webrevs.openjdk.org/?repo=jdk=16388=21-22 Stats: 18 lines in 1 file changed: 7 ins; 9 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/16388.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388 PR: https://git.openjdk.org/jdk/pull/16388
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v7]
On Fri, 26 Jan 2024 18:14:05 GMT, Pavel Rappo 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.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/CommentUtils.java
> line 629:
>
>> 627: public DocCommentTree parse(URI uri, String text) {
>> 628: return trees.getDocCommentTree(new SimpleJavaFileObject(
>> 629: uri, JavaFileObject.Kind.HTML) {
>
> Was it a bug before?
I would describe it as having been a "latent bug, waiting to happen".
Previously, all file objects were regarded as containing HTML content, and so
there was no need to use the parameter, although maybe it would have been good
to check it.
Now, file objects can be HTML or Markdown, and so we do need to use the
parameter.
In the case here, the method is used on strings specified in command-line
options, which are specified to be in HTML format. Yes, we might want to
change that, but that would be a different RFE separate from the work in this
PR. In that future evolution, I would suggest adding a `JavaFileObject.Kind`
parameter to parse and/or inferring the kind from the tail of the path in the
`uri` parameter.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1483458107
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v7]
On Fri, 26 Jan 2024 18:10:18 GMT, Pavel Rappo 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.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/markup/RawHtml.java
> line 145:
>
>> 143: }
>> 144:
>> 145: Pattern tag = Pattern.compile("<(?[A-Za-z0-9]+)(\\s|>)");
>
> I'm not sure I grok this pattern; what's up with `\\s`?
The match for a tag is one of
* `<` _tag-name_ `>`
* `<` _tag-name_ _whitespace_
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1483444658
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v22]
On Thu, 11 Jan 2024 15:07:33 GMT, Pavel Rappo wrote: >> test/langtools/tools/javac/classfiles/attributes/deprecated/DeprecatedTest.java >> line 26: >> >>> 24: /* >>> 25: * @test >>> 26: * @bug 8042261 8298405 >> >> This comment is not for this line, but for two compiler tests for >> `@deprecated` javadoc tag. It seemed quite contextual place to put it. >> >> Did I miss it, or you are planning to add a javadoc test that verifies that >> `@deprecated` appearing in a `///` comment has no [special meaning] it has >> in classic `/** */` comments? >> >> [special meaning]: >> https://github.com/openjdk/jdk/blob/128363bf3b57dfa05b3807271b47851733c1afb9/src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavaTokenizer.java#L1639-L1653 > > Ping. I do believe that it's important to have such a test. Ping. >> test/langtools/tools/javac/doctree/DocCommentTester.java line 1012: >> >>> 1010: //} >>> 1011: //return null; >>> 1012: //} >> >> Debugging leftover? > > If you want to leave it for debugging you can make it private and uncomment. Ping. >> test/langtools/tools/javac/doctree/MarkdownTest.java line 555: >> >>> 553: // block tags: empty >>> 554: //] >>> 555: //*/ >> >> Just to clarify: it is supposed to be commented out, right? If uncommented, >> this test fails with a slightly different error. > > Please update the DocComment printout in that commented out test: the actual > content is different. It would be nice if the test were passing at least at > the moment of its initial commit. > > Here's what I see locally: > > > Expect: > DocComment[DOC_COMMENT, pos:0 > firstSentence: 1 > Summary[SUMMARY, pos:4 > summary: 1 > Erroneous[ERRONEOUS, pos:14, prefPos:37 > code: compiler.err.dc.unterminated.inline.tag > body: abc_`|_def}|_rest_`more` > ] > ] > body: empty > block tags: empty > ] > > Found: > DocComment[DOC_COMMENT, pos:0 > firstSentence: 1 > Summary[SUMMARY, pos:1 > summary: 1 > Erroneous[ERRONEOUS, pos:11, prefPos:32 > code: compiler.err.dc.unterminated.inline.tag > body: abc_`|def}|rest_`more` > ] > ] > body: empty > block tags: empty > ] Ping. - PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1483179667 PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1483179302 PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1483178073
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v22]
On Wed, 8 Nov 2023 17:17:46 GMT, Pavel Rappo 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 28 commits: >> >> - Merge remote-tracking branch 'upstream/master' into >> 8298405.doclet-markdown-v3 # Please enter a commit message to explain why >> this merge is necessary, # especially if it >>merges an updated upstream into a topic branch. # # Lines starting with >> '#' will be ignored, and an empty message aborts # the commit. >> - add test case contributed by @lahodaj >> - initial fix for source offset problem >> - remove unused imports >> - First pass at remove DocCommentTransformer from the public API. >> >>It is still declared internally, and installed by default, using the >> service-provider mechanism. >>If the standard impl is not available, an identity transformer is used. >> - updates for "first sentence" issues and tests >> - add info about provenance of `jdk.internal.md` module >> - MarkdownTransformer: tweak doc comments >> - MarkdownTransformer: change `Lower.replaceIter` to be `private final` >> - MarkdownTransformer: use suggested text for using streams >> - ... and 18 more: https://git.openjdk.org/jdk/compare/18e24d06...63dd8bf4 > > test/langtools/jdk/javadoc/tool/sampleapi/lib/sampleapi/generator/ModuleGenerator.java > line 49: > >> 47: import sampleapi.util.PoorDocCommentTable; >> 48: >> 49: import static >> com.sun.tools.javac.parser.Tokens.Comment.CommentStyle.JAVADOC; > > To clarify, the change to this file is that you removed two unused imports, > right? Ping. Maybe if it is not essential, we could remove that change, to keep PR more focused. - PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1483182361
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v22]
On Wed, 7 Feb 2024 18:14:24 GMT, Jonathan Gibbons 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 28 commits: > > - Merge remote-tracking branch 'upstream/master' into > 8298405.doclet-markdown-v3 # Please enter a commit message to explain why > this merge is necessary, # especially if it >merges an updated upstream into a topic branch. # # Lines starting with > '#' will be ignored, and an empty message aborts # the commit. > - add test case contributed by @lahodaj > - initial fix for source offset problem > - remove unused imports > - First pass at remove DocCommentTransformer from the public API. > >It is still declared internally, and installed by default, using the > service-provider mechanism. >If the standard impl is not available, an identity transformer is used. > - updates for "first sentence" issues and tests > - add info about provenance of `jdk.internal.md` module > - MarkdownTransformer: tweak doc comments > - MarkdownTransformer: change `Lower.replaceIter` to be `private final` > - MarkdownTransformer: use suggested text for using streams > - ... and 18 more: https://git.openjdk.org/jdk/compare/18e24d06...63dd8bf4 Jon, here are a few reminders and misc comments. - PR Review: https://git.openjdk.org/jdk/pull/16388#pullrequestreview-1846272367
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v7]
On Fri, 19 Jan 2024 18:37:48 GMT, Jonathan Gibbons 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.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlConfiguration.java
line 329:
> 327: if (doclint == null) {
> 328: var trees = docEnv.getDocTrees();
> 329: if (trees.getDocCommentTreeTransformer()== null) {
Suggestion:
if (trees.getDocCommentTreeTransformer() == null) {
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/markup/RawHtml.java
line 145:
> 143: }
> 144:
> 145: Pattern tag = Pattern.compile("<(?[A-Za-z0-9]+)(\\s|>)");
I'm not sure I grok this pattern; what's up with `\\s`?
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/CommentUtils.java
line 629:
> 627: public DocCommentTree parse(URI uri, String text) {
> 628: return trees.getDocCommentTree(new SimpleJavaFileObject(
> 629: uri, JavaFileObject.Kind.HTML) {
Was it a bug before?
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/Env.java line 139:
> 137: this.types = types;
> 138:
> 139: if (this.trees.getDocCommentTreeTransformer()== null) {
Suggestion:
if (this.trees.getDocCommentTreeTransformer() == null) {
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1467988621
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1467977203
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1467980609
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1467982493
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v22]
> 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 28 commits: - Merge remote-tracking branch 'upstream/master' into 8298405.doclet-markdown-v3 # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit. - add test case contributed by @lahodaj - initial fix for source offset problem - remove unused imports - First pass at remove DocCommentTransformer from the public API. It is still declared internally, and installed by default, using the service-provider mechanism. If the standard impl is not available, an identity transformer is used. - updates for "first sentence" issues and tests - add info about provenance of `jdk.internal.md` module - MarkdownTransformer: tweak doc comments - MarkdownTransformer: change `Lower.replaceIter` to be `private final` - MarkdownTransformer: use suggested text for using streams - ... and 18 more: https://git.openjdk.org/jdk/compare/18e24d06...63dd8bf4 - Changes: https://git.openjdk.org/jdk/pull/16388/files Webrev: https://webrevs.openjdk.org/?repo=jdk=16388=21 Stats: 21713 lines in 193 files changed: 21042 ins; 274 del; 397 mod Patch: https://git.openjdk.org/jdk/pull/16388.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388 PR: https://git.openjdk.org/jdk/pull/16388
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v21]
> 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 incrementally with one additional commit since the last revision: add test case contributed by @lahodaj - Changes: - all: https://git.openjdk.org/jdk/pull/16388/files - new: https://git.openjdk.org/jdk/pull/16388/files/7c631688..5710c285 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16388=20 - incr: https://webrevs.openjdk.org/?repo=jdk=16388=19-20 Stats: 100 lines in 1 file changed: 100 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/16388.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388 PR: https://git.openjdk.org/jdk/pull/16388
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v18]
On Wed, 7 Feb 2024 00:16:57 GMT, Jonathan Gibbons wrote: >> There are two cases that need consideration: >> 1. A tree that is not modified during the transformation, as in the test >> case here, so that all nodes should be "as before" >> 2. A tree that is modified during the transformation, raising the issue of >> the positions of the new node and any enclosing node > > I found what I think was the issue, and I've pushed an initial fix for it, > which passes improved versions of existing tests. > I'll follow up with more tests and perhaps include your test case here as > well; thanks for providing it. Update: I've added your test case, which now passes OK. - PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1480724330
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v18]
On Tue, 6 Feb 2024 19:57:45 GMT, Jonathan Gibbons wrote: >> Uugh. Noted. > > There are two cases that need consideration: > 1. A tree that is not modified during the transformation, as in the test case > here, so that all nodes should be "as before" > 2. A tree that is modified during the transformation, raising the issue of > the positions of the new node and any enclosing node I found what I think was the issue, and I've pushed an initial fix for it, which passes improved versions of existing tests. I'll follow up with more tests and perhaps include your test case here as well; thanks for providing it. - PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1480710301
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v20]
> 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 incrementally with one additional commit since the last revision: initial fix for source offset problem - Changes: - all: https://git.openjdk.org/jdk/pull/16388/files - new: https://git.openjdk.org/jdk/pull/16388/files/2a20c74b..7c631688 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16388=19 - incr: https://webrevs.openjdk.org/?repo=jdk=16388=18-19 Stats: 32 lines in 3 files changed: 22 ins; 1 del; 9 mod Patch: https://git.openjdk.org/jdk/pull/16388.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388 PR: https://git.openjdk.org/jdk/pull/16388
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v19]
> 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 incrementally with one additional commit since the last revision: remove unused imports - Changes: - all: https://git.openjdk.org/jdk/pull/16388/files - new: https://git.openjdk.org/jdk/pull/16388/files/203532b2..2a20c74b Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16388=18 - incr: https://webrevs.openjdk.org/?repo=jdk=16388=17-18 Stats: 2 lines in 1 file changed: 0 ins; 2 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/16388.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388 PR: https://git.openjdk.org/jdk/pull/16388
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v18]
On Tue, 6 Feb 2024 19:27:55 GMT, Jonathan Gibbons wrote:
>> src/jdk.internal.md/share/classes/jdk/internal/markdown/MarkdownTransformer.java
>> line 1:
>>
>>> 1: /*
>>
>> This transformer seems to break positions of the `RawTextTree`.
>> For javadoc like:
>>
>> /// Markdown test
>> ///
>> /// @author testAuthor
>>
>> without this transformer, taking the start and end positions of the
>> `RawTextTree` and taking the text between these positions will lead to:
>> `[Markdown test, testAuthor]`. With this transfomer, it leads to `[Markdown
>> test, Markdown t]`, which is clearly suspicious.
>>
>> Testcase:
>>
>> /*
>> * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
>> * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>> *
>> * This code is free software; you can redistribute it and/or modify it
>> * under the terms of the GNU General Public License version 2 only, as
>> * published by the Free Software Foundation.
>> *
>> * This code is distributed in the hope that it will be useful, but WITHOUT
>> * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
>> * version 2 for more details (a copy is included in the LICENSE file that
>> * accompanied this code).
>> *
>> * You should have received a copy of the GNU General Public License version
>> * 2 along with this work; if not, write to the Free Software Foundation,
>> * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
>> *
>> * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
>> * or visit www.oracle.com if you need additional information or have any
>> * questions.
>> */
>>
>> /*
>> * @test
>> * @bug 999
>> * @summary XXX
>> * @run main/othervm --limit-modules jdk.compiler
>> MarkdownTransformerPositionTest
>> * @run main MarkdownTransformerPositionTest
>> */
>>
>> import com.sun.source.doctree.DocCommentTree;
>> import com.sun.source.doctree.RawTextTree;
>> import com.sun.source.tree.*;
>> import com.sun.source.util.*;
>>
>> import java.net.URI;
>> import java.util.*;
>> import javax.lang.model.element.Element;
>> import javax.tools.*;
>>
>>
>> public class MarkdownTransformerPositionTest {
>>
>> public static void main(String... args) throws Exception {
>> String source = """
>> /// Markdown test
>> ///
>> /// @author testAuthor
>> public class Test {
>> ...
>
> Uugh. Noted.
There are two cases that need consideration:
1. A tree that is not modified during the transformation, as in the test case
here, so that all nodes should be "as before"
2. A tree that is modified during the transformation, raising the issue of the
positions of the new node and any enclosing node
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1480467208
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v18]
On Tue, 6 Feb 2024 07:08:13 GMT, Jan Lahoda wrote:
>> Jonathan Gibbons has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> First pass at remove DocCommentTransformer from the public API.
>>
>> It is still declared internally, and installed by default, using the
>> service-provider mechanism.
>> If the standard impl is not available, an identity transformer is used.
>
> src/jdk.internal.md/share/classes/jdk/internal/markdown/MarkdownTransformer.java
> line 1:
>
>> 1: /*
>
> This transformer seems to break positions of the `RawTextTree`.
> For javadoc like:
>
> /// Markdown test
> ///
> /// @author testAuthor
>
> without this transformer, taking the start and end positions of the
> `RawTextTree` and taking the text between these positions will lead to:
> `[Markdown test, testAuthor]`. With this transfomer, it leads to `[Markdown
> test, Markdown t]`, which is clearly suspicious.
>
> Testcase:
>
> /*
> * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
> * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
> *
> * This code is free software; you can redistribute it and/or modify it
> * under the terms of the GNU General Public License version 2 only, as
> * published by the Free Software Foundation.
> *
> * This code is distributed in the hope that it will be useful, but WITHOUT
> * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
> * version 2 for more details (a copy is included in the LICENSE file that
> * accompanied this code).
> *
> * You should have received a copy of the GNU General Public License version
> * 2 along with this work; if not, write to the Free Software Foundation,
> * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
> *
> * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
> * or visit www.oracle.com if you need additional information or have any
> * questions.
> */
>
> /*
> * @test
> * @bug 999
> * @summary XXX
> * @run main/othervm --limit-modules jdk.compiler
> MarkdownTransformerPositionTest
> * @run main MarkdownTransformerPositionTest
> */
>
> import com.sun.source.doctree.DocCommentTree;
> import com.sun.source.doctree.RawTextTree;
> import com.sun.source.tree.*;
> import com.sun.source.util.*;
>
> import java.net.URI;
> import java.util.*;
> import javax.lang.model.element.Element;
> import javax.tools.*;
>
>
> public class MarkdownTransformerPositionTest {
>
> public static void main(String... args) throws Exception {
> String source = """
> /// Markdown test
> ///
> /// @author testAuthor
> public class Test {
> }
> """;
> JavaCompiler comp = ToolProvider.getSystemJavaCompiler();
> JavacTask ...
Uugh. Noted.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1480437320
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v18]
On Tue, 6 Feb 2024 01:36:58 GMT, Jonathan Gibbons 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 incrementally with one
> additional commit since the last revision:
>
> First pass at remove DocCommentTransformer from the public API.
>
> It is still declared internally, and installed by default, using the
> service-provider mechanism.
> If the standard impl is not available, an identity transformer is used.
src/jdk.internal.md/share/classes/jdk/internal/markdown/MarkdownTransformer.java
line 1:
> 1: /*
This transformer seems to break positions of the `RawTextTree`.
For javadoc like:
/// Markdown test
///
/// @author testAuthor
without this transformer, taking the start and end positions of the
`RawTextTree` and taking the text between these positions will lead to:
`[Markdown test, testAuthor]`. With this transfomer, it leads to `[Markdown
test, Markdown t]`, which is clearly suspicious.
Testcase:
/*
* Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
/*
* @test
* @bug 999
* @summary XXX
* @run main/othervm --limit-modules jdk.compiler
MarkdownTransformerPositionTest
* @run main MarkdownTransformerPositionTest
*/
import com.sun.source.doctree.DocCommentTree;
import com.sun.source.doctree.RawTextTree;
import com.sun.source.tree.*;
import com.sun.source.util.*;
import java.net.URI;
import java.util.*;
import javax.lang.model.element.Element;
import javax.tools.*;
public class MarkdownTransformerPositionTest {
public static void main(String... args) throws Exception {
String source = """
/// Markdown test
///
/// @author testAuthor
public class Test {
}
""";
JavaCompiler comp = ToolProvider.getSystemJavaCompiler();
JavacTask task = (JavacTask)comp.getTask(null, null, null, null, null,
Arrays.asList(new JavaSource(source)));
CompilationUnitTree cu = task.parse().iterator().next();
task.analyze();
DocTrees trees = DocTrees.instance(task);
List rawSpans = new ArrayList<>();
TreePath clazzTP = new TreePath(new TreePath(cu),
cu.getTypeDecls().get(0));
Element clazz = trees.getElement(clazzTP);
DocCommentTree docComment = trees.getDocCommentTree(clazz);
new DocTreeScanner() {
@Override
public Void visitRawText(RawTextTree node, Void p) {
int start = (int)
trees.getSourcePositions().getStartPosition(cu, docComment, node);
int end = (int) trees.getSourcePositions().getEndPosition(cu,
docComment, node);
rawSpans.add(source.substring(start, end));
return super.visitRawText(node, p);
}
}.scan(docComment, null);
List expectedRawSpans = List.of("Markdown test", "testAuthor");
if (!expectedRawSpans.equals(rawSpans)) {
throw new AssertionError("Incorrect raw text spans, should be: " +
expectedRawSpans + ", but is: " +
rawSpans);
}
System.err.println("Test result: success, boot modules: " +
ModuleLayer.boot().modules());
}
static class JavaSource extends SimpleJavaFileObject {
private final String source;
public JavaSource(String source) {
super(URI.create("myfo:/Test.java"), JavaFileObject.Kind.SOURCE);
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v18]
> 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 incrementally with one additional commit since the last revision: First pass at remove DocCommentTransformer from the public API. It is still declared internally, and installed by default, using the service-provider mechanism. If the standard impl is not available, an identity transformer is used. - Changes: - all: https://git.openjdk.org/jdk/pull/16388/files - new: https://git.openjdk.org/jdk/pull/16388/files/f086aaab..203532b2 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16388=17 - incr: https://webrevs.openjdk.org/?repo=jdk=16388=16-17 Stats: 191 lines in 11 files changed: 89 ins; 90 del; 12 mod Patch: https://git.openjdk.org/jdk/pull/16388.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388 PR: https://git.openjdk.org/jdk/pull/16388
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v17]
> 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 incrementally with one additional commit since the last revision: updates for "first sentence" issues and tests - Changes: - all: https://git.openjdk.org/jdk/pull/16388/files - new: https://git.openjdk.org/jdk/pull/16388/files/b02d4675..f086aaab Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16388=16 - incr: https://webrevs.openjdk.org/?repo=jdk=16388=15-16 Stats: 133 lines in 2 files changed: 133 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/16388.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388 PR: https://git.openjdk.org/jdk/pull/16388
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v16]
On Tue, 30 Jan 2024 23:07:46 GMT, Jonathan Gibbons wrote:
>> 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.
>
> First up: I do not understand your second sentence: _This means that ///
> comments might not "feel" like Markdown to authors._Please rephrase or
> clarify that.
>
> That aside, there's a big case of chickens and eggs here. The API assumes
> that the first sentence is distinct from the rest of the description, so we
> cannot transform it at that early stage. But generally, the first sentence
> is supposed to be reasonably simple text, and for cases where it is not, you
> can use the `summary` tag to circumvent any use of the sentence breaker.
>
> Bottom line, I do not see any cause for concern at this time.
To add to that, regarding this:
> but one thing that bothers me is that it sees a comment before that comment
> has been transformed
I don't think we should support any transformations (i.e. custom extensions to
Markdown) that would affect the detection of the end of the first sentence. As
of this review, the only custom extension is for enhanced links which simply
infer link definitions for appropriate reference links. If there is anything
that is worth checking, it is the handling of any links, including reference
links, in the first sentence.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1475229987
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v15]
On Thu, 1 Feb 2024 01:12:52 GMT, Jonathan Gibbons 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 incrementally with four > additional commits since the last revision: > > - MarkdownTransformer: tweak doc comments > - MarkdownTransformer: change `Lower.replaceIter` to be `private final` > - MarkdownTransformer: use suggested text for using streams > - remove obsolete debug code > On CommonMark. > > * `jdk.internal.md` contains 133 files, the vast majority of which are from > commonmark-java 0.21.0. According to > https://github.com/commonmark/commonmark-java/releases 0.21.0 is the > latest/current release; good. > Questions: > > * Did we take the tagged commit or mainline at some point after the tagged > commit? If it's the latter, we need to take the tagged version. > * What's the difference between those commonmark-java files in this PR and > official commonmark-java? In other words, how do we adapt them? It would be > nice to have a description of the procedure or a script to update those files. > * `jdk.internal.md` exports packages to `jdk.jshell`. A question for > @lahodaj, who maintains `jdk.jshell`: when do we need to create a new PR > similar to that withdrawn [8299902: Support for MarkDown javadoc in JShell > #11936](https://github.com/openjdk/jdk/pull/11936)? Added comment to `jdk.internal.md` `module-info.java` - PR Comment: https://git.openjdk.org/jdk/pull/16388#issuecomment-1922295907
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v16]
> 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 incrementally with one additional commit since the last revision: add info about provenance of `jdk.internal.md` module - Changes: - all: https://git.openjdk.org/jdk/pull/16388/files - new: https://git.openjdk.org/jdk/pull/16388/files/54d40b20..b02d4675 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16388=15 - incr: https://webrevs.openjdk.org/?repo=jdk=16388=14-15 Stats: 19 lines in 1 file changed: 19 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/16388.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388 PR: https://git.openjdk.org/jdk/pull/16388
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v15]
> 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 incrementally with four additional commits since the last revision: - MarkdownTransformer: tweak doc comments - MarkdownTransformer: change `Lower.replaceIter` to be `private final` - MarkdownTransformer: use suggested text for using streams - remove obsolete debug code - Changes: - all: https://git.openjdk.org/jdk/pull/16388/files - new: https://git.openjdk.org/jdk/pull/16388/files/586daddf..54d40b20 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16388=14 - incr: https://webrevs.openjdk.org/?repo=jdk=16388=13-14 Stats: 26 lines in 2 files changed: 2 ins; 13 del; 11 mod Patch: https://git.openjdk.org/jdk/pull/16388.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388 PR: https://git.openjdk.org/jdk/pull/16388
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v7]
On Fri, 26 Jan 2024 16:33:08 GMT, Pavel Rappo 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 668:
>
>> 666: * U+FFFC characters that were found in the original document.
>> 667: */
>> 668: Iterator replaceIter;
>
> Suggestion:
>
> final Iterator replaceIter;
actually, `private final` ... done
> 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();
done
it's borderline (to me) that it's worthwhile but given that my original code
used streams to create the final array, it sort-of makes sense to go "all in".
> 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}}
done
> 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
done
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1473668136
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1473670108
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1473668952
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1473668548
