On 27 Feb 2020, at 21:11, Jonathan Gibbons <[email protected]> wrote:
Please review the next round of update for definition lists in the generated
docs.
The primary focus of this round is to ensure that all <dl> elements have the
class attribute set.
In practice, there are only two kinds of definition lists:
• 1. the kind we now know as "notes", (where the <dt> elements are bold and
the <dd> elements not indented)
• 2. the entries in the index files.These previously did not have a class attribute set,
and so used the default style for definition lists; these lists now have the class attribute set to
"index", although no specific style is defined for "index" at this point, and
so they continue to use the default style for definition lists.
By defining the class attribute on all definition lists, we set up being able
to simplify the stylesheet (in the next webrev) to remove contentContainer
nodes and friends.
For the record, there seems to be at least one other type, "nameValue",
used in serialized-form.html.
The edits to the source files are generally simple, and mostly consist of using
new/improved factory methods for <dl> nodes that take a style parameter.
"Naked" calls gave way to convenience static methods that force you to use
"style", good.
Before:
HtmlTree.DL(paramInfo).setStyle(HtmlStyle.notes)
or even more mouthful
new HtmlTree(HtmlTag.DL).setStyle(HtmlStyle.notes)
After:
HtmlTree.DL(HtmlStyle.notes, paramInfo)
I have also done some local reordering of the code to setup for another cleanup
down the road. Previously, the code pattern was
• a. create the child <dt> node
• b. create the parent <dl> node containing the <dt> node
• c. create the child <dd> node and add it to the <dl> node
This was in response to a concern a long, long time ago about creating and
generating empty nodes: so the code style was to try and create nodes with
content, even though it led to the confusing sequence I just described.
Huh, I would've never guessed!
The new sequence is:
• a. create the parent <dl> node
• b. create the child <dt> node and add it to the <dl> node
• c. create the child <dd> node and add it to the <dl> node
The reordering sets up the option to introduce and use chained method calls in
future, to simplify the code even more, but that was too much of a change to
add in this changeset.
Thanks, it looks neat.
There's a bunch of updates I could (and want to) make to the comments
throughout the HtmlTree class, but I think they should be done separately so
that the file has a consistent comment style.
+1 for splitting the cleanup.
One other item that became clear in this work is the use of "singleton lists" in which there are a number of
instances where the <dl> node is always created with a single <dt>/<dd> pair. Moreover, a sequence of
these singleton lists may occur, depending on the characteristics of the class being documented. This changeset does _not_
address that issue, which should be handled separately. This is the definition-list equivalent of the "sequence of
singleton lists" that we fixed for <ul> lists a while back!
The tests are generally just updated to expect the class attribute to be set in
<dl> elements.
-- Jon
JBS: https://bugs.openjdk.java.net/browse/JDK-8240136
Webrev: http://cr.openjdk.java.net/~jjg/8240136/webrev.00/index.html
As always, thanks for improvements on the go such as fixing typos, broken
indentation, etc.
If it is still possible, could you please fix the spelling in the name of the
"testExternalOverridenMethod" folder? I think we missed that yesterday while
reviewing (now pushed) http://hg.openjdk.java.net/jdk/jdk/rev/52367bbd4bd4.
Looks good to me.
-Pavel
