RFR: JDK-8222669: Create and use new html.Entity class

[Jonathan Gibbons]

On 4/18/19 5:51 AM, Hannes Wallnöfer wrote:
> Hi Jon,
>
> The webrev URL was missing in your RFR, but I found it at the expected location:
>
> http://cr.openjdk.java.net/~jjg/8222669/webrev.00/
>
> Now this is quite a bit nicer than my „solution“ for RawHtml length calculation. Obviously, I still have a bit to learn in order to see potential for code cleanup and not assume things are the way they are supposed to be.

Generally, I consider RawHtml to be a "class of last resort" to be used 
when there is no better alternative. For example, given that we don't 
parse HTML text given on the command line (e.g. for the legal footer 
text), using RawHtml is a reasonable alternative. However, the reality 
of the implementation is somewhat worse, because for somewhat pragmatic 
reasons, the translations from DocTree nodes to HtmlTree nodes uses 
somewhat more use to DocTree::toString and RawHtml than I think is 
reasonable.  So, I consider that part of the world to be "OK, but could 
be better".

>
> The only suggestion I have is that there are three almost identical pieces of code that escape a String to a StringBuilder. It would be nice to factor that out into a shared method somewhere.

Yes, I was aware of that when I was modifying the code.  The code is not 
quite identical, but now that you make me look at it again, I can see 
how to make it happen. That will be a nice addition to this cleanup.  
Thanks for pointing this out.

-- Jon



>
> Hannes
>
>
>> Am 17.04.2019 um 22:02 schrieb Jonathan Gibbons <jonathan.gibbons at oracle.com>:
>>
>> Please review a relatively simple changeset to introduce and use a new class to explicitly handle HTML entities within an HtmlTree. This allows for some customized behavior (specifically a character count of 0 for a zero-width space) and avoids an otherwise unnecessary use of the RawHtml class.
>>
>> In addition to introducing the new class, the use of Contents.SPACE and Contents.ZERO_WIDTH_SPACE is updated to use the new entity constants. FInally, explicit uses of strings "<", ">" and "&" are updated to use appropriate new entities.
>>
>> -- Jon
>>
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8222669
>> Webrev: https://bugs.openjdk.java.net/browse/JDK-8222669
>>
>>