On Wed, 5 Nov 2025 21:00:37 GMT, Andy Goryachev <[email protected]> wrote:
>> Adds control of line endings (newline separators) in `StyledTextModel`,
>> `RichTextArea`, and `CodeArea`.
>>
>> The impacted areas are:
>> - saving to plain text
>> - copying to plain text
>> - IME
>>
>> This feature is implemented as a regular field in the `StyledTextModel`
>> (since it is ultimately an attribute of the model), with convenience setter
>> and getter in the `RichTextArea`.
>>
>> ### NOTES
>>
>> - some dependency on #1938 , resolved.
>
> Andy Goryachev has updated the pull request incrementally with one additional
> commit since the last revision:
>
> removed property
Looks good with a couple fairly minor comments inline.
modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/LineEnding.java
line 36:
> 34: /** Legacy Mac OS line ending, ASCII CR (0x0d). */
> 35: CR,
> 36: /** Windows line ending, ASCII LF (0x0a). */
The "ASCII LF (0x0a)" bit belongs to the `LF` enum value, not here.
modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/LineEnding.java
line 38:
> 36: /** Windows line ending, ASCII LF (0x0a). */
> 37: CRLF,
> 38: /** macOS/Unix line ending, sequence of CR/LF (0x0d 0x0a). */
The The "CR/LF (0x0d 0x0a)" bit belongs to the `CRLF` enum value, not here.
While you are at it, maybe add "ASCII" before "CR/LF" for consistency?
modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/model/StyledTextModel.java
line 1023:
> 1021: if (lineEnding == null) {
> 1022: lineEnding = LineEnding.SYSTEM;
> 1023: }
This isn't needed, right? `lineEnding` can never be null as long as you
initialize it to `LineEnding.SYSTEM `, since you check and throw on an attempt
to set it to null.
-------------
PR Review: https://git.openjdk.org/jfx/pull/1944#pullrequestreview-3430225520
PR Review Comment: https://git.openjdk.org/jfx/pull/1944#discussion_r2500587452
PR Review Comment: https://git.openjdk.org/jfx/pull/1944#discussion_r2500590389
PR Review Comment: https://git.openjdk.org/jfx/pull/1944#discussion_r2500604816
