RFR: 8366201: RichTextArea: remove allowUndo parameter [v3]

[Kevin Rushforth]

On Wed, 29 Oct 2025 21:54:17 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> Original user feedback (see https://mail.openjdk.org/pipermail/openjfx-discuss/2025-August/000267.html ) called for adding an `allowUndo` parameter to `applyStyle()` and `setStyle()` methods similarly to `replaceText()`.
>> 
>> Upon further analysis, the `allowUndo` parameter was a mistake: allowing the application code to disable creating undo/redo entries messes up the internal undo/redo stack.
>> There is an internal need (`UndoableChange`), but it should not be exposed via public API.
>> 
>> This PR also adds `isUndoRedoEnabled()` and `setUndoRedoEnabled()` to the `StyledTextModel`, as well as its forwarding aliases to `RichTextArea` to allow for the application to disable undo/redo temporarily, for example, when building a document from multiple segments.
>> 
>> WARNING this is an incompatible change, permitted because of the incubator.
>> 
>> There remains a possible issue with currently unlimited size of the undo/redo stack - perhaps we should limit its depth to maybe 100-200 entries, see https://bugs.openjdk.org/browse/JDK-8370447 .
>
> Andy Goryachev has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 11 additional commits since the last revision:
> 
>  - review comments
>  - Merge remote-tracking branch 'origin/master' into 8366201.allow.undo
>  - undo redo enabled logic
>  - cleanup
>  - cleanup
>  - removed allow undo parameter
>  - nl
>  - test
>  - append insert text
>  - tests
>  - ... and 1 more: https://git.openjdk.org/jfx/compare/08cea2b1...ffe6894c

I left a couple more doc comments. Otherwise the API docs look ready to go.

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

> 439:      * Replaces text in this CodeArea.
> 440:      * It creates an undo/redo entry if the model's
> 441:      * {@link StyledTextModel#isUndoRedoEnabled() isUndoRedoEnabled()} returns {@code true}.

I think it is cleaner to refer to the `isUndoRedoEnable()` method in _this_ class. No need to refer to the model here. Something like:

     * It creates an undo/redo entry if
     * {@link #isUndoRedoEnabled() isUndoRedoEnabled()} returns {@code true}.

modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/RichTextArea.java line 1077:

> 1075:      * sequences result in a new paragraph being added.
> 1076:      * It creates an undo/redo entry if the model's
> 1077:      * {@link StyledTextModel#isUndoRedoEnabled() isUndoRedoEnabled()} returns {@code true}.

No need to refer to the model.

modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/model/StyledTextModel.java line 634:

> 632:      * {@code start} position.
> 633:      * It creates an undo/redo entry if the model's
> 634:      * {@link StyledTextModel#isUndoRedoEnabled() isUndoRedoEnabled()} returns {@code true}.

Since this is a method in the model, saying "the model's ..." is not needed. Maybe just say:


     * It creates an undo/redo entry if
     * {@link #isUndoRedoEnabled() isUndoRedoEnabled()} returns {@code true}.


(like you did in a below method)

modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/model/StyledTextModel.java line 844:

> 842:      * Indicates whether undo/redo functionality is enabled.
> 843:      * @return true if undo/redo functionality is enabled
> 844:      * @since 26

Please add:


     * @defaultValue {@code true}

modules/jfx.incubator.richtext/src/test/java/test/jfx/incubator/scene/control/richtext/RichTextAreaTest.java line 796:

> 794:         assertTrue(control.isUndoable());
> 795:         control.setUndoRedoEnabled(false);
> 796:         assertFalse(control.isUndoable());

To check that the stack is actually cleared, you might want to re-enable undoRedo, append text, undo, check for "23", undo again, check that it is still "23".

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

PR Review: https://git.openjdk.org/jfx/pull/1941#pullrequestreview-3399343298
PR Review Comment: https://git.openjdk.org/jfx/pull/1941#discussion_r2477917825
PR Review Comment: https://git.openjdk.org/jfx/pull/1941#discussion_r2477971753
PR Review Comment: https://git.openjdk.org/jfx/pull/1941#discussion_r2477929496
PR Review Comment: https://git.openjdk.org/jfx/pull/1941#discussion_r2477935045
PR Review Comment: https://git.openjdk.org/jfx/pull/1941#discussion_r2477902651