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