On Mon, 8 Aug 2022 10:23:02 GMT, Hannes Wallnöfer <[email protected]> wrote:
> This looks quite good. Apart from the inline comments, I wondered about the
> following:
>
> * Would the methods and constant you moved to `Text` be better placed in
> `Content` as they are used in other Content instances?
> * There are quite a few uses of the `"\n"` string literal in a content or
> pre-content context where the `NL` constant couuld be used.
1. I did wonder about moving the methods and constant into `Content` instead of
`Text` but I think it is better to keep `Content` more abstract, and the new
methods and constant are more "text-y". I also like the definitions being up
in the `markup` package.
2. Yes, there may be references in the code to `\n`, but I don't think we need
to go overboard to find and change them. To me, this is (generally) another
reason to use simple `\n` in text strings, because that is the more usual
convention in Java source code. Note that sometimes these occurrences
originate in properties files, and so cannot use the `NL` constant, and it
seems silly to use `.replace("\n", NL)` on all such strings.
-------------
PR: https://git.openjdk.org/jdk/pull/9691
