On Wed, 5 Nov 2025 18:14:51 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), and as a property in the >> `CodeArea`. >> >> ### NOTES >> >> - some dependency on #1938 , resolved. >> - reviewers may use the Monkey Tester with the `CodeArea.lineEnding` >> property enabled from this branch: >> https://github.com/andy-goryachev-oracle/MonkeyTest/tree/line.ending > > Andy Goryachev has updated the pull request incrementally with one additional > commit since the last revision: > > code area test modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/CodeArea.java line 472: > 470: protected void invalidated() { > 471: LineEnding v = get(); > 472: if(v == null) { Minor: space after `if` modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/CodeArea.java line 473: > 471: LineEnding v = get(); > 472: if(v == null) { > 473: set(old); You also need to unbind if bound. modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/LineEnding.java line 34: > 32: */ > 33: public enum LineEnding { > 34: /** Classic Mac OS line ending, ASCII CR (0x0d) */ I would say "Legacy" rather than "Classic", and maybe add a sentence that this was used on Mac systems prior to macOS 10. modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/LineEnding.java line 37: > 35: CR, > 36: /** Windows line ending, ASCII LF (0x0a) */ > 37: CRLF, Can you add a `.` at the end of the sentence (here and for all the enums)? modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/LineEnding.java line 41: > 39: LF, > 40: /** Platform default line ending, using the value of {@code > System.getProperty("line.separator")} */ > 41: SYSTEM_DEFAULT; or maybe just `SYSTEM`? Now that I look at it, I'm not sure we need the "DEFAULT" suffix (nor the word "default" in the description). modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/model/StyledOutput.java line 59: > 57: */ > 58: public static StyledOutput forPlainText() { > 59: return new StringBuilderStyledOutput(null); Null is not allowed. Pass `LineEnding.SYSTEM_DEFAULT`. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1944#discussion_r2495753140 PR Review Comment: https://git.openjdk.org/jfx/pull/1944#discussion_r2495755365 PR Review Comment: https://git.openjdk.org/jfx/pull/1944#discussion_r2495712433 PR Review Comment: https://git.openjdk.org/jfx/pull/1944#discussion_r2495714570 PR Review Comment: https://git.openjdk.org/jfx/pull/1944#discussion_r2495698978 PR Review Comment: https://git.openjdk.org/jfx/pull/1944#discussion_r2495749183
