On Thu, 30 Oct 2025 17:39:18 GMT, Andy Goryachev <[email protected]> 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 incrementally with one additional 
> commit since the last revision:
> 
>   javadoc

While reviewing the CSR I found 3 more places where you missed adding `if 
{@link #isUndoRedoEnabled()} returns {@code true}` to a method that creates an 
undo/redo entry.

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

> 1158: 
> 1159:     /**
> 1160:      * Clears the document, creating an undo/redo entry.

Another place to add `if {@link #isUndoRedoEnabled()} returns {@code true}`

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

> 1858: 
> 1859:     /**
> 1860:      * Replaces the specified range with the new text.  This method 
> creates an undo entry.

Another place to add `if {@link #isUndoRedoEnabled()} returns {@code true}`

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

Changes requested by kcr (Lead).

PR Review: https://git.openjdk.org/jfx/pull/1941#pullrequestreview-3401847653
PR Review Comment: https://git.openjdk.org/jfx/pull/1941#discussion_r2479659148
PR Review Comment: https://git.openjdk.org/jfx/pull/1941#discussion_r2479659974

Reply via email to