Unexpected change in UndoManager #903
Comments
|
Sorry Gary I've had a look and I don't have any suggestions. (I confess I don't know how the Undo/Redo code works.) In my own code I just used a plain text undo manager to get around it, but then I don't have any other styling needs except for spelling errors. Unfortunately this won't work for you though. |
|
Hi Gary, after fixing 904 I'm willing to have another crack at it. |
|
Hey! I may not be Gary, but as I've recently stumbled upon the same issue (and was able to reproduce it in I believe Gary's solution is derived from #735, which is also where I derived mine from. However, this solution seems to no longer work. Basically, the idea is to pass a suspendable event stream to a custom A plain text undo manager is, of course, the ideal solution -- however, that is unfortunately not possible (at least in my case) as I am working on a limited rich-text editor that both allows users to set simple formatting (like bold or italics) while also automatically computing some styles based on token matching (for instance, any text enclosed in square brackets are considered comments and grayed out). The I was able to reproduce this in the area.setUndoManager(UndoManagerFactory.unlimitedHistoryFactory().createMultiChangeUM(area.multiRichChanges().conditionOn(allowChange), TextChange::invert, UndoUtils.applyMultiRichTextChange(area), TextChange::mergeWith, TextChange::isIdentity));
private SuspendableYes allowChange = new SuspendableYes();Then, all that remains is to use private void updateTextColor(Color color) {
if(!updatingToolbar.get()) {
allowChange.suspendWhile(() -> updateStyleInSelection(TextStyle.textColor(color)));
}
}If you then start the demo, insert text, change its color, and attempt to undo it, you will be hit with an error somewhat like the one at the end of my comment. This should not happen. After this, any change to the text area (including a simple change that adds or removes a character) causes more errors. Because of that, I can't really test the redo, but I imagine it produces a similar error. What should actually happen is that the
If this is too difficult to implement/fix (although I imagine it worked at some point if it was the recommended course of action 168 revisions ago), might it instead be possible to integrate this kind of functionality directly into the If there's any other information or assistance you need, please let me know. I really appreciate you wanting to look into this. |
|
What I have found so far is that the current UndoManager implementation does a check after an undo/redo to see if the change it submitted was applied correctly. It does this by comparing the change event emitted afterwards by the text control with what it submitted. If they don't match then the reported exception gets thrown. In the case of RichTextFX the change event emitted contains a sub-document made up of text as well as styles of the altered area of text. So when the undo manager is suspended and a style change occurs, it isn't registered, with the result being that after the next undo the style is still present in the document and therefore won't match subsequent undo change comparisons for that portion of text. Unfortunately the current implementation can't be extended and the offending method overridden. So the only way around it at this stage is to create a custom UndoManager implementation that doesn't do the post change comparison check. I've done a basic test and it seems to work, so could you both please thoroughly test this and give me feedback. (nocheckUndo.zip only contains the following folder org/fxmisc/richtext/util with three files) Extract into the root of your project and use with: |
|
Hey Jugen; I've tested all of my use cases with your files and am happy to report that they all work wonderfully (though, granted, I only really have two use cases for these suspendable changes at the moment). I have also tested "normal" undo/redo operations involving both plain text (removing characters or words or adding them) and rich text (formatting) changes and can report that those also still work as expected. I will continue to monitor the undos and redos during regular work on the project and will edit this comment or drop another comment if I notice anything strange. Of course, this does make one wonder—if those undo/redo checks could be safely removed with no adverse effects, then why were they there to begin with? Just for added safety? It seems like it caused more issues than it potentially solved, but it can't be that simple. I feel like we're missing something. Do you have any idea what problems those undo/redo checks could have potentially solved? Thank you very much for your work. |
|
Hey cengels, that's great. I think the checks are there to help developers adding UndoFx to their projects as well as to insure the general consistency of the document so that subsequent undo/redo calls produce predictable results. For instance with the solution provided above if you were to change the actual text while suspending the undo manager then any subsequent undo will probably wreck your text. Without feedback from the undo manager a developer wouldn't have any insight as to why this was happening. Here is my second attempt which uses plain text comparison and doesn't compare styling. As before extract into the root of your project and use with: (Note the different method name) |
|
Hey again. I tested the new |
|
Thanks for the feedback @cengels, I hope to hear from @garybentley as well .... ? |
|
Hi Jugen, |
|
Wow that's great Gary, for starting a degree. And all the best in getting it done. |
|
I finally got some time to test your fix Jugen. It looks like it's working, at least I'm not seeing the exception anymore. Thanks for fixing it :) |
I'm getting the following exception from the UndoManager:
This is because it is trying to undo a change to text that has a style that has not been recorded.
The style in this case is a "spelling error" indicator for a word.
I'm adding the style to the word using:
The suspendWhile is blocking the change from being recorded.
My undo manager is created using:
And my segment has the following equals method (which is returning true when it's trying to do an undo):
So the question is how can I add the spelling error style (which I don't want to record in the undo list) but still have the undo work?
The text was updated successfully, but these errors were encountered: