Bug 324859

Comment 1 Michael Wenz 2010-09-10 07:23:00 EDT

Some discussions and a first try showed a possible solution. Further investigations necessary.

Comment 2 Michael Wenz 2011-04-07 10:36:43 EDT

We already tried to implement this feature last year. It was still work in progress, but at least partly checked-in. Parts of it needed to be reverted because it introduced a regression (see Bugzilla 326731 - https://bugs.eclipse.org/bugs/show_bug.cgi?id=326731).
The regression appeared whenever commands were executed via the standard EMF command stack and not via the Graphiti command stack used from within the editor.
This can happen e.g. when editing an object from a Graphiti editor from the standard Eclipse property sheet (the table version, not tabbed ones). Issue is, that in this case the Graphiti command stack soes not have the chance to update the second stack that was introduced to maintain the non-EMF history.

Comment 3 Michael Wenz 2011-04-07 11:04:00 EDT

Given the facts described in comment 2 it seems not to be possible to implement the second stack for the non-EMF undo inside GFCommandStack which was the first try.
- Direct implementation in the methods execute, undo, redo fail because they might not be called when command sare triggered from the outside (property sheet)
- Implementation in the class GFCommandStack commandStackChanged method (it is a CommandStackChangedListener) fails because the relevant information to create the non-EMF stack entries is not present. It might be constructed in GFCommmandStack and passed to the listener, but that will only introduce the beforementioned issue again.

Comment 4 Shenxue Zhou 2011-04-07 11:53:12 EDT

I wonder whether we can simplify this complicated problem slightly. If model objects are modified outside Graphiti Diagram editor (either on the model source directly or through property sheet), then it's not Diagram Editor's responsibility to handle undo/redo of those operations. In another word, Graphiti Diagram Editor is only responsible for undo/redo of operations directly invoked within it.

Thoughts?

Comment 5 Michael Wenz 2011-04-08 05:17:32 EDT

(In reply to comment #4)
Shenxue, thanks for this comment. For really external changes this is definitely true, but in this case the property sheet is attached to the diagram editor and the change happens via the (EMF) command stack of the diagram editor. The problem is that as soon as the EMF change is on the EMF command stack, the non-EMF change must also be on the secondary command stack; otherwise we will run into trouble later when we try to undo or redo changes, since we don't know if there is an according non-EMF change to an EMF change.
There might be another solution, though: I'm currently trying to move the secondary non-EMF command stack to our EMF command stack implementation (GFWorkspaceCommandStackImpl); looks quiete promising so far.

Michael

Comment 6 Michael Wenz 2011-04-12 09:47:51 EDT

I was able to move the secondary stack (containing IExecutionInfo objects, basically the executed feature instance along with their context instance) to the class GFWorkspaceCommandStackImpl. Now all changes done via the EMF command stack will create such a secondary stack entry and it will no longer fail becuase of not synchronized stacks.

Comment 7 Michael Wenz 2011-04-12 10:06:14 EDT

The following solution has been checked-in to head:
- Users can implement the new interface ICustomUndoableFeature within their features in case they need to implement additional undo/redo stuff
- In case a feature implements this interface the Graphiti command stack will care about the EMF undo/redo and _additionally_ call the appropriate methods (canUndo and undo resp. canRedo and redo) within the feature.
- Inside the feature coding for those methods users can use the information passed (the executed feature instance will be called with the instance of its context) to undo the operations done in execute
- In the features execute method users might add additional information needed to perform the undo to the context object which will be available later for undo/redo.
- The decision to implement ICustomUndoableFeature can be taken indiviually for each feature
- For the pattern approach a similiar interface has been introduced: ICustomUndoablePattern, for which the beforementioned also applies accordingly
- An automated test is available in the AllJunitUiTests

By introducing this functionality it is now possible for users of Graphiti to implement undo and redo functionalities also for non-EMF domain changes; an additional advantage is that adding the functionality will not influence standard implementors (EMF domain models), although they also might choose to implement the interfaces if they need to.
Note: All changes done inside the Graphiti diagram editor (no matter if EMF changes or non-EMF changes) will write an IExecutionInfo entry to the stack that will be available with the according feature and context for undo/redo. External changes (e.g. changes done from the standard property sheet) will not break the editor, but will not necessarily lead to data being available inside the IExecutionInfo object written:
- EMF changes done on the EMF command stack (e.g. from the standard property sheet) have no associated feature and context. Therefore an empty IExecutionInfo entry will be written. On the other hand all changes done in that case will automatically be undone/redone by the EMF command stack, so there should be no need to do additional stuff
- Non-EMF changes done e.g. from a standard property sheet will naturally not go through the EMF command stack of the editor, so no IExecutionInfo stack entry will be written (in fact also no EMF command stack entry will be written of course). In this case users are responsible to add their own undo/redo functionality relying on whatever technology they use for their domain model.

Michael

Comment 8 Michael Wenz 2011-06-24 09:12:53 EDT

Part of Graphiti Indigo 0.8.0