[bndtools] Undo functionality not working

[kriegfrj]

A few of us had noticed that the undo functionality in Bndtools on Eclipse had gone a git awry. Often (always?) it doesn't work at all.

I think we had assumed that it was an Eclipse bug (on Slack, https://bugs.eclipse.org/bugs/show_bug.cgi?id=564366 was floated as a candidate). But what I've observed doesn't seem to apply to editing Java files - only bnd.bnd files. But I just observed something that might suggest otherwise and seems possibly related to some other weird issues I've seen with bndrun file editing (see #4202 for example). Forgive me for the long-winded description here; I'm not sure if I'll be able to reproduce it so I want to experiment and capture everything I'm doing while I've still got the issue happening.

• I have two bnd.bnd files open in two different editor windows.
• One of them is in a project in my workspace (let us call it "A"). One of them is from a project that is not in my workspace ("B"). * * A and B were both open and dirty (though I'm not sure if they started out dirty). The were in separate windows on separate screens so I could see both at the same time.
• My cursor was in A. I made a change in A and attempted to undo it with Ctrl-Z.
• A did not undo; however I noticed that B changed.
• Further use of Ctrl-Z and Ctrl-Y showed that the undo/redo was being consistently applied to editor B, even though regular keystrokes were being consistently picked up by editor A.
• If I click in editor B and start typing, the regular keystrokes get picked up by editor B rather than editor A. Undo/redo continues to get picked up by editor B.
• If I click back to editor A and start typing, regular keystrokes return to editor A, but undo/redo continues to be applied to editor B.
• I open a third bnd.bnd file from a second workspace project ("C"). Edits now get sent to C; undo/redo continues to get sent to B.
• I save the changes to C; undo/redo still goes to B.
• I close A without saving changes and return editor focus to C; undo/redo still goes to B.
• I save B and return focus to C; undo/redo still goes to B.
• I close B without saving; undo/redo seems to go nowhere (or else is going to an editor that I haven't found yet).
• I re-open A and B and make edits in both. Undos in A again go to B.
• Close B and open another bnd.bnd file from a non-workspace project ("D"). Undo/redo now goes to project D.
• Open B again. Undo/redo goes to B. Now even when editing project D, undo/redo goes to project B.
• Close B. Undo/redo in editor D now seems to go nowhere.
• Open another workspace bnd.bnd file ("E"). All undo/redo now goes to this file. If I close this file, it goes nowhere.

So I think the summary from the above is: whenever you open a new Bnd editor, it seems to steal the "undo focus" from whichever Bnd editor had it. When you close that editor, the focus does not seem to return to any other Bnd editor.

This also explains why some of us had seen it and others not, or why we'd only seen it sometimes - if you freshly open a new file and start editing it, undo will work. But if you often have multiple editors open and switch between them, it won't work.

I just double checked and the same applies to Bndrun editors. There doesn't appear to be any cross-contamination between Bnd and Bndrun editors; it seems to remain "in the family".

It is possible that this is still an Eclipse issue; but it's also possible that we're doing something wrong with our working copy management that has exposed this bug. Not handling working copies properly could also cause other weird issues like #4202. Another possibility is a problem in BndEditModel somewhere.

I recall at some point it used to be the case that when you tried to open
A couple of possibly-related bug fixes that may have contributed:

• Something that I've not been able to find a reference to - I remember at one point when you tried to open a bnd.bnd file that was already open in an editor, it would open a new editor for the same file. This is in contrast to JDT, which attempts to move focus to the editor that is already opened. Now, it seems opening bnd.bnd files also moves focus to the editor that's already open (if any). Was this an active fix from one of us (if so, the fix is possibly related to this issue)? Or was it a side-effect of some other change?
• [bndtools] Editor support for bnd files outside of current workspace #3773 (editor support for Bnd files outside of the workspace)

[kriegfrj]

I can reproduce this issue at will in my Eclipse installation, which is 2020-09. But when I run it out of the Eclipse workspace which uses 2018-12, the issue does not manifest. This makes a bit hard to debug. I'll need to come up with a bndrun that can launch a test Bndtools instance in Eclipse 2020-09 to debug it.

Moreover, I can see that there is no special undo handling implemented in BndSourceEditorPage - it inherits from TextEditor. I tried using a plain text editor in Eclipse 2020-09 to see if I could reproduce the problem (in case it is a bug in the underlying Eclipse), but it doesn't happen.

[kriegfrj]

Just noting that upgrading to Eclipse 2020-12 didn't seem to fix this problem either.

[bjhargrave]

I have also seen the undo not working for a Java source file. I am using 2020-12. So perhaps the issue is something in Eclipse itself since it also happened on a Java source file as well as a Bnd file.

[kriegfrj]

There might be other issues with undo in Java, but this issue with the Bnd(run) editor is very specific and repeatable - the undo command gets redirected to the last editor opened, not in the editor that was active when the command was issued. I can manifest this bug at will with Bnd(run) files, but the same set of steps doesn't manifest with JDT's Java editor or even Eclipse's plain text editor.

There may be some other undo issues with Java, but I suspect that they are not directly related to this issue. I suspect that this is a Bndtools bug.

I've managed to get a resolving Eclipse 2020-09 instance in my Bndtools workspace (thanks to #4479) that I can launch and run through the debugger, so I should be able to get to the bottom of it soon.

[kriegfrj]

Ok, debugging this is proving to be a bit more of a nightmare than I had anticipated. I can reproduce the bug and step through it in the debugger, but the bit of code where I think stuff is going wrong is in EclipseContext which is frequently called for many reasons, so I need to set a conditional breakpoint to get it to stop at the interesting part - otherwise it stops all the time and there's a lot of manual stepping to do. Unfortunately, I hit #4506, but now I have a work-around.

[kriegfrj]

Just making a note on an important finding: given that this bug does not seem to affect other subclasses of TextEditor, I thought perhaps the reason might be that our text editor is a page in a multi-page form editor. To rule this out, I tried creating two PDE projects to see if the PDE manifest source editor had the same problem. For reference, it does not - you can have two PDE editors open for two separate manifests in two projects, and the undo histories remain independent. So it should be possible for this to work as a page in a multi-page form. I will keep digging.

[Gohla]

We ran into the exact same bug as described here. The issue was a missing IEditorActionBarContributor implementation for the editor, which is responsible for setting up actions such as undo/redo with the correct editor when there are multiple editors of the same type open. To fix it, simply add contributorClass="org.eclipse.ui.texteditor.BasicTextEditorActionContributor" to the editor extension in plugin.xml, for example:

<extension point="org.eclipse.ui.editors">
  <editor
    name="Spoofax dynamic editor"
    contributorClass="org.eclipse.ui.texteditor.BasicTextEditorActionContributor"
    class="mb.spoofax.lwb.eclipse.dynamicloading.DynamicEditor"
    id="spoofax.lwb.eclipse.dynamicloading.editor"
  />
</extension>

[kriegfrj]

@Gohla , thank you so much for this contribution[1]! I'll give this a try and hopefully it will fix this longstanding issue. I always thought it should be something simple missing, the difficult part was understanding which something...

[1] Pun intended