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 [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 [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 [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&pr=16388&range=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
