RFR: JDK-8190295: Introduce a new Table builder class
Jonathan Gibbons
jonathan.gibbons at oracle.com
Tue Nov 7 02:17:28 UTC 2017
Kumar,
Thank you for your feedback. I've made the changes you suggested, and
included the proposal
as part of a series of related changes to generally cleanup more of the
same.
The overall webrev is now at
http://cr.openjdk.java.net/~jjg/8190295-etc/webrev.00/
which contains a series of webrevs as follows:
https://bugs.openjdk.java.net/browse/JDK-8190295
http://cr.openjdk.java.net/~jjg/8190295-etc/webrev.00/webrev.tables/index.html
changeset: 47482:d222b617b9d2
tag: tables
user: jjg
date: Mon Nov 06 17:17:07 2017 -0800
summary: 8190295: Introduce a new Table builder class
https://bugs.openjdk.java.net/browse/JDK-8190818
http://cr.openjdk.java.net/~jjg/8190295-etc/webrev.00/webrev.scripts/index.html
changeset: 47483:875dd666843f
tag: scripts
user: jjg
date: Mon Nov 06 17:18:34 2017 -0800
summary: 8190818: Introduce a new Script builder class
https://bugs.openjdk.java.net/browse/JDK-8190819
http://cr.openjdk.java.net/~jjg/8190295-etc/webrev.00/webrev.htmlDocument/index.html
changeset: 47484:aaf2675c4f4b
tag: htmlDocument
user: jjg
date: Mon Nov 06 17:19:06 2017 -0800
summary: 8190819: Merge HtmlWriter into HtmlDocument
https://bugs.openjdk.java.net/browse/JDK-8190820
http://cr.openjdk.java.net/~jjg/8190295-etc/webrev.00/webrev.head/index.html
changeset: 47485:ca0c08593f7a
tag: head
user: jjg
date: Mon Nov 06 17:19:28 2017 -0800
summary: 8190820: Introduce a new Head builder class
https://bugs.openjdk.java.net/browse/JDK-8190821
http://cr.openjdk.java.net/~jjg/8190295-etc/webrev.00/webrev.links/index.html
changeset: 47486:3e139d148884
tag: links
user: jjg
date: Mon Nov 06 17:19:47 2017 -0800
summary: 8190821: Introduce a new Links builder class
https://bugs.openjdk.java.net/browse/JDK-8190822
http://cr.openjdk.java.net/~jjg/8190295-etc/webrev.00/webrev.linkInfoImplStylename/index.html
changeset: 47487:762be115f89a
tag: linkInfoImplStylename
user: jjg
date: Mon Nov 06 17:20:49 2017 -0800
summary: 8190822: Remove dead code that could lead to invalid HTML
https://bugs.openjdk.java.net/browse/JDK-8190824
http://cr.openjdk.java.net/~jjg/8190295-etc/webrev.00/webrev.htmldocwriter/index.html
changeset: 47488:c09c658d94b3
tag: htmldocwriter
tag: qtip
tag: tip
user: jjg
date: Mon Nov 06 17:21:00 2017 -0800
summary: 8190824: Eliminate HtmlDocWriter
The first webrev (for Table.java) is the biggie. The others are all much
simpler.
-- Jon
On 11/06/2017 01:48 PM, Kumar Srinivasan wrote:
>
> Hi Jon,
>
> This is an enormous improvement and simplification!. Thanks!
>
> Table.java:
>
> 304 * <li>The prefix should probably be a value such that the
> genenrated ids cannot
>
> s/genenrated/generated/
>
>
> The previous block is ternary operation, would make it nicer if this
> is also so.
>
> HtmlTree cell;
> if ((colIndex) == rowScopeColumnIndex) {
> cell = HtmlTree.TH(cellStyle, "row", c);
> } else {
> cell = HtmlTree.TD(cellStyle, c);
> }
>
> final HtmlTree cell = HtmlTree.TH(cellStyle, "row", c)
> ? HtmlTree.TH(cellStyle, "row", c)
> : HtmlTree.TD(cellStyle, c);
>
>
>
> AbstractMemberWriter.java
>
> + TypeElement te =
> utils.getEnclosingTypeElement(element); // is this different from
> typeElement?
>
> I ran a quick test, and encountered several test failures, the reason
> is ClassUseWriter uses this method,
> and it creates AbstractMemberWriter with typeElement = null, via
> public AbstractMemberWriter(SubWriterHolderWriter writer) {
> this(writer, null);
> }
> and thus typeElement could be a null, the following might save some
> cpu cycles,
> I verified all tests pass and the comparison tests also pass.
>
> + TypeElement te = typeElement == null
> + ? utils.getEnclosingTypeElement(element)
> + : typeElement;
>
>
> There were other details we discussed offlist, and that clarifies the
> questions I had,
> regarding Table.java. Most of this is in Bhavesh's expertise, I will
> leave it to him.
>
> Kumar
>
>
>> Please review the following change, to replace the code to create
>> HTML tables with a new
>> builder-style Table class. With the exception of some minor
>> differences (bug fixes) on
>> the deprecated-list page, the generated output should be unchanged
>> for now, although
>> we may want to remove some backwards-compatibility hacks at some
>> point in the future
>> to make all tables be more uniform when appropriate.
>>
>> Before reading the detailed changes in the modified files, you might
>> want to start by
>> reviewing the new Table class.
>>
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8190295
>> Webrev: http://cr.openjdk.java.net/~jjg/8190295/webrev.00/
>>
>>
>> Notes:
>>
>> (Various files)
>> Stuff that has been deleted has generally moved in some form or other
>> into the new Table class
>> or otherwise made redundant.
>>
>> AbstractMemberWriter:118-138
>> Previously, the state defining the table was indirectly managed in
>> the MemberSummaryBuilder
>> class via tableContents and counter (uugh.) Now, the state is kept in
>> the AbstractMemberWriter
>> and lazily created by getSummaryTable, line 120.
>>
>> AbstractMemberWriter:433-437
>> Here's a relatively simple example of creating a Table, new-style.
>> The table is created (433-437), rows are added (463) and the HTML
>> generated (465)
>> Other Tables are created in *WriterImpl classes.
>>
>> AbstractMemberWriter: old lines 535-553
>> Anything to do with TableTabs is now mostly hidden inside the Table
>> class, driven by
>> addTab calls when the Table is initialized.
>>
>> AbstractMemberWriter:583-592
>> The lines are called by the MemberSummaryBuilder to conclude its work
>> on the table.
>>
>> DeprecatedListWriter:
>> There is a small, deliberate (bug fix?) change in behavior here.
>> Previously, modules
>> in the deprecated list used the colFirst style, and other types used
>> colDeprecatedItemName.
>> Now, everything uses the same style, colDeprecatedItemName. It didn't
>> seem worth
>> fixing for the old bad behavior. Also note that some cells were
>> omitted from the table,
>> causing inconsistent visual appear of the striping. That is also fixed.
>>
>> HtmlDocletWriter:2548
>> This is made available, for now. The script item itself is in
>> HtmlWriter (uugh). Long term,
>> this should move into a new Head class (builder for <head> element)
>>
>> MethodWriterImpl.java:258-278
>> This is definitely the most exciting part of this work. This is the
>> client-side replacement for
>> all the previous code to manage table tabs. Now, each tab is just a
>> method call with a
>> name and a predicate.
>>
>> ModuleIndexWriter.java:135-141
>> Following on from the previous comment, here in ModuleIndexWriter,
>> and corresponding code
>> in PackageIndexWriter, we have a dynamically added set of tabs, based
>> on command line options.
>>
>> ModuleWriterImpl.java:635-648
>> For now, for compatibility the rows are added in the same order as
>> before.
>> Going forward, we might want to add the rows in overall sorted order.
>>
>> PropertyWriterImpl.java:234-245
>> Because we create differently configures tables for properties and
>> methods, one without tabs, one with,
>> it falls out of the design that the rows in the properties table
>> don't get "ids", thus avoiding the earlier
>> "duplicate id" problem.
>>
>> Table.java:
>> The new class that does all the work. It's reasonably well commented,
>> so nothing to add here.
>>
>> -- Jon
>>
>
More information about the javadoc-dev
mailing list