RFR: 8370140: RichTextArea: line endings [v6]

[Kevin Rushforth]

On Fri, 31 Oct 2025 18:48:57 GMT, Andy Goryachev <angorya at openjdk.org> 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), and as a property in the `CodeArea`.
>> 
>> NOTE:
>> - some dependency on #1938 , resolved.
>
> Andy Goryachev has updated the pull request incrementally with one additional commit since the last revision:
> 
>   get text

I looked over the proposed API and left some inline comments. I'll repeat my two main points here:

1. Rather than a Nullable enum, create an explicit enum value for "use the `line.separator` property"
2. Why is the new lineEnding property on the `CodeArea` subclass and not on `RichTextArea`?

modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/CodeArea.java line 466:

> 464:      * @return the line ending property
> 465:      * @since 26
> 466:      * @defaultValue null

I can't think of a good reason for this to be a Nullable property. Instead, define a new enum value that means "use the value of the `line.separator` system property". It seems much cleaner (I don't like relying on `null` for out of band enum values, since it will cause an NPE if you switch on it without checking for null first).

Separately, should "use line.separator" should be the default value or should it use "\n" instead. Probably using "line.separator" is a better choice, which is what this PR does. The reason I bring this up is so that we make a conscious decision.

modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/CodeArea.java line 468:

> 466:      * @defaultValue null
> 467:      */
> 468:     public final ObjectProperty<LineEnding> lineEndingProperty() {

Shouldn't this be a property of RichTextArea?

modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/LineEnding.java line 33:

> 31:  * @since 26
> 32:  */
> 33: public enum LineEnding {

Please add an explicit enum value that means "use the value of System.getProperty("line.separator")". Relying on `null` to mean this is not clean for two reasons: 1) it is more difficult to document (case in point, it isn't documented here); and 2) using an enum that might be null in a switch statement without first checking for null will cause an NPE.

Possible names: `NATIVE`? `LINE_SEPARATOR`? something else?

modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/LineEnding.java line 35:

> 33: public enum LineEnding {
> 34:     /** Classic Mac OS */
> 35:     CR,

I question the need for this enum value. CR was only used prior to macOS 10.0, which is so old as to be uninteresting without a good reason (for comparison, the very first version of Java for macOS was for macOS 10.5). Do you have a use case in mind?

modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/LineEnding.java line 39:

> 37:     CRLF,
> 38:     /** macOS/Unix */
> 39:     LF

Since this shows up in the javadoc-generated API docs, it might be worth expanding this a bit.

-------------

PR Review: https://git.openjdk.org/jfx/pull/1944#pullrequestreview-3418252114
PR Review Comment: https://git.openjdk.org/jfx/pull/1944#discussion_r2491791031
PR Review Comment: https://git.openjdk.org/jfx/pull/1944#discussion_r2491987923
PR Review Comment: https://git.openjdk.org/jfx/pull/1944#discussion_r2491756972
PR Review Comment: https://git.openjdk.org/jfx/pull/1944#discussion_r2491766155
PR Review Comment: https://git.openjdk.org/jfx/pull/1944#discussion_r2491767288