On Thu, 20 May 2021 15:22:08 GMT, liach <github.com+7806504+li...@openjdk.org>
wrote:
>> This is a simple cleanup to replace the sentinel `HtmlTree.EMPTY` text
>> constant with an instance that achieves the same by overriding `isValid()`.
>> I think this is the nicer solution, and it allows us to remove the special
>> case identity check in `HtmlTree.add(Content)`.
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/markup/HtmlTree.java
> line 174:
>
>> 172: ((ContentBuilder) content).contents.forEach(this::add);
>> 173: }
>> 174: else if (content.isValid()) {
>
> Should the content builder have a similar validity check to ensure if it's
> not empty, its contents are always valid? otherwise it's quite hard to define
> if the content builder is valid or not as it can be considered either and
> always need special case in client code. In comparison, the html tree's
> contents are always valid no matter if the outer tags are valid or not.
I think the current behaviour of `ContentBuilder` to accept all content and
check validity on demand is the better solution. At least in theory it's
possible that invalid content (e.g. an empty HtmlTree) is added to a
ContentBuilder which later becomes valid by adding valid content to it. On the
other hand, one could argue that `ContentBuilder` and `HtmlTree` should behave
the same, which would speak for checking validity when adding to
`ContentBuilder`.
-------------
PR: https://git.openjdk.java.net/jdk/pull/4130
