On Wed, 4 Nov 2020 20:57:06 GMT, Jonathan Gibbons <j...@openjdk.org> wrote:
>> This introduces support for a new `@spec` tag that can be used as either an
>> inline tag or as a block tag. It is used to identify references to external
>> specifications, in such a way that the references can be collected together
>> on a new summary page, called "Other Specifications", available from either
>> the static INDEX page or the interactive search box.
>>
>> As an inline tag, the format is `{@spec url label}`, which is roughly
>> translated to `<a href="url">label</a>` in the generated docs.
>>
>> As a block tag, the format is simply
>>
>> @spec url label
>>
>> which is handled in a manner analogous to
>>
>> @see <a href="url">label</a>
>>
>> The tag is notable for being the first standard/supported tag that can be
>> used as either an inline or block tag. (We have had support for bimodal
>> non-standard/custom tags for a while, such as `{@jls}` and `{@jvms}`.) To
>> support bimodal standard tags, some changes to `DocCommentParser` are
>> incorporated here.
>>
>> This change is only the _support_ for the new tag; it does _not_ include
>> any changes to API docs to _use_ the new tag.
>
> Jonathan Gibbons has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now contains 11 commits:
>
> - Fix merge issues; review feedback
> - Merge with master
> - allow rich content in createAnchorAndSearchIndex
> - update Docs.gmk to stop disabling spec tag
> - fix TestSpecTag.testEncodedURI
> - fix tests
> - remove support to workaround legacy @spec tag
> - Merge remote-tracking branch 'upstream/master' into new-spec-tag
> - fix trailing whitespace in test
> - temporarily allow existing legacy usage `@spec JPMS` `@spec jsr-51`
> - ... and 1 more:
> https://git.openjdk.java.net/jdk/compare/804bd725...ed5512d9
1. Thanks for incorporating my previous offline feedback.
2. Since Hannes and Erik seem to have looked at everything else, I looked
mostly at changes in `src/jdk.compiler/share/classes/com/sun/source/**`, which
are good!
3. There should be a corresponding but separate change to "Documentation
Comment Specification for the Standard Doclet".
4. Can we use this new `@since` tag to refer to the spec at
`com/sun/tools/javac/parser/DocCommentParser.java:1116`?
5. Should we specify in `com.sun.source.doctree.SpecTree` that both `url` and
`label` parts are mandatory?
6. `DCSpec extends DCEndPosTree`, sigh... Although that is not a public API,
this design suggests we could improve that abstraction sometime later.
src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java
line 1104:
> 1102:
> 1103: DCTree parse(int pos, Kind kind) throws ParseException {
> 1104: if (kind != this.kind && this.kind != Kind.EITHER) {
This condition looks right, but shouldn't we add another one to guard against
accidental passing of `kind == EITHER`?
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/markup/HtmlTree.java
line 306:
> 304: * The {@code ref} argument will be URL-encoded for use as the
> attribute value.
> 305: *
> 306: * @param ref the value for the {@code href} attribute}
Since you are here, could you please remove that trailing `}`?
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/markup/HtmlTree.java
line 322:
> 320: * {@link URI#toASCIIString() converted} to ASCII for use as the
> attribute value.
> 321: *
> 322: * @param ref the value for the {@code href} attribute}
One more, perhaps copy-pasted, trailing `}`.
test/langtools/tools/javac/diags/examples/NoLabel.java line 2:
> 1: /*
> 2: * Copyright (c) 2012, 2020 Oracle and/or its affiliates. All rights
> reserved.
Should be:
`Copyright (c) 2012, 2020, Oracle and/or its affiliates. All rights reserved.`
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/TagletWriterImpl.java
line 266:
> 264: }
> 265:
> 266: String textOf(List<? extends DocTree> trees) {
I wonder if this method should work **recursively** rather than shallowly.
Consider the following example:
@spec http://example.com Some code: {@code text}
I reckon we'll end up with only `Some code: `.
src/jdk.compiler/share/classes/com/sun/tools/javac/tree/DCTree.java line 81:
> 79: return list.stream().allMatch(DCTree::isBlank);
> 80: }
> 81:
Adding these two methods **here** might be overkill.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/taglets/TagletManager.java
line 737:
> 735:
> 736: for (Taglet t : taglets) {
> 737: String name = t.isBlockTag() ? "@" + t.getName() : "{@" +
> t.getName() + "}";
Out of curiosity, why did you flip that?
-------------
Changes requested by prappo (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/790
