All the actions using the DocumentUtilities.setTypingModification() will likely use the new API too. I'll fix them all as part of this issue.
Comment 2Vladimir Voskresensky
2016-04-19 13:13:11 UTC
I'm increasing priority because a lot of actions which cause document modifications misses correct caret after Undo.
recently I've faced comment line, uncomment line, delete line (Ctrl+E) and I believe any others touching document
I'd like to ask for review of this API change which will allow actions in an arbitrary module to restore caret positions properly upon undo/redo.
The editor.document is enhanced by CustomUndoDocument interface which is implemented by BaseDocument and it allows to add custom undoable edits during atomic transactions over the document.
This API is then used by EditorUtilities.addCaretUndoableEdit() method in editor.lib2 module which is intended to be used by the actions.
I've rewritten several actions in BaseKit as part of the diff and I'll update all the remaining actions once the change becomes integrated.
Comment 5Vladimir Voskresensky
2016-04-20 10:07:00 UTC
VV1: I do not see CustomUndoDocument in the diff. Do I miss something?
Vladimir, thanks, the original diff was unrelated.
Comment 8Vladimir Voskresensky
2016-04-20 12:21:36 UTC
VV2: What if EditorUtilities.addCaretUndoableEdit() called twice? I.e. one by editor lib internals and another by the client with the same/different caret states
Comment 9Vladimir Voskresensky
2016-04-20 12:23:52 UTC
VV3: Is it required now to update all clients what to expect to old clients? I.e. now comment/uncomment inserts removes "//" but doesn't call this method. Now on Undo caret is not restored. Would it be fixed as part of this change?
Comment 10Miloslav Metelka
2016-04-20 14:18:24 UTC
ad VV2) It can be called as many times as desired during a single document atomic transaction. But in fact only the first snapshot will be "effective" after undo of the transaction and only the last snapshot will be effective for redo of the transaction.
Generally if an action does some extra caret movements after text insertion/deletion it should call EditorUtilities.addCaretUndoableEdit() at the end of the transaction since that will ensure that when redoing the effect of the action the caret(s) will be at proper place(s).
As I've already mentioned in javadoc of CaretUndo.createCaretUndoEdit() in the future we could add a code to EditorCaret that would track if any explicit caret moves were done since last call to EditorUtilities.addCaretUndoableEdit() in a single atomic transaction and if none then the CaretUndo.createCaretUndoEdit() could return null to not consume any extra space in UndoManager's queue. Both undo and even redo should still work properly just with the initial restoration at action's begining.
The original undo approach for a single caret was to navigate the caret to document modifications boundaries but that's no longer feasible with a general case of multiple modifications done by multiple carets (or at least it would be rather tricky and complex to pin the modifications to carets).
The EditorUtilities.addCaretUndoableEdit() approach will also allow us to get rid of a dependency on BaseDocumentEvent (especially isInUndo()) and work well with DefaultDocumentEvent instances in the future too. In the past it was possible to distinguish an undo of a DocumentEvent because there was removeUpdate() called on an event which had DocumentEvent.getType() == DocumentEvent.EventType.INSERT (similarly for removals). Then the Swing team changed the situation by wrapping the original event into AbstractDocument.UndoRedoDocumentEvent which however is not public so it's no longer possible to cleanly distinguish undos even for a single-caret case with DefaultDocumentEvent. Our approach should cover this too.
Proper text selection restoration is now possible too.
Ad VV3: I would definitely prefer to update the actions and I'll do it as part of this change, I just did not want to inflate the diff by these action updates.
Comment 11Vladimir Voskresensky
2016-04-20 15:17:39 UTC
Re Ad VV3: My question is: what would happen with existing plugins doing modifications (comment/uncomment is like and example)? Would they all be broken in terms of undo/redo comparing to 8.1? If yes you should increase major version of editor lib to notify incompatible change introduced by multi-carets, isn't it?
Comment 12Miloslav Metelka
2016-04-21 09:03:49 UTC
Yes, I'll attempt to extend the patch to request a compatible scrolling on modifications too.
Comment 13Miloslav Metelka
2016-04-28 22:41:48 UTC