-
Notifications
You must be signed in to change notification settings - Fork 5.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
JDK-8200337: Generalize see and link tags for user-defined anchors #10395
Conversation
👋 Welcome back hannesw! A progress list of the required criteria for merging this PR into |
Generate an error if no label is provided. Now is the time to set the rules. |
Webrevs
|
There's cognitive dissonance here.
At a minimum, we should give a warning, or even an error. We can use the |
src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java
Show resolved
Hide resolved
src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java
Outdated
Show resolved
Hide resolved
src/jdk.compiler/share/classes/com/sun/tools/javac/parser/ReferenceParser.java
Outdated
Show resolved
Hide resolved
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/TagletWriterImpl.java
Outdated
Show resolved
Hide resolved
(repeating? after first message was lost...?) I primarily reviewed the Removing the If anything you should honor the |
The |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the new support in the doc-comment-parser for kinds of references. While docent may be able to make better checks, it can also be disabled, so it is good to catch things early where that is possible.
I noticed various additional new/improved doc comments as well: nice!
Check the following question in TestSeeLinkAnchor.java
test/langtools/jdk/javadoc/doclet/testSeeLinkAnchor/TestSeeLinkAnchor.java
Show resolved
Hide resolved
@hns 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:
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 596 new commits pushed to the
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 |
/integrate |
Going to push as commit 5622b09.
Your commit was automatically rebased without conflicts. |
Please review a a new feature to allow
@link
,@linkplain
and@see
tags to link to arbitrary URI fragments in the generated documentation (including in auxiliarydoc-files
documentation).The changes in module
jdk.compiler
are mostly cleanup changes retained from earlier versions of the patch. The current proposed version uses a very simple change inReferenceParser
to avoid parsing the member name section of the reference when a non-member fragment is encountered.The implementation introduces a new form of reference with a double hash mark (
##
) separator. This is a change from the previous implementation which also auto-recognized URI fragments and documentation paths by looking for-
characters which are not allowed in member names. This feature was removed upon further consideration because it makes the feature more complex and less recognizable.Links to auxiliary documentation files follow the same rules. They are recognized by looking for
/
characters in the fragment name. This means that ordinaryid
attribute values must not contain/
, while auxiliary file paths must contain a/
character. Both restrictions should be easy to sustain.One thing that is difficult for this feature is to provide a good link label if no label is supplied in the tag. In contrast to program element names a fragment name does usually not make a good human readable name. The solution is to use the fragment name as default label text. I expect that the feature will usually be used with a user provided label.
Progress
Issues
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk pull/10395/head:pull/10395
$ git checkout pull/10395
Update a local copy of the PR:
$ git checkout pull/10395
$ git pull https://git.openjdk.org/jdk pull/10395/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 10395
View PR using the GUI difftool:
$ git pr show -t 10395
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/10395.diff