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