Skip to content
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-8262992: Improve @see output #2894

Closed
wants to merge 6 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -486,14 +486,14 @@
* @since 1.4
*
* @see <a href="http://www.ietf.org/rfc/rfc2279.txt"><i>RFC&nbsp;2279: UTF-8, a
* transformation format of ISO 10646</i></a>, <br><a
* href="http://www.ietf.org/rfc/rfc2373.txt"><i>RFC&nbsp;2373: IPv6 Addressing
* Architecture</i></a>, <br><a
* href="http://www.ietf.org/rfc/rfc2396.txt"><i>RFC&nbsp;2396: Uniform
* Resource Identifiers (URI): Generic Syntax</i></a>, <br><a
* href="http://www.ietf.org/rfc/rfc2732.txt"><i>RFC&nbsp;2732: Format for
* Literal IPv6 Addresses in URLs</i></a>, <br><a
* href="URISyntaxException.html">URISyntaxException</a>
* transformation format of ISO 10646</i></a>
* @see <a href="http://www.ietf.org/rfc/rfc2373.txt"><i>RFC&nbsp;2373: IPv6 Addressing
* Architecture</i></a>
* @see <a href="http://www.ietf.org/rfc/rfc2396.txt"><i>RFC&nbsp;2396: Uniform
* Resource Identifiers (URI): Generic Syntax</i></a>
* @see <a href="http://www.ietf.org/rfc/rfc2732.txt"><i>RFC&nbsp;2732: Format for
* Literal IPv6 Addresses in URLs</i></a>
* @see <a href="URISyntaxException.html">URISyntaxException</a>
*/

public final class URI
@@ -91,10 +91,10 @@
*
* @see <a href="http://www.ietf.org/rfc/rfc2560.txt"><i>RFC&nbsp;2560: X.509
* Internet Public Key Infrastructure Online Certificate Status Protocol -
* OCSP</i></a>, <br><a
* href="http://www.ietf.org/rfc/rfc5280.txt"><i>RFC&nbsp;5280: Internet X.509
* Public Key Infrastructure Certificate and Certificate Revocation List (CRL)
* Profile</i></a>
* OCSP</i></a>
* @see <a href="http://www.ietf.org/rfc/rfc5280.txt"><i>RFC&nbsp;5280:
* Internet X.509 Public Key Infrastructure Certificate and Certificate
* Revocation List (CRL) Profile</i></a>
Copy link
Contributor

@jonathan-gibbons jonathan-gibbons Mar 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

*/
public abstract class PKIXRevocationChecker extends PKIXCertPathChecker {
private URI ocspResponder;
@@ -25,6 +25,7 @@

package jdk.javadoc.internal.doclets.formats.html;

import java.util.ArrayList;
import java.util.EnumSet;
import java.util.List;
import java.util.Set;
@@ -53,6 +54,7 @@
import jdk.javadoc.internal.doclets.formats.html.markup.HtmlStyle;
import jdk.javadoc.internal.doclets.formats.html.markup.HtmlTree;
import jdk.javadoc.internal.doclets.formats.html.markup.RawHtml;
import jdk.javadoc.internal.doclets.formats.html.markup.TagName;
import jdk.javadoc.internal.doclets.formats.html.markup.Text;
import jdk.javadoc.internal.doclets.toolkit.BaseConfiguration;
import jdk.javadoc.internal.doclets.toolkit.Content;
@@ -141,6 +143,9 @@ Context within(DocTree tree) {
private final Contents contents;
private final Context context;

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

Copy link
Contributor

@jonathan-gibbons jonathan-gibbons Mar 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

/**
* Creates a taglet writer.
*
@@ -310,48 +315,47 @@ public Content returnTagOutput(Element element, ReturnTree returnTag, boolean in

@Override
public Content seeTagOutput(Element holder, List<? extends SeeTree> seeTags) {
ContentBuilder body = new ContentBuilder();
List<Content> links = new ArrayList<>();
for (DocTree dt : seeTags) {
appendSeparatorIfNotEmpty(body);
body.add(htmlWriter.seeTagToContent(holder, dt, context.within(dt)));
links.add(htmlWriter.seeTagToContent(holder, dt, context.within(dt)));
}
if (utils.isVariableElement(holder) && ((VariableElement)holder).getConstantValue() != null &&
htmlWriter instanceof ClassWriterImpl) {
//Automatically add link to constant values page for constant fields.
appendSeparatorIfNotEmpty(body);
DocPath constantsPath =
htmlWriter.pathToRoot.resolve(DocPaths.CONSTANT_VALUES);
String whichConstant =
((ClassWriterImpl) htmlWriter).getTypeElement().getQualifiedName() + "." +
utils.getSimpleName(holder);
DocLink link = constantsPath.fragment(whichConstant);
body.add(htmlWriter.links.createLink(link,
links.add(htmlWriter.links.createLink(link,
Text.of(resources.getText("doclet.Constants_Summary"))));
}
if (utils.isClass(holder) && utils.isSerializable((TypeElement)holder)) {
//Automatically add link to serialized form page for serializable classes.
if (SerializedFormBuilder.serialInclude(utils, holder) &&
SerializedFormBuilder.serialInclude(utils, utils.containingPackage(holder))) {
appendSeparatorIfNotEmpty(body);
DocPath serialPath = htmlWriter.pathToRoot.resolve(DocPaths.SERIALIZED_FORM);
DocLink link = serialPath.fragment(utils.getFullyQualifiedName(holder));
body.add(htmlWriter.links.createLink(link,
links.add(htmlWriter.links.createLink(link,
Text.of(resources.getText("doclet.Serialized_Form"))));
}
}
if (body.isEmpty())
return body;
if (links.isEmpty()) {
return Text.EMPTY;
}
// Use a different style if any link label is longer than 30 chars or contains commas.
boolean hasLongLabels = links.stream()
.anyMatch(c -> c.charCount() > SEE_TAG_MAX_INLINE_LENGTH || c.toString().contains(","));
HtmlTree seeList = new HtmlTree(TagName.UL)
.setStyle(hasLongLabels ? HtmlStyle.seeListLong : HtmlStyle.seeList);
links.stream().filter(Content::isValid).forEach(item -> {
seeList.add(HtmlTree.LI(item));
});

return new ContentBuilder(
HtmlTree.DT(contents.seeAlso),
HtmlTree.DD(body));
}

private void appendSeparatorIfNotEmpty(ContentBuilder body) {
if (!body.isEmpty()) {
body.add(", ");
body.add(DocletConstants.NL);
}
HtmlTree.DD(seeList));
}

@Override
@@ -339,6 +339,17 @@ public enum HtmlStyle {
*/
propertyDetails,

/**
* The class for the list containing the {@code @see} tags of an element.
*/
seeList,

/**
* The class for the list containing the {@code @see} tags of an element
* when some of the tags have longer labels.
*/
seeListLong,

Copy link
Contributor

@jonathan-gibbons jonathan-gibbons Mar 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

/**
* The class for a {@code section} element containing details of the
* serialized form of an element, on the "Serialized Form" page.
@@ -332,6 +332,18 @@ ul.summary-list > li {
margin-top:0;
margin-bottom:1px;
}
ul.see-list, ul.see-list-long {
padding-left: 0;
list-style: none;
}
ul.see-list li {
display: inline;
}
ul.see-list li:not(:last-child):after,
ul.see-list-long li:not(:last-child):after {
content: ", ";
white-space: pre-wrap;
}
Copy link
Contributor

@jonathan-gibbons jonathan-gibbons Mar 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

@hns hns Mar 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to MDN documentation all used pseudo-classes/elements are supported in all browsers including Internet Explorer 9.

https://developer.mozilla.org/en-US/docs/Web/CSS/:not
https://developer.mozilla.org/en-US/docs/Web/CSS/:last-child
https://developer.mozilla.org/en-US/docs/Web/CSS/::after

I have tested the pages on my old laptop using IE 11 and Edge 44. These are the oldest browsers I have access to, they both rendered both list styles correctly.

/*
* Styles for tables.
*/
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2013, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2013, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -50,12 +50,16 @@ public void test() {
checkOutput("pkg1/Outer.html", true,
"""
<dt>See Also:</dt>
<dd><a href="Outer.Inner.html#%3Cinit%3E()"><code>Inner()</code></a>,\s
<a href="Outer.Inner.html#%3Cinit%3E(int)"><code>Inner(int)</code></a>,\s
<a href="Outer.Inner.NestedInner.html#%3Cinit%3E()"><code>NestedInner()</code></a>,\s
<a href="Outer.Inner.NestedInner.html#%3Cinit%3E(int)"><code>NestedInner(int)</code></a>,\s
<a href="#%3Cinit%3E()"><code>Outer()</code></a>,\s
<a href="#%3Cinit%3E(int)"><code>Outer(int)</code></a></dd>""",
<dd>
<ul class="see-list">
<li><a href="Outer.Inner.html#%3Cinit%3E()"><code>Inner()</code></a></li>
<li><a href="Outer.Inner.html#%3Cinit%3E(int)"><code>Inner(int)</code></a></li>
<li><a href="Outer.Inner.NestedInner.html#%3Cinit%3E()"><code>NestedInner()</code></a></li>
<li><a href="Outer.Inner.NestedInner.html#%3Cinit%3E(int)"><code>NestedInner(int)</code></a></li>
<li><a href="#%3Cinit%3E()"><code>Outer()</code></a></li>
<li><a href="#%3Cinit%3E(int)"><code>Outer(int)</code></a></li>
</ul>
</dd>""",
"""
Copy link
Contributor

@jonathan-gibbons jonathan-gibbons Mar 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Link: <a href="Outer.Inner.html#%3Cinit%3E()"><code>Inner()</code></a>, <a href=\
"#%3Cinit%3E(int)"><code>Outer(int)</code></a>, <a href="Outer.Inner.NestedInner\