On Thu, 20 Jan 2022 19:31:41 GMT, Jonathan Gibbons <j...@openjdk.org> wrote:
> Please review a simple `noreg-clean` update to `HtmlTree` and a couple of > friends. The updates are to continue the trend towards the use of Facebook > tory methods for `HtmlTree` objects, and to leverage generic methods and > lambdas to generate composite content. The cleanup is mostly extracted from > some parallel work for a separate PR. Some of the more general cleanup was > suggested by an IDE. > > All javadoc tests pass locally; I have a job in progress to validate the fix > on all platforms, although there is no reason to believe there might be any > issues. Looks good! I would propose to move the implementation of `addAll` from `HtmlTree` to `Content` as explained in the in-code comment. One additional place where `addAll` could be used is in TagletWriterImpl.java around line 356 where the stream/filter/forEach construct may be replaced with `seeList.addAll(links, HtmlTree::LI);` (the filtering of invalid/empty elements is actually redundant since this is already done in `HtmlTree.add(Content)`). src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/markup/HtmlTree.java line 240: > 238: list.forEach(item -> add(mapper.apply(item))); > 239: return this; > 240: } We could move this implementation of `addAll` to the `Content` base class as it is except for changing the return type to `Content`. This would make the method available in other subclasses that override/support `add(Content)`, such as `ContentBuilder`. The only observable change would be that it wouldn't throw `UnsupportedOperationException` when called with an empty collection in non-addable classes. I think that's an acceptable behaviour for an internal API. ------------- Marked as reviewed by hannesw (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7166
