Community
Participate
Working Groups
With Eclipse IDE I20201202-1800 (IIRC other 2020/2021 versions as well), undo intermittently stops working causing a user to suffer permanent data loss. The actual behavior can be seen in the two screencasts in https://github.com/tlaplus/tlaplus/issues/568#issuecomment-774079477, which have been recorded with an Eclipse RCP. This issue applies there too. Both the keyboard commands as well as the menu item no longer work.
This might be related to Bug 559539. Would you be able to see if the operation history gets flushed/disposed, and where that call originates from? That is, is this methed called and by who: PlatformUI.getWorkbench().getOperationSupport().getOperationHistory().dispose() Otherwise the IOperationHistory#dispose call would be a candidate to check it it gets called.
org.eclipse.core.commands.operations.DefaultOperationHistory.dispose(IUndoContext, boolean, boolean, boolean) doesn't get called during relevant user actions. The screencasts below show some really strange behavior s.a. undo/redo applied to the inactive editor. This is independent of undo/redo invoked via the menu or through the keyboard shortcut. http://tla.msr-inria.inria.fr/kuppe/EclipseUndoRedo.gif http://tla.msr-inria.inria.fr/kuppe/EclipseUndoRedo2.gif http://tla.msr-inria.inria.fr/kuppe/EclipseUndoRedo3.gif
It seems that the undo/redo actions of the editor first editor are not activated after an editor switch. Therefore the undo/redo actions of the previous editor remain active. What happens when more editors are open? Are indeed the undo/redo actions not activated on one editor only? The undo/redo actions should be set when a part gets activated: CompilationUnitEditorActionContributor(BasicTextEditorActionContributor).doSetActiveEditor(IEditorPart) line: 220 CompilationUnitEditorActionContributor(BasicTextEditorActionContributor).setActiveEditor(IEditorPart) line: 255 CompilationUnitEditorActionContributor(BasicJavaEditorActionContributor).setActiveEditor(IEditorPart) line: 180 CompilationUnitEditorActionContributor(BasicCompilationUnitEditorActionContributor).setActiveEditor(IEditorPart) line: 148 CompilationUnitEditorActionContributor.setActiveEditor(IEditorPart) line: 65 EditorActionBars.partChanged(IWorkbenchPart) line: 335 WorkbenchPage.updateActivations(MPart) line: 329 WorkbenchPage$E4PartListener.partActivated(MPart) line: 218 PartServiceImpl$2.run() line: 249 ... Can you validate that this code is still executed for the editor in which the undo/redo actions are not working anymore? Are there any error in the error log that are triggered by switching the editor?
I took the effort to connect a debugger to TLA+. For the default text editor, global actions are installed when a part is activated by BasicTextEditorActionContributor#setActiveEditor and BasicTextEditorActionContributor#doSetActiveEditor. In the TLA case, the TLAMultiPageEditorActionBarContributor is used. In this implementation, the global actions are not installed when a part is activated. If the TLAMultiPageEditorActionBarContributor would have extended MultiPageContributor instead of MultiPageEditorActionBarContributor some of the editor functionality would have been installed. However still much of the text editors would not have been installed. Looking at the code, I am kind of surprised that the old code worked at all. For now I am inclined to say that it is not an Eclipse issue. Did the old version have an undo-history that was shared among multiple editors? That is, would it first undo the last changes in the current editor, and then undo previous edits in another editor?
After all, it does seem to be caused by different behavior of Eclipse. Not sure though if the old or the new behavior is the correct behavior. The old behavior might have hidden bugs in your code. What I observe is that the undo-handler is registered two times, with different active when expressions. Probably, one is a global registered and the other is local to the editor. One expression is a AND expression of LegacyHandlerSubmission and WorkbenchWindowExpression, the other is a AND expression of LegacyEditorActionBarExpression and WorkbenchWindowExpression. The priority of the first expression seems to change in the newer Eclipse versions, therefore the priority of the two handlers is inversed. This inversion might uncover an existing problem in the previously low-priority handler. Will look further when I have some more spare time left.
The double registration happens inside AbstractTextEditor#registerUndoRedoAction, first as the result of setAction with on a command with a keybinding, second in setGlobalActionHandler. The inversion of priority is the result of Bug 426557, that simplified the expression of the first registration which increased its priority.
Thanks for your efforts! After some nire trial-and-error, it seems as if forcefully resetting the actionbar's global handlers with the editor's actions in org.eclipse.ui.part.MultiPageEditorActionBarContributor.setActivePage(IEditorPart) alleviates the issue: > final IActionBars actionBars = getActionBars(); > actionBars.setGlobalActionHandler("undo", editor.getAction("undo")); > actionBars.setGlobalActionHandler("redo", editor.getAction("redo")); https://github.com/tlaplus/tlaplus/commit/be769427fd35b46d6bb0c14a4314df53523e294c
