Any action that would want to properly restore caret state upon undo/redo will need to use the API.
All the actions using the DocumentUtilities.setTypingModification() will likely use the new API too. I'll fix them all as part of this issue.
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
Created attachment 159335 [details]
Apologies, this is an invalid diff unrelated to this change.
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.
VV1: I do not see CustomUndoDocument in the diff. Do I miss something?
Created attachment 159341 [details]
This is a proper diff of the change
Vladimir, thanks, the original diff was unrelated.
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
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?
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.
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?
Yes, I'll attempt to extend the patch to request a compatible scrolling on modifications too.
I have updated the patch with a compatible scrolling on modifications:
Integrated into 'main-silver', will be available in build *201605020002* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
User: Miloslav Metelka <firstname.lastname@example.org>
Log: #258798 - CaretUndo.createCaretUndoEdit() needs to become an official API for actions.
Unfortunately the commit caused regression #259061 but it's already fixed in jet-main.