Community
Participate
Working Groups
Created attachment 281598 [details] Project for reproducing the issue 0. Install Wild Web Developer from SimRel 2019-12 into Eclipse SDK 4.14. 1. Import the attached project `foo`. 2. Open `Foo.java`. 3. Refactor -> Rename... on `Foo` and rename `Foo` to `Bar`. 4. Source -> Generate Element Comment on `Bar`. 5. Open `plugin.xml`. 6. Slowly start typing an XML comment, `<!--`. 7. Activate `Package Explorer` view. 8. Try to Edit -> Undo Rename Type. Observe that it does not do anything. The reason is that WorkspaceUndoMonitor has flushed the history for the workspace undo context at step 6. More Information: Currently, WorkspaceUndoMonitor considers any resource change a change worth tracking. That includes marker changes. Unlike the usual Eclipse approach where marker changes are produced by project builders, a new breed of tools based on the Language Server Protocol (LSP) tend to produce marker changes on each document change (e.g., typing). This quickly increments the number of resource changes tracked by WorkspaceUndoMonitor to CHANGE_THRESHHOLD (currently, 10), which causes checking the operation history. Possible Fix: As stated by a comment in the WorkspaceUndoMonitor#getResourceChangeListener() method, "We can be more specific later if warranted". I propose to ignore any marker changes in the resource change listener.
Sounds reasonable Vladimir. Can you upload a Gerrit for that?
(In reply to Lars Vogel from comment #1) > Sounds reasonable Vladimir. Can you upload a Gerrit for that? Thanks for the quick response Lars. I will try to do it during the upcoming week.
New Gerrit change created: https://git.eclipse.org/r/156658
May be we should move it to the LSP4E domain? The uploaded change may fix the described scenario, but it adds more complexity to the WorkspaceUndoMonitor that already has some distance from being perfect.
Isn't there a flag or something to describe that a given Workspace operation shouldn't be considered in the undo stack? Indeed, for the case of LSP4E at least, the markers don't need to affect the undo history. I see for instance POST_AUTO_BUILD is ignored while POST_BUILD is honored, so it seems to imply that only operations that are triggered by users should be part of the undo stack. It usually not the case for marker manipulation, so it would seem OK to me to ignore marker changes from the undo stack in general.
Setting to 4.16 M1 so that I get reminded to review.
Gerrit change https://git.eclipse.org/r/156658 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=1bbeaae37a13d93cea30c163b172e826a771d4b1
Mickael, as you merged this, please add to N&N. Vladimir, sorry for never keeping my promisse to review your change. Looks like your change convinced Mickael. :-)
(In reply to Lars Vogel from comment #8) > Mickael, as you merged this, please add to N&N. This is a bugfix. It IMO doesn't qualify for N&N.
(In reply to Lars Vogel from comment #8) > Vladimir, sorry for never keeping my promisse to review your change. Looks > like your change convinced Mickael. :-) Thank you! :-)
(In reply to Mickael Istria from comment #9) > (In reply to Lars Vogel from comment #8) > > Mickael, as you merged this, please add to N&N. > > This is a bugfix. It IMO doesn't qualify for N&N. Fine for me, I just wanted to avoid that we surprise users / adapters with an undocumented behaviour change. But I leave the decision to you.
