Bug 191754 - [undo] SSE Undo stack different from standard operation history
Summary: [undo] SSE Undo stack different from standard operation history
Status: NEW
Alias: None
Product: WTP Source Editing
Classification: WebTools
Component: wst.sse (show other bugs)
Version: 3.1   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: Future   Edit
Assignee: Nitin Dahyabhai CLA
QA Contact: Nitin Dahyabhai CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-06-08 14:13 EDT by Michael Valenta CLA
Modified: 2023-06-08 00:53 EDT (History)
13 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Valenta CLA 2007-06-08 14:13:25 EDT
Without WST loaded, if you make a change in a text editor (or other editor like Compare or plug-in manufest) to an xml file and then undo it using the action or CTRL-Z, the editor will revert to being clean. If I have WST loaded, the undo does not revert the editor to clean. 

The problem is caused by the fact that WST uses deprecated API to register a document provider at the file buffer level and provides a total rewrite of the IDocument which has some subtle behavior differences from the IDocument implementation provided by the Platform.
Comment 1 Nitin Dahyabhai CLA 2007-06-08 14:44:55 EDT
Agreed, we'd rather be using the provided implementations as best we can.
Comment 2 Nitin Dahyabhai CLA 2007-06-11 11:51:35 EDT
Michael, have you spotted any places where we violate the IDocument* contract?  I'm not sure where to start looking for these "subtle differences" you mentioned.
Comment 3 Michael Valenta CLA 2007-06-11 12:12:39 EDT
The only subtle difference I know of is from the scenario I mentioned in my original comment (i.e. CTRL-Z will reset the editor to clean using the Platform IDocument implementation but will not using the WTP IDocument implementation) but I wouldn't be surprised if there are more given that you have two entirely separate implementations that are, is essence, doing the same thing. I don't know enough about the Platform implementation of IDocument to say what sequence of event notifications is required to get the expected behavior for the Undo case. 

Dani, do you have any insight to offer here?
Comment 4 Dani Megert CLA 2007-06-12 02:41:14 EDT
It might not just be different document and/or event flow but how the IOperationHistory is used, which undo context is taken and how the undo/redo actions are set up.

Things to check besides IDocument implementation:
  DocumentUndoManager
  TextViewerUndoManager
  AbstractTextEditor.createUndoRedoActions()
Comment 5 Nitin Dahyabhai CLA 2009-02-03 11:09:26 EST
Modifying the summary to reflect the current state.  Bug 239115, covering just the breakage in the Ant and manifest editors, has been fixed, but the findamental difference between how our Undo stack and how the platform's standard Undo stack are implemented remains.
Comment 6 Ian Tewksbury CLA 2010-02-16 16:27:26 EST
Not really sure what the purpose of this bug is anymore.  If its the description then I think it has run its course and its time to resolve.  If the purpose is comment #4 then that's a bit vague and needs to be clarified for this bug having any chance of going anywhere.
Comment 7 David Carver CLA 2010-02-16 16:40:41 EST
(In reply to comment #6)
> Not really sure what the purpose of this bug is anymore.  If its the
> description then I think it has run its course and its time to resolve.  If the
> purpose is comment #4 then that's a bit vague and needs to be clarified for
> this bug having any chance of going anywhere.

Ian, basically my understanding is that the SSE's undo behavior, behaves differently than the standard text editors.   We need to make sure that our SSE based editors undo functionality is behaving the same way and not using any deprecated API.
Comment 8 Nitin Dahyabhai CLA 2010-02-23 11:06:35 EST
Bug must remain open.  Right now we essentially have two undo stacks operating simultaneously that are not synchronized--an Undo call on one is not recorded as an Undo in the other.
Comment 9 Dawid Pakula CLA 2020-04-01 06:11:36 EDT
I found this task during looking investigation for bug 551827.

Undo/redo inside WTP HTML editor sometimes destroy code. StructuredTextViewerUndoManager is really necessary?
Comment 10 Dawid Pakula CLA 2020-04-01 06:56:57 EDT
As I see main difference is ability to record and label computed changes for concrete document portion.

But this ability is hidden by IUndoManager and generally not used on UI, at least on HTML/CSS/XML editors. Maybe it's time to drop it completely and do not reinvent circle ;)
Comment 11 Eclipse Genie CLA 2020-04-01 13:52:09 EDT
New Gerrit change created: https://git.eclipse.org/r/160320
Comment 12 Nitin Dahyabhai CLA 2020-04-01 14:05:50 EDT
The other big difference, and this isn't in the UI side so much as the headless, is that the current undo stack can be paused; iirc it was related to EMF2DOM adapters, the usage stats for which I'm not privy to.
Comment 13 Dawid Pakula CLA 2020-04-01 14:22:55 EDT
(In reply to Nitin Dahyabhai from comment #12)
> The other big difference, and this isn't in the UI side so much as the
> headless, is that the current undo stack can be paused; iirc it was related
> to EMF2DOM adapters, the usage stats for which I'm not privy to.

Other differences:
1. Nested begin/end calls
2. Due nested support, ability to force-break change session
3. Access to binded Commands via CommandStack
4. Overriden Command labels in UI

Point 1 and 2 could be easy implemented via specialised proxies, but 3 and 4 probably should dropped.
Comment 14 Dawid Pakula CLA 2020-06-22 17:51:01 EDT
Can we continue during 2020-09 phase?
Comment 15 Nitin Dahyabhai CLA 2020-06-22 20:43:12 EDT
What's point 3 from comment 13?