RFR: JDK-6251738: Want a top-level summary page that itemizes all spec documents referenced from javadocs (OEM spec) [v2]

[Pavel Rappo]

On Wed, 4 Nov 2020 20:57:06 GMT, Jonathan Gibbons <jjg at 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