On Tue, 9 Mar 2021 16:23:26 GMT, Hannes Wallnöfer <hann...@openjdk.org> wrote:

>> This changes the output for `@see` tags to a `<ul>` structure. A different 
>> CSS style is used if any of the `@see` tag labels are longer than 30 
>> characters or contain a comma. 
>> 
>> The layout for the default CSS style is similar to the one we had so far 
>> with multiple links displayed inline and separated by commas. The CSS style 
>> for lists containing longer labels displays the list in block mode, but 
>> still separated by commas and without list markers. Note that the commas are 
>> generated via CSS in both cases.
>> 
>> As expected, there's a lot of test churn with this change. I converted some 
>> tests to text blocks that were still using old style string concatenation. 
>> In `TestSingletonLists.java` I had to exclude `@see` lists from the 
>> singleton check as javadoc generates a "See also" item for constant field 
>> values.
>
> Hannes Wallnöfer has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Update `@see` tag in class java.security.cert.PKIXRevocationChecker

Nice!  I like the basic change to `TagletWriterImpl`.

Close to being approved, but there are some minor comments inline for 
disciussion or to be addressed.

src/java.base/share/classes/java/security/cert/PKIXRevocationChecker.java line 
97:

> 95:  * @see <a href="http://www.ietf.org/rfc/rfc5280.txt";><i>RFC&nbsp;5280:
> 96:  * Internet X.509 Public Key Infrastructure Certificate and Certificate
> 97:  * Revocation List (CRL) Profile</i></a>

It's not wrong, but the use of `<i>...</i>` is non-standard.  But, while we 
should split the malformed entry, it's arguably out of scope to tweak the style.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/TagletWriterImpl.java
 line 148:

> 146:     // Threshold for length of @see tag label for switching from inline 
> to block layout.
> 147:     private static final int SEE_TAG_MAX_INLINE_LENGTH = 30;
> 148: 

For future consideration, we should look at encoding numbers like this in a 
config file.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/markup/HtmlStyle.java
 line 361:

> 359:      */
> 360:     seeListLong,
> 361: 

The list is alpha-sorted in each group, suggesting that these new entries 
should go before `serializedClassDetails`.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/resources/stylesheet.css
 line 346:

> 344:     content: ", ";
> 345:     white-space: pre-wrap;
> 346: }

Cool! I didn't know you could do that. Is this widely supported?

test/langtools/jdk/javadoc/doclet/testConstructors/TestConstructors.java line 
63:

> 61:                     </ul>
> 62:                     </dd>""",
> 63:                 """

Nice new rendering ... i.e. the general use of `<ul>`

test/langtools/jdk/javadoc/doclet/testLinkOption/pkg/B.java line 64:

> 62:     * Literal IPv6 Addresses in URLs</i></a>
> 63:     * @see <a href="C.html">A nearby file</a>
> 64:     */

Wow, I didn't know we had "bad" examples in the test code.   If there isn't one 
already, can you file an issue for me to updated DocLint with a check for this?

test/langtools/jdk/javadoc/doclet/testSingletonLists/TestSingletonLists.java 
line 270:

> 268:                     if ("see-list".equals(attrs.get("class"))) {
> 269:                         inSeeList = true;
> 270:                     }

This is OK for now, because the lists from `@see` tags cannot be nested. But, I 
had a similar problem in my work yesterday, and so going forward we might want 
a more general solution to track the class of each list on the stack.

test/langtools/jdk/javadoc/doclet/testSingletonLists/TestSingletonLists.java 
line 287:

> 285:                     Map<String,Integer> c = counts.pop();
> 286:                     if (inSeeList) {
> 287:                         // Ignore "Constant Field Values" @see items for 
> final fields created by javadoc

I think the comment is too specific. Won't the problem also be triggered for 
classes with a single `@see` and no constant values?  It m might also be 
triggered on serializable classes, with the link to the `Serialized Form`.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2894

Reply via email to