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