Community
Participate
Working Groups
UndoStack fails to properly commit deletion changes when the happen at offset 0. STR: 1. Open Orion with some code and go to line 0, offset 0. 2. Delete several chars. 3. Press Ctrl-Z then Ctrl-Y. By now you can see that undo/redo fail to work. Problem: UndoStack tracks changes by storing a negative _undoStart for deletion and these changes are committed together in a single change. However, when _undoStart is 0 ... the commitUndo function fails to record the removal. Fix: track the undo type separately. Will submit a proposed patch.
Proposed patch: https://github.com/mihaisucan/orion.client/tree/bug-370606 Please review and let me know if this fix can land. Thank you!
Please note that I also made a change to ignore empty changes. For example if the user presses Backspace at the beginning of the document or delete at the end, they are stored as individual empty changes that are confusing when one tries to undo/redo across multiple changes in the document.
(In reply to comment #2) > Please note that I also made a change to ignore empty changes. For example if > the user presses Backspace at the beginning of the document or delete at the > end, they are stored as individual empty changes that are confusing when one > tries to undo/redo across multiple changes in the document. This looks like a bug in the text model. We should not be getting Chaging/Changed events if there is no change in the document.
The other changes are fine to be committed, but I believe we should fix textModel.js for invalid/empty Changing/Changed events. We can just add this line to setText() after the undefined checks. if (start === end && text === "") { return; }
Thank you Silenio! Shall I make the change you suggested in TextModel setText()? If yes, shall I land the updated patch?
I fixed the textModel problem: http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=cec71bddaf32251c34d3728df5da13c130d14f33 Mihai, please go ahead and land your updated changes. Thanks.
Thank you Silenio! I have updated the patch and landed: http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=3ce24b94f1d8103b16b9cf16f2f50a6302d43b18
Mihai, I just found a bug with the new code: 1) Type a letter "a" 2) Type backspace to remove letter "a" 3) Type Ctrl+z to undo change Note that "aa" gets inserted. It should be only "a".
(In reply to comment #8) > Mihai, I just found a bug with the new code: > > 1) Type a letter "a" > 2) Type backspace to remove letter "a" > 3) Type Ctrl+z to undo change > > Note that "aa" gets inserted. It should be only "a". I committed a fix for this problem. I will leave the bug open because it needs more testing. http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=27177e9a3dc70c20b4877e3eab3adfff1d56e342
(In reply to comment #9) > (In reply to comment #8) > > Mihai, I just found a bug with the new code: > > > > 1) Type a letter "a" > > 2) Type backspace to remove letter "a" > > 3) Type Ctrl+z to undo change > > > > Note that "aa" gets inserted. It should be only "a". > > I committed a fix for this problem. I will leave the bug open because it needs > more testing. > > http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=27177e9a3dc70c20b4877e3eab3adfff1d56e342 Thanks for your fix! Tested and it seems to work fine for me.
Closing
