On Mon, 27 Jan 2025 22:10:18 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
> Fixes minor issues uncovered while writing API tests #1677:
>
> - StyledTextModel::clamp L559 is incorrect at the end of paragraph
> - CodeArea.getText() adds extra newline at the end
>
> This change should be back-ported to jfx24.
Looks good, including the updated title.
I left a question and suggestion and will reapprove if you make changes.
@arapte or @lukostyra can one of you be the second reviewer?
modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/CodeArea.java
line 424:
> 422: StyledTextModel m = getModel();
> 423: if (m == null) {
> 424: return null;
This didn't used to return `null` before this fix (it would throw NPE on a null
model). Perhaps returning the empty string be better? Either way, it would be
helpful to document whether or not this method can return null.
modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/CodeArea.java
line 433:
> 431: } catch (IOException e) {
> 432: // should not happen
> 433: throw new RuntimeException(e);
Minor: wrapping it in `UncheckedIOException` is preferred over raw
`RuntimeException`.
-------------
Marked as reviewed by kcr (Lead).
PR Review: https://git.openjdk.org/jfx/pull/1684#pullrequestreview-2578992848
PR Comment: https://git.openjdk.org/jfx/pull/1684#issuecomment-2619689946
PR Review Comment: https://git.openjdk.org/jfx/pull/1684#discussion_r1932600423
PR Review Comment: https://git.openjdk.org/jfx/pull/1684#discussion_r1932602977
