On Tue, 17 May 2022 03:19:39 GMT, Jonathan Gibbons <j...@openjdk.org> wrote:
>> This is a conceptually simple change, but it has a few ramifications that >> make it at least look a bit bigger than it is. >> >> At the core, this adds a new method to the `Table` class that allows to >> generate tables with multiple categories but omit the generation of table >> tabs. This is used in the "New API" and "Deprecated API" pages to render the >> release selectors as a global row of checkboxes that control release >> information for all tables in the page. Also, the element name and release >> columns in those pages are now sortable by clicking on the table headers. >> >> Output generated with this change is available here: >> >> http://cr.openjdk.java.net/~hannesw/8268422/api.06/new-list.html >> http://cr.openjdk.java.net/~hannesw/8268422/api.06/deprecated-list.html >> >> Now to the details and aforementioned ramifications: >> >> - Since I added new functionality to the table class I thought it woudl be >> a good occasion to remove some other functionality that was never really >> used by our code. This includes the possibility to set the table tab and >> striped row styles. We always use the default styles for these, and if >> somebody wanted to change the look of the styles the way to go would be to >> change the style definitions in the stylesheets rather then the style names. >> This change made the inclusion of a table-specific script obsolete, so >> you'll see some removals of `table.needsScript()` and `table.getScript()` in >> unrelated code. >> - `SummaryListWriter`, the base class for the New API and Deprecated API >> pages, gets a few new protected methods that are overridden in subclasses to >> generate the extra content as needed. I think the solution is suboptimal and >> a bit noisy. The protected classes have generic names such as >> `getExtraContent` and the parent class is not really aware of the different >> releases, since one subclass never displays multi-release information >> (Preview API), one does sometimes (Deprecated API) and one does always (New >> API). I think we could come up with a nicer solution, but I think it's not >> terrible and didn't want to delay integration too much. >> - We have yet more JavaScript for global table category selection and >> sortable table columns as well as new CSS classes, including inlined SVGs >> for the sortable column icons. I tried to keep the additions as small and >> clean as possible and added a short comment for each function. >> - The changes in `TestNewApiList.java` are huge. I couldn't have made those >> changes without improving the output of `JavadocTester` by adding the output >> directory to the test name. This change makes it much easier to read jtreg >> output for tests with many similar but slightly different checks. I probably >> should have done this a long time ago! > > test/langtools/jdk/javadoc/lib/javadoc/tester/JavadocTester.java line 1089: > >> 1087: content = null; >> 1088: } else { >> 1089: name = outputDir + "/" + file; > > I'm trying to understand the merit of this, since the output directory is > often just `out` or `api` isn't it? Don't you want the test method name in > there, for increased resolution? For test classes containing multiple test methods the output directory is actually the best way to identify which run of javadoc generated the output. If a test were to do multiple runs of javadoc with the same output directory I would certainly consider it a serious bug. Also, a single test method could easily do multiple runs of javadoc (I'm not sure we do that in our test suite, but I wouldn't be too surprised). Adding the output directory disambiguates the file name which is already there, making it easy to locate the file itself and the code that generated it. ------------- PR: https://git.openjdk.java.net/jdk/pull/8657
