On Tue, 4 Nov 2025 21:06:59 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 I think we need to update comments related to appendText() method. Please correct me if i am wrong. Also if we are updating the identified comments, we also need to update the CSR. modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/RichTextArea.java line 103: > 101: * // build the content > 102: * textArea.setUndoRedoEnabled(false); > 103: * textArea.appendText("RichTextArea\n", heading, false); We don't pass any per method undo/redo flag as argument in appendText. This needs to be updated. modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/RichTextArea.java line 1105: > 1103: * <p> > 1104: * This convenience method is equivalent to calling > 1105: * {@code appendText(text, StyleAttributeMap.EMPTY, true);} I dont see any appendText() method with third argument. And we have already specified in StyledTextModel that default value for undoRedoEnabled is `true`. So this comment needs to be updated. ------------- PR Review: https://git.openjdk.org/jfx/pull/1941#pullrequestreview-3428171390 PR Review Comment: https://git.openjdk.org/jfx/pull/1941#discussion_r2498951695 PR Review Comment: https://git.openjdk.org/jfx/pull/1941#discussion_r2498963504
