Community
Participate
Working Groups
The problem is that executing the operation passed to open operation might not be executed successful. However clients should close an operation even if it failed from what I interpret into the API.
Checking the operation will not work for the refactoring case since the actual undo object will only be available after refactoring has closed the operation. So closeOperation should provide a paramter to indicate success or failure.
Ideally, the sequence after an open/close should be similar to that of an execute. Since open/close leaves the client in charge of executing, it's reasonable for the close operation to specify whether there was success or failure. One difference is that execute checks the operation's #canUndo before adding it to the history (see bug #84444), but in this case you do not want this behavior. To summarize: - you would like a boolean parameter on closeOperation to indicate whether the operation successfully executed. - if this parameter is false (failure) than the history should send an OPERATION_NOT_OK notification instead of a DONE notification. - if this parameter is true (success) than the history should send a DONE notification. - if this parameter is true, than the history should add the operation to the undo history and send an OPERATION_ADDED notification, **independent of the #canUndo state of the operation** This last part seems inconsistent with bug #84444 and I'd rather treat the operation similarly to #execute, but if there is no way for you to work around this, i can accept it, since this whole API is for legacy integration. Please confirm.
Susan, you are correct. This is inconsistent to bug 84444 and we should try to make it more consistent. The problem in the old refactoring API is that when I get the performed call which must call the closeOperation method I don't have the undo object yet. To make this API more consistent I propose to add two extra parameters to closeOperation: - executionSuccessful: a specified by you - addToUndoStack: if this one is true the operation is added to the undo history and the framework will do the canUndo verification. If false, nothing is added. The parameter will be ignored if executionSuccessful is false. The refactoring framework will push the operation onto the undo stack when it receives the corresponding addUndo call by forwarding it to IOperationHistory#add.
This seems reasonable. I will implement as suggested. (avoiding the temptation to say...hmmm...should #execute then also take a parameter for whether to add to the stack?...) ;-)
Having this for #execute would make sense, but adding it seems to a overkill. If someone doesn't want to add it to the history during execute he can use open/close.
Agree it's overkill for now. Execute was first added to the API to support older frameworks such as GEF and EMF commands who had no #add protocol and always implied an add inside the execute. If it's needed later, someone can request it. I'll implement close as you suggest. Will annotate this bug when it's committed.
Implemented for I20050301 and beyond. Released to HEAD on 2/25/2005.
this bug has been fixed in M6 cycle.
verified on I20050330-0500
