jdk JDK-8298405: Implement JEP 467: Markdown Documentation Comments

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

Progress

[ ] Change must be properly reviewed (1 review required, with at least 1 Reviewer)
[x] Change must not contain extraneous whitespace
[x] Commit message must refer to an issue
[ ] Change requires CSR request JDK-8298687 to be approved

Issues

JDK-8298405: Implement JEP 467: Markdown Documentation Comments (Enhancement - P3)
JDK-8298687: Support Markdown in the standard doclet (CSR)

Contributors

Jim Laskey <[email protected]>

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388
$ git checkout pull/16388

Update a local copy of the PR:
$ git checkout pull/16388
$ git pull https://git.openjdk.org/jdk.git pull/16388/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 16388

View PR using the GUI difftool:
$ git pr show -t 16388

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/16388.diff

Webrev

Link to Webrev Comment

Oct 26 '23 23:10 jonathan-gibbons

:wave: Welcome back jjg! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

Oct 26 '23 23:10 bridgekeeper[bot]

@jonathan-gibbons The following labels will be automatically applied to this pull request:

build
compiler
javadoc

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

Oct 26 '23 23:10 openjdk[bot]

Webrevs

Nov 06 '23 19:11 mlbridge[bot]

@jonathan-gibbons This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

Dec 20 '23 12:12 bridgekeeper[bot]

@jonathan-gibbons, please sync this PR with mainline. As for Skara bots, this comment will hopefully deter them for another 4 weeks.

Jan 08 '24 13:01 pavelrappo

Jon, please sync this PR with mainline to resolve conflicts.

Jan 19 '24 10:01 pavelrappo

/contributor add @JimLaskey

Jan 30 '24 22:01 jonathan-gibbons

@jonathan-gibbons Contributor Jim Laskey <[email protected]> successfully added.

Jan 30 '24 22:01 openjdk[bot]

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?

Added comment to jdk.internal.md module-info.java

Feb 01 '24 21:02 jonathan-gibbons

Re: https://github.com/openjdk/jdk/pull/16388#discussion_r1483182361 See https://github.com/openjdk/jdk/pull/17782

Feb 08 '24 23:02 jonathan-gibbons

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!
///
/// <p>
/// @param <T> hello
/// </p>
///
public class C<T> { }

Mar 04 '24 14:03 pavelrappo

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 <T> hello
/// 
///
public class C<T> { }

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
</p>

(Note, the content includes .)

Mar 04 '24 15:03 pavelrappo

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 <T> hello
/// 
///
public class C<T> { }
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 .)

The Markdown aspect of this is behaving about as well as can be expected. But the generated form is certainly confusing and can be considered a less-than-optimal presentation of a paragraph within a definition list. You can see the same behavior in a traditional comment with @param <T> hello. Ideally, we could/should tweak the CSS for this situation, in a separate changeset.

Mar 05 '24 19:03 jonathan-gibbons

@jonathan-gibbons this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout 8298405.doclet-markdown-v3
git fetch https://git.openjdk.org/jdk.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

Mar 11 '24 21:03 openjdk[bot]

@jonathan-gibbons This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8298405: Implement JEP 467: Markdown Documentation Comments
8329296: Update Elements for '///' documentation comments

Co-authored-by: Jim Laskey <[email protected]>
Reviewed-by: prappo, darcy, hannesw

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 31 new commits pushed to the master branch:

0b0445be2833286b4eace698b91a658de3e7608b: 8331724: Refactor j.l.constant implementation to internal package
d84a8fd8762fe9448e73d75ec9dc8c4876b1a709: 8332327: Return _methods_jmethod_ids field back in VMStructs
f1ce9b0ecce9b506f5bf7a66fcf03c93b9ae8fed: 8331557: Serial: Refactor SerialHeap::do_collection
14198f502f0a721e479adc754a2c7d94b665fbe6: 8329653: JLILaunchTest fails on AIX after JDK-8329131
ae999eae7e61072ad964a43f622fa930ce1179f7: 8129418: JShell: better highlighting of errors in imports on demand
6422efa3c7917525a879e80657ca4dcfb6d67514: 8332394: Add friendly output when @IR rule missing value
9160ef8b9d9f2c87ca6df08d85dad4271085f0ac: 8332237: [nmt] Remove the need for ThreadStackTracker::track_as_vm()
7c750fd95b83d0a93b0cce681dcfbbae1f220fdd: 8331746: Create a test to verify that the cmm id is not ignored
de57d4b2e0fe3add0ef09945b34ddd0b67bbfa2b: 8332257: Shenandoah: Move evacuation methods to implementation file
da9c23ace9bdf398d811a88ed137217dd3167231: 8325384: sun/security/ssl/SSLSessionImpl/ResumptionUpdateBoundValues.java failing intermittently when main thread is a virtual thread
... and 21 more: https://git.openjdk.org/jdk/compare/61aff6db15d5bdda77427af5ce34d0fe43373197...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

Mar 13 '24 20:03 openjdk[bot]

⚠️ @jonathan-gibbons This pull request contains merges that bring in commits not present in the target repository. Since this is not a "merge style" pull request, these changes will be squashed when this pull request in integrated. If this is your intention, then please ignore this message. If you want to preserve the commit structure, you must change the title of this pull request to Merge <project>:<branch> where <project> is the name of another project in the OpenJDK organization (for example Merge jdk:master).

Mar 21 '24 21:03 openjdk[bot]

/issue add jdk-8303766

Apr 08 '24 20:04 jonathan-gibbons

@jonathan-gibbons Adding additional issue to issue list: 8303766: Support /// for documentation comments.

Apr 08 '24 20:04 openjdk[bot]

/issue remove jdk-8303766

Apr 08 '24 20:04 jonathan-gibbons

@jonathan-gibbons Removing additional issue from issue list: 8303766.

Apr 08 '24 20:04 openjdk[bot]

/issue add JDK-8329296

Apr 08 '24 20:04 jonathan-gibbons

@jonathan-gibbons Adding additional issue to issue list: 8329296: Update Elements for '///' documentation comments.

Apr 08 '24 20:04 openjdk[bot]

There should be some quick testing of the new default method on Elements using the VacuousElements implementation; see test/langtools/tools/javac/processing/model/util/elements for some examples.

Apr 12 '24 17:04 jddarcy

There should be some quick testing of the new default method on Elements using the VacuousElements implementation; see test/langtools/tools/javac/processing/model/util/elements for some examples.

@jddarcy I've reorganized the tests in this area a bit, and moved the testing for DocCommentKind to a new test, which includes testing the default method, using VacuousElements (nice trick, that, by the way.). Although now renamed, the older TestGetDocComments is now closer to the original form, with the testing for getDocCommentKind being moved to a new test class.

May 01 '24 20:05 jonathan-gibbons

There should be some quick testing of the new default method on Elements using the VacuousElements implementation; see test/langtools/tools/javac/processing/model/util/elements for some examples.

@jddarcy I've reorganized the tests in this area a bit, and moved the testing for DocCommentKind to a new test, which includes testing the default method, using VacuousElements (nice trick, that, by the way.). Although now renamed, the older TestGetDocComments is now closer to the original form, with the testing for getDocCommentKind being moved to a new test class.

Creating one VacuousElements was preferable to creating one each time a new default method was added :-)

May 07 '24 21:05 jddarcy

/reviewers 2 reviewer

May 07 '24 21:05 jddarcy

@jddarcy The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 2 Reviewers).

May 07 '24 21:05 openjdk[bot]

I think we should add a test to verify that if --disable-line-doc-comments is specified, no /// dangling comments are reported.

May 15 '24 10:05 pavelrappo

I think we should add a test to verify that if --disable-line-doc-comments is specified, no /// dangling comments are reported.

Added more tests for dangling doc comments.

Note we cannot currently use -Xlint:dangling-doc-comments in javadoc itself, so the new tests are part of the javac set of tests.

May 15 '24 21:05 jonathan-gibbons

A meta question about the JEP: Why don't Javadoc features (like snippets and markdown comments) don't go through preview, while Compiler features mostly do? Is there any bar for previews?

May 16 '24 13:05 liach

jdk jdk copied to clipboard

JDK-8298405: Implement JEP 467: Markdown Documentation Comments

Progress

Issues

Contributors

Reviewing

Webrev

Webrevs

jdk
jdk copied to clipboard