RFR: JDK-8240697: convert builders to high-level Content blocks
Pavel Rappo
pavel.rappo at oracle.com
Mon Mar 9 11:38:42 UTC 2020
1. BodyContents, Head, Table, and TableHeader are now of type Content.
2. Navigation.Position.{TOP, BOTTOM} is used instead of `boolean top`.
PageMode's ALL_CLASSES, CONSTANT_VALUES, DOC_FILE, SERIALIZED_FORM,
SYSTEM_PROPERTIES, etc. are changed to use _ (underscore) in their names.
3. Many conversion calls to toContent() have gone.
4. Updated documentation.
5. Could HtmlTree.java:197 use a covariant parameter, List<? extends Content> list?
This change looks good to me.
Notes
=====
None of the below items need to be addressed in this change.
1. When I see all that code that deals with I/O, it makes me wonder if we could
somehow get rid of (checked) IOException. The current code is unfriendly to
Stream API, which otherwise could be used beneficially in many places across
the jdk.javadoc codebase.
2. Is it just me, or it is indeed hard to comprehend the structure of the HTML
being composed in some of the places? Maybe we could think of slightly bending
the code formatting rules for the sake of readability where practical?
Before:
HtmlTree flexHeader = HtmlTree.HEADER()
.setStyle(HtmlStyle.flexHeader)
.add(header);
HtmlTree flexContent = HtmlTree.DIV(HtmlStyle.flexContent)
.add(HtmlTree.MAIN().add(mainContents))
.add(footer);
return HtmlTree.DIV(HtmlStyle.flexBox)
.add(flexHeader)
.add(flexContent);
After:
return DIV(HtmlStyle.flexBox,
HEADER(HtmlStyle.flexHeader
header
),
DIV(HtmlStyle.flexContent,
MAIN(mainContents),
footer
)
);
Before:
Content li = HtmlTree.LI(HtmlStyle.blockList, table);
Content ul = HtmlTree.UL(HtmlStyle.blockList, li);
contentTree.add(ul);
After:
contentTree.add(
UL(HtmlStyle.blockList,
LI(HtmlStyle.blockList, table)
)
);
One added benefit of structuring HTML generation like that is that the resulting
code is less error-prone. When no intermediate reference is created, it is much
harder to pass a single "Content" into multiple places (which would be a bug).
-Pavel
> On 7 Mar 2020, at 02:58, Jonathan Gibbons <jonathan.gibbons at oracle.com> wrote:
>
> Please review a moderately simple changeset to convert some HTML builders to be high-level Content blocks instead.
> The conceptual change is (almost) as simple as making the objects be subtypes of Content, and implementing the `write` method, so that clients can simply add instances to enclosing trees, without having to call .toContent().
>
> This is in line with creating creating additional high-level builders later on that encapsulate code that is currently distributed in many places in the Writer(Impl) classes.
>
> In adding, the Navigation class is moved from the formats.html.markup package (where it is out of place) to the formats..html package (where it better belongs.) It was intended to also convert this "builder" into a Content block, like the other builders, but this is an unusual builder in that the builder is used to create two similar but different nav bars ... for the top and bottom of the page. This was obscurely indicated by a boolean parameter to Navigation.getContent, which is replaced by using the already-existing enum Navigation.Position { TOP, BOTTOM }. Various additional cleanup is done on Navigation, but this is not the time to do a more substantial overhaul.
>
> This is all internal cleanup. There is no change to any generated code, and no tests need to be created or updated.
>
> -- Jon
>
>
> JBS: https://bugs.openjdk.java.net/browse/JDK-8240697
> Webrev: http://cr.openjdk.java.net/~jjg/8240697/webrev.00/
>
More information about the javadoc-dev
mailing list