RFR: JDK-8253735: Cleanup SearchIndexItem API
Jonathan Gibbons
jjg at openjdk.java.net
Mon Oct 5 17:17:48 UTC 2020
This pull-request is for a substantial refactoring and cleanup of the code
to build the static index pages and the files for the interactive search
There is no significant change in functionality, and all tests continue to
pass without change.
## Improvements
* `SearchIndexItem` is merged into `IndexItem`
* `SearchIndexItems` (the collection of `SearchIndexItem`s indexed by
`Category`) is merged into `IndexBuilder`, which now maintains
both maps: index items grouped by first character, and grouped by category
* In `Category`, the members `INDEX` and `SYSTEM_PROPERTY` are merged into
single new entry `TAGS`, such that the members of `Category` now directly
correspond with the JavaScript files generated for interactive search
* `IndexItem` now provides access to the `DocTree` node for those items
that previously were `SearchIndexItem`. This can be used to differentiate
between items for `{@index...}` and `{@systemProperty}`.
This specific change was a primary motivation for all this work, in order
to facilitate supporting additional new tags that can contribute items
for the index.
* All index items (i.e. including those that previously were `SearchIndexItem`)
are now created by static factory methods instead of directly calling
constructors. This allows many values to be precomputed and stored in
final fields or made available by overriding accessor methods in
anonymous subtypes.
* The comparators for index items have been cleaned up. Previously, one
of the comparators was "unstable", meaning that it could fall back on
the value of mutable fields, and the `toString` output (which was used
to generate the content for the JavaScript files.)
* Work to generate the JavaScript files has been moved out of
`AbstractIndexBuilder` and its subtypes into a new class, `HtmlIndexBuilder`,
that subclasses the main `IndexBuilder`.
To facilitate that, some methods were moved from `HtmlDocletWriter` to
`Links`. This is not ideal, because it introduces a dependence on `Utils`
in `Links` that was not there before, but the choice was pragmatic and the
least bad of the alternatives. Long term, we might want to move most
of the `formats/html/markup` package into a new more standalone package that
does not rely on other javadoc internals, and at that time, the factory
objects like `Links`, `TableHead` and `Table` would just move up to the
`formats/html` package.
## The Changes
The work is done in a series of steps/commits, each with a specific focus
of the work involved. It may be instructive to review the changes in each
commit, to follow the overall evolution of the work. From the Git log,
the changes are as follows (oldest first)
* Move `SearchIndexItem`, `SearchIndexItems` to `toolkit.utils`
* Cleanup `SearchIndexItem`
* fix bug
* Simplify `SearchIndexItems`
* simplify statement
* Merge `SearchIndexItems` into `IndexBuilder`
* Cleanup `IndexItem` prior to merge with `SearchIndexItem`
* Cleanup `SearchIndexItem` prior to merge with `IndexItem`
* Merge `SearchIndexItem` into `IndexItem`
(without changing how items are added to the index collections)
* simplify adding index items to the index builder
* move comparators for index items into IndexBuilder
* improve init for some index items
improve comments in IndexItem
* Bug fix: obsolete call to add an item to the index
Improve comparators used to build index
* Move `getAnchor` methods from `HtmlDocletWriter` to `Links`.
This is slightly undesirable because it means that `Links` requires access to `Utils`, but it is less bad than the
alternatives.
* Move code to complete initialization of index items and to write JavaScript index files from `AbstractIndexWriter` and
subtypes to new subtype of `IndexBuilder`: `HtmlIndexBuilder`
At each stage, the repo should build and all javadoc tests should pass (and there is no reason to believe that any
other tests may fail.)
## Future work
Instead of maintaining collections of `SortedSet`s in `IndexBuilder`, it might
be more effective to use `List` instead, and just sort the list as needed.
There is little need to eagerly build both maps in `IndexBuilder`. As long as
there is at least one collection, such as the `itemsByFirstCharacter`, we could
defer generating `itemsByCategory` until we need to write out the JavaScript
files. There is one place in the code, in `SystemPropertyWriter`, where we
look at `itemsByCategory` to determine whether there were any
`{@systemProperty...}` tags, but at the time we need that information, it
would not be significantly more expensive to scan `indexByFirstCharacter`,
because the items for elements need not have been added at this time.
-------------
Commit messages:
- Merge master
- Move code to complete initialization of index items and to write JavaScript index files from AbstractIndexWriter and subtypes to new subtype of IndexBuilder: HtmlIndexBuilder
- Move `getAnchor` methods from HtmlDocletWriter to Links.
- Bug fix: obsolete call to add an item to the index
- improve init for some index items
- move comparators for index items into IndexBuilder
- simplify adding index items to the index builder
- Merge SearchIndexItem into IndexItem
- Cleanup SearchIndexItem prior to merge with IndexItem
- Cleanup IndexItem prior to merge with SearchIndexItem
- ... and 6 more: https://git.openjdk.java.net/jdk/compare/1c2754bf...5343e741
Changes: https://git.openjdk.java.net/jdk/pull/499/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=499&range=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8253735
Stats: 1747 lines in 22 files changed: 839 ins; 679 del; 229 mod
Patch: https://git.openjdk.java.net/jdk/pull/499.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/499/head:pull/499
PR: https://git.openjdk.java.net/jdk/pull/499
More information about the javadoc-dev
mailing list