Home » Modeling » EMF » Determining when a ChangeCommand did not actually change anything
| | | | | |
| Re: Determining when a ChangeCommand did not actually change anything [message #418728 is a reply to message #418721] |
Fri, 25 April 2008 12:42   |
 Eric Rizzo
Messages: 3070
Registered: July 2009 |
Senior Member |
|
|
Ed Merks wrote:
> Eric,
>
> It sounds like your editor would consider itself dirty even if you did a
> successful copy. That can't be so good. It also sounds like it will
> consider itself dirty if you did something and undid it. It sounds like
> you'd want it to check isSaveNeeded before your editor assumes that
> executing a command dirtied the state. Even the Eclipse editors now go
> back to clean state if you undo back to the beginning so it sounds like
> you should even be considering going back to a clean state if you want
> your editors to be just as good as Eclipse's editors. :-P
No, my implementation of isDirty() calls CommandStack.isSaveNeeded() so
undo/redo interact correctly with dirty state.
And we don't support model-level copy/paste in the editor (it makes much
less sense in a forms-based editor like ours than it does in a
tree-based on like the generated editors).
So, do you have any advise for a custom CommandStack where I don't have
to copy+paste all that code from BasicCommandStack? I'm surprised that
the command notification mechanism is so limited, so I'm thinking I
might be missing something.
Eric
> Eric Rizzo wrote:
>> Ed Merks wrote:
>>> Eric,
>>>
>>> Maybe you'd want to specialize the command to throw a special
>>> exception which you could specialize the command stack to ignore.
>>> Then there'd be no command at all on the stack, which seems a bit
>>> better.
>>
>> The problem I see there is that re-implementing
>> BasicCommandStack.execute(Command) would mean a lot of copy-paste
>> coding because the implementation of execute() catches all
>> RuntimeExceptions and calls notifyListners() regardless. My problem is
>> that notifyListeners() notifies my editor which in turn triggers it to
>> mark itself dirty.
>> Is there any clean way for my editor (a CommandStackListener) to know
>> to ignore the notification form commands that threw an exception?
>>
>> Eric
>>
>>
>>
>>> Eric Rizzo wrote:
>>>> Today I was looking for a way to determine if a ChangeCommand has
>>>> actually recorded any changes. The reason for needing this is that
>>>> if the editor is using the CommandStack to determine isDirty() and
>>>> you execute a ChangeCommand that has not recorded any actual
>>>> changes, the editor will think it is dirty even though the model has
>>>> not actually changed. So if I can ask the ChangeCommand if it has
>>>> any changes, I can avoid executing it when the answer is "no."
>>>> All I could find to support this was to subclass ChangeCommand and
>>>> add a new method, hasChanges(), that interrogates its
>>>> ChangeDescription to see if getObjectChanges() is empty. This seems
>>>> like a very convenient thing to be able to do, so is it worth
>>>> entering a bugzilla (enhancement) for? Or is there some flaw in this
>>>> approach?
>>>>
>>>> Eric
|
|
|
| Re: Determining when a ChangeCommand did not actually change anything [message #418729 is a reply to message #418728] |
Fri, 25 April 2008 13:17 |
Ed Merks
Messages: 33218
Registered: July 2009 |
Senior Member |
|
|
This is a multi-part message in MIME format.
--------------090004080302000608070502
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Eric,
You could specialize handleError to ignore rather than log the exception.
It also seems to me there's a bug in BasicCommandStack that's causing
your problem, i.e, the "tricky code is kickingin". I wonder if this
restructured expression wouldn't be better:
public void execute(Command command)
{
// If the command is executable, record and execute it.
//
if (command != null)
{
if (command.canExecute())
{
try
{
command.execute();
// Clear the list past the top.
//
for (Iterator<Command> commands =
commandList.listIterator(top + 1); commands.hasNext();
commands.remove())
{
Command otherCommand = commands.next();
otherCommand.dispose();
}
// Record the successfully executed command.
//
mostRecentCommand = command;
commandList.add(command);
++top;
// This is kind of tricky.
// If the saveIndex was in the redo part of the command
list which has now been wiped out,
// then we can never reach a point where a save is not
necessary, not even if we undo all the way back to the beginning.
//
if (saveIndex >= top)
{
// This forces isSaveNeded to always be true.
//
saveIndex = -2;
}
}
catch (RuntimeException exception)
{
handleError(exception);
mostRecentCommand = null;
command.dispose();
}
notifyListeners();
}
else
{
command.dispose();
}
}
}
I.e., discard the stuff above the top and mess with the saveIndex only
if the command executes successfully...
Eric Rizzo wrote:
> Ed Merks wrote:
>> Eric,
>>
>> It sounds like your editor would consider itself dirty even if you
>> did a successful copy. That can't be so good. It also sounds like
>> it will consider itself dirty if you did something and undid it. It
>> sounds like you'd want it to check isSaveNeeded before your editor
>> assumes that executing a command dirtied the state. Even the Eclipse
>> editors now go back to clean state if you undo back to the beginning
>> so it sounds like you should even be considering going back to a
>> clean state if you want your editors to be just as good as Eclipse's
>> editors. :-P
>
> No, my implementation of isDirty() calls CommandStack.isSaveNeeded()
> so undo/redo interact correctly with dirty state.
> And we don't support model-level copy/paste in the editor (it makes
> much less sense in a forms-based editor like ours than it does in a
> tree-based on like the generated editors).
>
> So, do you have any advise for a custom CommandStack where I don't
> have to copy+paste all that code from BasicCommandStack? I'm surprised
> that the command notification mechanism is so limited, so I'm thinking
> I might be missing something.
>
> Eric
>
>
>> Eric Rizzo wrote:
>>> Ed Merks wrote:
>>>> Eric,
>>>>
>>>> Maybe you'd want to specialize the command to throw a special
>>>> exception which you could specialize the command stack to ignore.
>>>> Then there'd be no command at all on the stack, which seems a bit
>>>> better.
>>>
>>> The problem I see there is that re-implementing
>>> BasicCommandStack.execute(Command) would mean a lot of copy-paste
>>> coding because the implementation of execute() catches all
>>> RuntimeExceptions and calls notifyListners() regardless. My problem
>>> is that notifyListeners() notifies my editor which in turn triggers
>>> it to mark itself dirty.
>>> Is there any clean way for my editor (a CommandStackListener) to
>>> know to ignore the notification form commands that threw an exception?
>>>
>>> Eric
>>>
>>>
>>>
>>>> Eric Rizzo wrote:
>>>>> Today I was looking for a way to determine if a ChangeCommand has
>>>>> actually recorded any changes. The reason for needing this is that
>>>>> if the editor is using the CommandStack to determine isDirty() and
>>>>> you execute a ChangeCommand that has not recorded any actual
>>>>> changes, the editor will think it is dirty even though the model
>>>>> has not actually changed. So if I can ask the ChangeCommand if it
>>>>> has any changes, I can avoid executing it when the answer is "no."
>>>>> All I could find to support this was to subclass ChangeCommand and
>>>>> add a new method, hasChanges(), that interrogates its
>>>>> ChangeDescription to see if getObjectChanges() is empty. This
>>>>> seems like a very convenient thing to be able to do, so is it
>>>>> worth entering a bugzilla (enhancement) for? Or is there some flaw
>>>>> in this approach?
>>>>>
>>>>> Eric
--------------090004080302000608070502
Content-Type: text/html; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<meta content="text/html;charset=ISO-8859-1" http-equiv="Content-Type">
</head>
<body bgcolor="#ffffff" text="#000000">
Eric,<br>
<br>
You could specialize handleError to ignore rather than log the
exception.<br>
<br>
It also seems to me there's a bug in BasicCommandStack that's causing
your problem, i.e, the "tricky code is kickingin". I wonder if this
restructured expression wouldn't be better:<br>
<blockquote><small> public void execute(Command command) <br>
{<br>
// If the command is executable, record and execute it.<br>
//<br>
if (command != null)<br>
{<br>
if (command.canExecute())<br>
{<br>
try<br>
{<br>
command.execute();<br>
<br>
// Clear the list past the top.<br>
//<br>
for (Iterator<Command> commands =
commandList.listIterator(top + 1); commands.hasNext();
commands.remove())<br>
{<br>
Command otherCommand = commands.next();<br>
otherCommand.dispose();<br>
}<br>
<br>
// Record the successfully executed command.<br>
//<br>
mostRecentCommand = command;<br>
commandList.add(command);<br>
++top;<br>
<br>
// This is kind of tricky.<br>
// If the saveIndex was in the redo part of the command list
which has now been wiped out,<br>
// then we can never reach a point where a save is not
necessary, not even if we undo all the way back to the beginning.<br>
//<br>
if (saveIndex >= top)<br>
{<br>
// This forces isSaveNeded to always be true.<br>
//<br>
saveIndex = -2;<br>
}<br>
}<br>
catch (RuntimeException exception)<br>
{<br>
handleError(exception);<br>
<br>
mostRecentCommand = null;<br>
command.dispose();<br>
}<br>
<br>
notifyListeners();<br>
}<br>
else<br>
{<br>
command.dispose();<br>
}<br>
}<br>
}</small><br>
</blockquote>
I.e., discard the stuff above the top and mess with the saveIndex only
if the command executes successfully...<br>
<br>
<br>
Eric Rizzo wrote:
<blockquote cite="mid:fusjia$7gt$1@build.eclipse.org" type="cite">Ed
Merks wrote:
<br>
<blockquote type="cite">Eric,
<br>
<br>
It sounds like your editor would consider itself dirty even if you did
a successful copy. That can't be so good. It also sounds like it
will consider itself dirty if you did something and undid it. It
sounds like you'd want it to check isSaveNeeded before your editor
assumes that executing a command dirtied the state. Even the Eclipse
editors now go back to clean state if you undo back to the beginning so
it sounds like you should even be considering going back to a clean
state if you want your editors to be just as good as Eclipse's
editors. :-P
<br>
</blockquote>
<br>
No, my implementation of isDirty() calls CommandStack.isSaveNeeded() so
undo/redo interact correctly with dirty state.
<br>
And we don't support model-level copy/paste in the editor (it makes
much less sense in a forms-based editor like ours than it does in a
tree-based on like the generated editors).
<br>
<br>
So, do you have any advise for a custom CommandStack where I don't have
to copy+paste all that code from BasicCommandStack? I'm surprised that
the command notification mechanism is so limited, so I'm thinking I
might be missing something.
<br>
<br>
Eric
<br>
<br>
<br>
<blockquote type="cite">Eric Rizzo wrote:
<br>
<blockquote type="cite">Ed Merks wrote:
<br>
<blockquote type="cite">Eric,
<br>
<br>
Maybe you'd want to specialize the command to throw a special exception
which you could specialize the command stack to ignore. Then there'd
be no command at all on the stack, which seems a bit better.
<br>
</blockquote>
<br>
The problem I see there is that re-implementing
BasicCommandStack.execute(Command) would mean a lot of copy-paste
coding because the implementation of execute() catches all
RuntimeExceptions and calls notifyListners() regardless. My problem is
that notifyListeners() notifies my editor which in turn triggers it to
mark itself dirty.
<br>
Is there any clean way for my editor (a CommandStackListener) to know
to ignore the notification form commands that threw an exception?
<br>
<br>
Eric
<br>
<br>
<br>
<br>
<blockquote type="cite">Eric Rizzo wrote:
<br>
<blockquote type="cite">Today I was looking for a way to
determine if a ChangeCommand has actually recorded any changes. The
reason for needing this is that if the editor is using the CommandStack
to determine isDirty() and you execute a ChangeCommand that has not
recorded any actual changes, the editor will think it is dirty even
though the model has not actually changed. So if I can ask the
ChangeCommand if it has any changes, I can avoid executing it when the
answer is "no."
<br>
All I could find to support this was to subclass ChangeCommand and add
a new method, hasChanges(), that interrogates its ChangeDescription to
see if getObjectChanges() is empty. This seems like a very convenient
thing to be able to do, so is it worth entering a bugzilla
(enhancement) for? Or is there some flaw in this approach?
<br>
<br>
Eric
<br>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
<br>
</body>
</html>
--------------090004080302000608070502--
Ed Merks
Professional Support: https://www.macromodeling.com/
|
|
|
| Re: Determining when a ChangeCommand did not actually change anything [message #418793 is a reply to message #418729] |
Tue, 29 April 2008 22:44 |
David Steinberg
Messages: 489
Registered: July 2009 |
Senior Member |
|
|
Hi Eric, hi Ed,
The restructuring Ed mentioned is now done in bug 229498. This bug also
adds an AbortExecutionException to the framework and handles it
appropriately in BasicCommandStack.execute(). So you can just throw that
from your ChangeComand's doExecute().
You didn't even have to open the Bugzilla. ;)
Cheers,
Dave
Ed Merks wrote:
> Eric,
>
> You could specialize handleError to ignore rather than log the exception.
>
> It also seems to me there's a bug in BasicCommandStack that's causing
> your problem, i.e, the "tricky code is kickingin". I wonder if this
> restructured expression wouldn't be better:
>
> public void execute(Command command)
> {
> // If the command is executable, record and execute it.
> //
> if (command != null)
> {
> if (command.canExecute())
> {
> try
> {
> command.execute();
>
> // Clear the list past the top.
> //
> for (Iterator<Command> commands =
> commandList.listIterator(top + 1); commands.hasNext();
> commands.remove())
> {
> Command otherCommand = commands.next();
> otherCommand.dispose();
> }
>
> // Record the successfully executed command.
> //
> mostRecentCommand = command;
> commandList.add(command);
> ++top;
>
> // This is kind of tricky.
> // If the saveIndex was in the redo part of the command
> list which has now been wiped out,
> // then we can never reach a point where a save is not
> necessary, not even if we undo all the way back to the beginning.
> //
> if (saveIndex >= top)
> {
> // This forces isSaveNeded to always be true.
> //
> saveIndex = -2;
> }
> }
> catch (RuntimeException exception)
> {
> handleError(exception);
>
> mostRecentCommand = null;
> command.dispose();
> }
>
> notifyListeners();
> }
> else
> {
> command.dispose();
> }
> }
> }
>
> I.e., discard the stuff above the top and mess with the saveIndex only
> if the command executes successfully...
>
>
> Eric Rizzo wrote:
>> Ed Merks wrote:
>>> Eric,
>>>
>>> It sounds like your editor would consider itself dirty even if you
>>> did a successful copy. That can't be so good. It also sounds like
>>> it will consider itself dirty if you did something and undid it. It
>>> sounds like you'd want it to check isSaveNeeded before your editor
>>> assumes that executing a command dirtied the state. Even the Eclipse
>>> editors now go back to clean state if you undo back to the beginning
>>> so it sounds like you should even be considering going back to a
>>> clean state if you want your editors to be just as good as Eclipse's
>>> editors. :-P
>>
>> No, my implementation of isDirty() calls CommandStack.isSaveNeeded()
>> so undo/redo interact correctly with dirty state.
>> And we don't support model-level copy/paste in the editor (it makes
>> much less sense in a forms-based editor like ours than it does in a
>> tree-based on like the generated editors).
>>
>> So, do you have any advise for a custom CommandStack where I don't
>> have to copy+paste all that code from BasicCommandStack? I'm surprised
>> that the command notification mechanism is so limited, so I'm thinking
>> I might be missing something.
>>
>> Eric
>>
>>
>>> Eric Rizzo wrote:
>>>> Ed Merks wrote:
>>>>> Eric,
>>>>>
>>>>> Maybe you'd want to specialize the command to throw a special
>>>>> exception which you could specialize the command stack to ignore.
>>>>> Then there'd be no command at all on the stack, which seems a bit
>>>>> better.
>>>>
>>>> The problem I see there is that re-implementing
>>>> BasicCommandStack.execute(Command) would mean a lot of copy-paste
>>>> coding because the implementation of execute() catches all
>>>> RuntimeExceptions and calls notifyListners() regardless. My problem
>>>> is that notifyListeners() notifies my editor which in turn triggers
>>>> it to mark itself dirty.
>>>> Is there any clean way for my editor (a CommandStackListener) to
>>>> know to ignore the notification form commands that threw an exception?
>>>>
>>>> Eric
>>>>
>>>>
>>>>
>>>>> Eric Rizzo wrote:
>>>>>> Today I was looking for a way to determine if a ChangeCommand has
>>>>>> actually recorded any changes. The reason for needing this is that
>>>>>> if the editor is using the CommandStack to determine isDirty() and
>>>>>> you execute a ChangeCommand that has not recorded any actual
>>>>>> changes, the editor will think it is dirty even though the model
>>>>>> has not actually changed. So if I can ask the ChangeCommand if it
>>>>>> has any changes, I can avoid executing it when the answer is "no."
>>>>>> All I could find to support this was to subclass ChangeCommand and
>>>>>> add a new method, hasChanges(), that interrogates its
>>>>>> ChangeDescription to see if getObjectChanges() is empty. This
>>>>>> seems like a very convenient thing to be able to do, so is it
>>>>>> worth entering a bugzilla (enhancement) for? Or is there some flaw
>>>>>> in this approach?
>>>>>>
>>>>>> Eric
>
|
|
|
| Re: Determining when a ChangeCommand did not actually change anything [message #418883 is a reply to message #418729] |
Wed, 30 April 2008 23:03 |
Eric Rizzo
Messages: 3070
Registered: July 2009 |
Senior Member |
|
|
Ed Merks wrote:
> Eric,
>
> You could specialize handleError to ignore rather than log the exception.
I don't see how that helps me - I need to keep the command from
modifying the stack.
>
> It also seems to me there's a bug in BasicCommandStack that's causing
> your problem, i.e, the "tricky code is kickingin". I wonder if this
> restructured expression wouldn't be better:
I do think so - should I file a bug about this?
Eric
>
> public void execute(Command command)
> {
> // If the command is executable, record and execute it.
> //
> if (command != null)
> {
> if (command.canExecute())
> {
> try
> {
> command.execute();
>
> // Clear the list past the top.
> //
> for (Iterator<Command> commands =
> commandList.listIterator(top + 1); commands.hasNext();
> commands.remove())
> {
> Command otherCommand = commands.next();
> otherCommand.dispose();
> }
>
> // Record the successfully executed command.
> //
> mostRecentCommand = command;
> commandList.add(command);
> ++top;
>
> // This is kind of tricky.
> // If the saveIndex was in the redo part of the command
> list which has now been wiped out,
> // then we can never reach a point where a save is not
> necessary, not even if we undo all the way back to the beginning.
> //
> if (saveIndex >= top)
> {
> // This forces isSaveNeded to always be true.
> //
> saveIndex = -2;
> }
> }
> catch (RuntimeException exception)
> {
> handleError(exception);
>
> mostRecentCommand = null;
> command.dispose();
> }
>
> notifyListeners();
> }
> else
> {
> command.dispose();
> }
> }
> }
>
> I.e., discard the stuff above the top and mess with the saveIndex only
> if the command executes successfully...
>
>
> Eric Rizzo wrote:
>> Ed Merks wrote:
>>> Eric,
>>>
>>> It sounds like your editor would consider itself dirty even if you
>>> did a successful copy. That can't be so good. It also sounds like
>>> it will consider itself dirty if you did something and undid it. It
>>> sounds like you'd want it to check isSaveNeeded before your editor
>>> assumes that executing a command dirtied the state. Even the Eclipse
>>> editors now go back to clean state if you undo back to the beginning
>>> so it sounds like you should even be considering going back to a
>>> clean state if you want your editors to be just as good as Eclipse's
>>> editors. :-P
>>
>> No, my implementation of isDirty() calls CommandStack.isSaveNeeded()
>> so undo/redo interact correctly with dirty state.
>> And we don't support model-level copy/paste in the editor (it makes
>> much less sense in a forms-based editor like ours than it does in a
>> tree-based on like the generated editors).
>>
>> So, do you have any advise for a custom CommandStack where I don't
>> have to copy+paste all that code from BasicCommandStack? I'm surprised
>> that the command notification mechanism is so limited, so I'm thinking
>> I might be missing something.
>>
>> Eric
>>
>>
>>> Eric Rizzo wrote:
>>>> Ed Merks wrote:
>>>>> Eric,
>>>>>
>>>>> Maybe you'd want to specialize the command to throw a special
>>>>> exception which you could specialize the command stack to ignore.
>>>>> Then there'd be no command at all on the stack, which seems a bit
>>>>> better.
>>>>
>>>> The problem I see there is that re-implementing
>>>> BasicCommandStack.execute(Command) would mean a lot of copy-paste
>>>> coding because the implementation of execute() catches all
>>>> RuntimeExceptions and calls notifyListners() regardless. My problem
>>>> is that notifyListeners() notifies my editor which in turn triggers
>>>> it to mark itself dirty.
>>>> Is there any clean way for my editor (a CommandStackListener) to
>>>> know to ignore the notification form commands that threw an exception?
>>>>
>>>> Eric
>>>>
>>>>
>>>>
>>>>> Eric Rizzo wrote:
>>>>>> Today I was looking for a way to determine if a ChangeCommand has
>>>>>> actually recorded any changes. The reason for needing this is that
>>>>>> if the editor is using the CommandStack to determine isDirty() and
>>>>>> you execute a ChangeCommand that has not recorded any actual
>>>>>> changes, the editor will think it is dirty even though the model
>>>>>> has not actually changed. So if I can ask the ChangeCommand if it
>>>>>> has any changes, I can avoid executing it when the answer is "no."
>>>>>> All I could find to support this was to subclass ChangeCommand and
>>>>>> add a new method, hasChanges(), that interrogates its
>>>>>> ChangeDescription to see if getObjectChanges() is empty. This
>>>>>> seems like a very convenient thing to be able to do, so is it
>>>>>> worth entering a bugzilla (enhancement) for? Or is there some flaw
>>>>>> in this approach?
>>>>>>
>>>>>> Eric
>
|
|
|
| Re: Determining when a ChangeCommand did not actually change anything [message #418886 is a reply to message #418883] |
Wed, 30 April 2008 23:41 |
Ed Merks
Messages: 33218
Registered: July 2009 |
Senior Member |
|
|
Eric,
You were slow to respond, so we already committed the changes in
https://bugs.eclipse.org/bugs/show_bug.cgi?id=229498. If you throw an
AbortExcecutionException, the result will be effectively the same as for
a command that returns false for canExecute.
Eric Rizzo wrote:
> Ed Merks wrote:
>> Eric,
>>
>> You could specialize handleError to ignore rather than log the
>> exception.
>
> I don't see how that helps me - I need to keep the command from
> modifying the stack.
>>
>> It also seems to me there's a bug in BasicCommandStack that's causing
>> your problem, i.e, the "tricky code is kickingin". I wonder if this
>> restructured expression wouldn't be better:
>
> I do think so - should I file a bug about this?
>
> Eric
>
>>
>> public void execute(Command command)
>> {
>> // If the command is executable, record and execute it.
>> //
>> if (command != null)
>> {
>> if (command.canExecute())
>> {
>> try
>> {
>> command.execute();
>>
>> // Clear the list past the top.
>> //
>> for (Iterator<Command> commands =
>> commandList.listIterator(top + 1); commands.hasNext();
>> commands.remove())
>> {
>> Command otherCommand = commands.next();
>> otherCommand.dispose();
>> }
>> // Record the successfully executed command.
>> //
>> mostRecentCommand = command;
>> commandList.add(command);
>> ++top;
>>
>> // This is kind of tricky.
>> // If the saveIndex was in the redo part of the command
>> list which has now been wiped out,
>> // then we can never reach a point where a save is not
>> necessary, not even if we undo all the way back to the beginning.
>> //
>> if (saveIndex >= top)
>> {
>> // This forces isSaveNeded to always be true.
>> //
>> saveIndex = -2;
>> }
>> }
>> catch (RuntimeException exception)
>> {
>> handleError(exception);
>> mostRecentCommand = null;
>> command.dispose();
>> }
>> notifyListeners();
>> }
>> else
>> {
>> command.dispose();
>> }
>> }
>> }
>>
>> I.e., discard the stuff above the top and mess with the saveIndex
>> only if the command executes successfully...
>>
>>
>> Eric Rizzo wrote:
>>> Ed Merks wrote:
>>>> Eric,
>>>>
>>>> It sounds like your editor would consider itself dirty even if you
>>>> did a successful copy. That can't be so good. It also sounds
>>>> like it will consider itself dirty if you did something and undid
>>>> it. It sounds like you'd want it to check isSaveNeeded before your
>>>> editor assumes that executing a command dirtied the state. Even
>>>> the Eclipse editors now go back to clean state if you undo back to
>>>> the beginning so it sounds like you should even be considering
>>>> going back to a clean state if you want your editors to be just as
>>>> good as Eclipse's editors. :-P
>>>
>>> No, my implementation of isDirty() calls CommandStack.isSaveNeeded()
>>> so undo/redo interact correctly with dirty state.
>>> And we don't support model-level copy/paste in the editor (it makes
>>> much less sense in a forms-based editor like ours than it does in a
>>> tree-based on like the generated editors).
>>>
>>> So, do you have any advise for a custom CommandStack where I don't
>>> have to copy+paste all that code from BasicCommandStack? I'm
>>> surprised that the command notification mechanism is so limited, so
>>> I'm thinking I might be missing something.
>>>
>>> Eric
>>>
>>>
>>>> Eric Rizzo wrote:
>>>>> Ed Merks wrote:
>>>>>> Eric,
>>>>>>
>>>>>> Maybe you'd want to specialize the command to throw a special
>>>>>> exception which you could specialize the command stack to
>>>>>> ignore. Then there'd be no command at all on the stack, which
>>>>>> seems a bit better.
>>>>>
>>>>> The problem I see there is that re-implementing
>>>>> BasicCommandStack.execute(Command) would mean a lot of copy-paste
>>>>> coding because the implementation of execute() catches all
>>>>> RuntimeExceptions and calls notifyListners() regardless. My
>>>>> problem is that notifyListeners() notifies my editor which in turn
>>>>> triggers it to mark itself dirty.
>>>>> Is there any clean way for my editor (a CommandStackListener) to
>>>>> know to ignore the notification form commands that threw an
>>>>> exception?
>>>>>
>>>>> Eric
>>>>>
>>>>>
>>>>>
>>>>>> Eric Rizzo wrote:
>>>>>>> Today I was looking for a way to determine if a ChangeCommand
>>>>>>> has actually recorded any changes. The reason for needing this
>>>>>>> is that if the editor is using the CommandStack to determine
>>>>>>> isDirty() and you execute a ChangeCommand that has not recorded
>>>>>>> any actual changes, the editor will think it is dirty even
>>>>>>> though the model has not actually changed. So if I can ask the
>>>>>>> ChangeCommand if it has any changes, I can avoid executing it
>>>>>>> when the answer is "no."
>>>>>>> All I could find to support this was to subclass ChangeCommand
>>>>>>> and add a new method, hasChanges(), that interrogates its
>>>>>>> ChangeDescription to see if getObjectChanges() is empty. This
>>>>>>> seems like a very convenient thing to be able to do, so is it
>>>>>>> worth entering a bugzilla (enhancement) for? Or is there some
>>>>>>> flaw in this approach?
>>>>>>>
>>>>>>> Eric
>>
Ed Merks
Professional Support: https://www.macromodeling.com/
|
|
|
| Re: Determining when a ChangeCommand did not actually change anything [message #418908 is a reply to message #418886] |
Thu, 01 May 2008 14:06 |
Eric Rizzo
Messages: 3070
Registered: July 2009 |
Senior Member |
|
|
Ed Merks wrote:
> Eric,
>
> You were slow to respond, so we already committed the changes in
> https://bugs.eclipse.org/bugs/show_bug.cgi?id=229498. If you throw an
> AbortExcecutionException, the result will be effectively the same as for
> a command that returns false for canExecute.
Excellent - thanks for you and your team's prompt replies and efforts
(as usual).
Eric
> Eric Rizzo wrote:
>> Ed Merks wrote:
>>> Eric,
>>>
>>> You could specialize handleError to ignore rather than log the
>>> exception.
>>
>> I don't see how that helps me - I need to keep the command from
>> modifying the stack.
>>>
>>> It also seems to me there's a bug in BasicCommandStack that's causing
>>> your problem, i.e, the "tricky code is kickingin". I wonder if this
>>> restructured expression wouldn't be better:
>>
>> I do think so - should I file a bug about this?
>>
>> Eric
>>
>>>
>>> public void execute(Command command)
>>> {
>>> // If the command is executable, record and execute it.
>>> //
>>> if (command != null)
>>> {
>>> if (command.canExecute())
>>> {
>>> try
>>> {
>>> command.execute();
>>>
>>> // Clear the list past the top.
>>> //
>>> for (Iterator<Command> commands =
>>> commandList.listIterator(top + 1); commands.hasNext();
>>> commands.remove())
>>> {
>>> Command otherCommand = commands.next();
>>> otherCommand.dispose();
>>> }
>>> // Record the successfully executed command.
>>> //
>>> mostRecentCommand = command;
>>> commandList.add(command);
>>> ++top;
>>>
>>> // This is kind of tricky.
>>> // If the saveIndex was in the redo part of the command
>>> list which has now been wiped out,
>>> // then we can never reach a point where a save is not
>>> necessary, not even if we undo all the way back to the beginning.
>>> //
>>> if (saveIndex >= top)
>>> {
>>> // This forces isSaveNeded to always be true.
>>> //
>>> saveIndex = -2;
>>> }
>>> }
>>> catch (RuntimeException exception)
>>> {
>>> handleError(exception);
>>> mostRecentCommand = null;
>>> command.dispose();
>>> }
>>> notifyListeners();
>>> }
>>> else
>>> {
>>> command.dispose();
>>> }
>>> }
>>> }
>>>
>>> I.e., discard the stuff above the top and mess with the saveIndex
>>> only if the command executes successfully...
>>>
>>>
>>> Eric Rizzo wrote:
>>>> Ed Merks wrote:
>>>>> Eric,
>>>>>
>>>>> It sounds like your editor would consider itself dirty even if you
>>>>> did a successful copy. That can't be so good. It also sounds
>>>>> like it will consider itself dirty if you did something and undid
>>>>> it. It sounds like you'd want it to check isSaveNeeded before your
>>>>> editor assumes that executing a command dirtied the state. Even
>>>>> the Eclipse editors now go back to clean state if you undo back to
>>>>> the beginning so it sounds like you should even be considering
>>>>> going back to a clean state if you want your editors to be just as
>>>>> good as Eclipse's editors. :-P
>>>>
>>>> No, my implementation of isDirty() calls CommandStack.isSaveNeeded()
>>>> so undo/redo interact correctly with dirty state.
>>>> And we don't support model-level copy/paste in the editor (it makes
>>>> much less sense in a forms-based editor like ours than it does in a
>>>> tree-based on like the generated editors).
>>>>
>>>> So, do you have any advise for a custom CommandStack where I don't
>>>> have to copy+paste all that code from BasicCommandStack? I'm
>>>> surprised that the command notification mechanism is so limited, so
>>>> I'm thinking I might be missing something.
>>>>
>>>> Eric
>>>>
>>>>
>>>>> Eric Rizzo wrote:
>>>>>> Ed Merks wrote:
>>>>>>> Eric,
>>>>>>>
>>>>>>> Maybe you'd want to specialize the command to throw a special
>>>>>>> exception which you could specialize the command stack to
>>>>>>> ignore. Then there'd be no command at all on the stack, which
>>>>>>> seems a bit better.
>>>>>>
>>>>>> The problem I see there is that re-implementing
>>>>>> BasicCommandStack.execute(Command) would mean a lot of copy-paste
>>>>>> coding because the implementation of execute() catches all
>>>>>> RuntimeExceptions and calls notifyListners() regardless. My
>>>>>> problem is that notifyListeners() notifies my editor which in turn
>>>>>> triggers it to mark itself dirty.
>>>>>> Is there any clean way for my editor (a CommandStackListener) to
>>>>>> know to ignore the notification form commands that threw an
>>>>>> exception?
>>>>>>
>>>>>> Eric
>>>>>>
>>>>>>
>>>>>>
>>>>>>> Eric Rizzo wrote:
>>>>>>>> Today I was looking for a way to determine if a ChangeCommand
>>>>>>>> has actually recorded any changes. The reason for needing this
>>>>>>>> is that if the editor is using the CommandStack to determine
>>>>>>>> isDirty() and you execute a ChangeCommand that has not recorded
>>>>>>>> any actual changes, the editor will think it is dirty even
>>>>>>>> though the model has not actually changed. So if I can ask the
>>>>>>>> ChangeCommand if it has any changes, I can avoid executing it
>>>>>>>> when the answer is "no."
>>>>>>>> All I could find to support this was to subclass ChangeCommand
>>>>>>>> and add a new method, hasChanges(), that interrogates its
>>>>>>>> ChangeDescription to see if getObjectChanges() is empty. This
>>>>>>>> seems like a very convenient thing to be able to do, so is it
>>>>>>>> worth entering a bugzilla (enhancement) for? Or is there some
>>>>>>>> flaw in this approach?
>>>>>>>>
>>>>>>>> Eric
>>>
|
|
|
| Re: Determining when a ChangeCommand did not actually change anything [message #418936 is a reply to message #418793] |
Sun, 04 May 2008 01:41 |
Eclipse User |
|
|
|
Originally posted by: give.a.damus.gmail.com
Hi, Dave,
Did you try this new exception with a TransactionalCommandStack? I
imagine that it will be caught and handled like any other run-time
exception, which probably isn't good (it results in a scary log
message). Obviously, roll-back would still be desired, but I wonder
what the exact logging etc. behaviour is that we want?
Thanks,
Christian
On Tuesday 04-29-2008 (06:44), Dave Steinberg wrote:
> Hi Eric, hi Ed,
> The restructuring Ed mentioned is now done in bug 229498. This bug
> also adds an AbortExecutionException to the framework and handles it
> appropriately in BasicCommandStack.execute(). So you can just throw
> that from your ChangeComand's doExecute().
> You didn't even have to open the Bugzilla. ;)
> Cheers,
> Dave
-----8<-----
--
I'm trying a new usenet client for Mac, Nemo OS X.
You can download it at http://www.malcom-mac.com/nemo
|
|
|
| Re: Determining when a ChangeCommand did not actually change anything [message #418938 is a reply to message #418936] |
Sun, 04 May 2008 10:14 |
Ed Merks
Messages: 33218
Registered: July 2009 |
Senior Member |
|
|
This is a multi-part message in MIME format.
--------------060608070205010201060309
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Christian,
This exception should be used if and only if the state of the model
hasn't changed, i.e., if no further action needs to be taken. It should
be handled silently (we don't call handleError for it) so that
effectively it is treated as a command that returns canExecute false,
but has decided too late...
Probably this change is an appropriate way of deal with in in the
transaction framework.
### Eclipse Workspace Patch 1.0
#P org.eclipse.emf.transaction
Index:
src/org/eclipse/emf/transaction/impl/AbstractTransactionalCo mmandStack.java
============================================================ =======
RCS file:
/cvsroot/modeling/org.eclipse.emf/org.eclipse.emf.transactio n/plugins/org.eclipse.emf.transaction/src/org/eclipse/emf/tr ansaction/impl/AbstractTransactionalCommandStack.java,v
retrieving revision 1.5
diff -u -r1.5 AbstractTransactionalCommandStack.java
---
src/org/eclipse/emf/transaction/impl/AbstractTransactionalCo mmandStack.java
14 Nov 2007 18:14:00 -0000 1.5
+++
src/org/eclipse/emf/transaction/impl/AbstractTransactionalCo mmandStack.java
4 May 2008 10:09:14 -0000
@@ -22,6 +22,7 @@
import org.eclipse.core.runtime.IStatus;
import org.eclipse.core.runtime.OperationCanceledException;
import org.eclipse.core.runtime.Status;
+import org.eclipse.emf.common.command.AbortExecutionException;
import org.eclipse.emf.common.command.BasicCommandStack;
import org.eclipse.emf.common.command.Command;
import org.eclipse.emf.transaction.ExceptionHandler;
@@ -163,6 +164,8 @@
if ((command != null) && command.canExecute()) {
try {
doExecute(command, options);
*+ } catch (AbortExecutionException e) {
+ command.dispose();*
} catch (RollbackException e) {
handleRollback(command, e);
I suppose an attempt to rollback when there's nothing to do would be
harmless? (So it might be treated like OperationCanceledException.)
Christian W. Damus wrote:
> Hi, Dave,
>
> Did you try this new exception with a TransactionalCommandStack? I
> imagine that it will be caught and handled like any other run-time
> exception, which probably isn't good (it results in a scary log
> message). Obviously, roll-back would still be desired, but I wonder
> what the exact logging etc. behaviour is that we want?
>
> Thanks,
>
> Christian
>
>
> On Tuesday 04-29-2008 (06:44), Dave Steinberg wrote:
>
>> Hi Eric, hi Ed,
>>
>
>
>> The restructuring Ed mentioned is now done in bug 229498. This bug
>> also adds an AbortExecutionException to the framework and handles it
>> appropriately in BasicCommandStack.execute(). So you can just throw
>> that from your ChangeComand's doExecute().
>>
>
>
>> You didn't even have to open the Bugzilla. ;)
>>
>
>
>> Cheers,
>> Dave
>>
>
> -----8<-----
>
>
>
--------------060608070205010201060309
Content-Type: text/html; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<meta content="text/html;charset=ISO-8859-1" http-equiv="Content-Type">
</head>
<body bgcolor="#ffffff" text="#000000">
Christian,<br>
<br>
This exception should be used if and only if the state of the model
hasn't changed, i.e., if no further action needs to be taken. It
should be handled silently (we don't call handleError for it) so that
effectively it is treated as a command that returns canExecute false,
but has decided too late...<br>
<br>
Probably this change is an appropriate way of deal with in in the
transaction framework.<br>
<br>
<small>### Eclipse Workspace Patch 1.0<br>
#P org.eclipse.emf.transaction<br>
Index:
src/org/eclipse/emf/transaction/impl/AbstractTransactionalCo mmandStack.java <br>
============================================================ ======= <br>
RCS file:
/cvsroot/modeling/org.eclipse.emf/org.eclipse.emf.transactio n/plugins/org.eclipse.emf.transaction/src/org/eclipse/emf/tr ansaction/impl/AbstractTransactionalCommandStack.java,v <br>
retrieving revision 1.5<br>
diff -u -r1.5 AbstractTransactionalCommandStack.java<br>
---
src/org/eclipse/emf/transaction/impl/AbstractTransactionalCo mmandStack.java
14 Nov 2007 18:14:00 -0000 1.5<br>
+++
src/org/eclipse/emf/transaction/impl/AbstractTransactionalCo mmandStack.java
4 May 2008 10:09:14 -0000<br>
@@ -22,6 +22,7 @@<br>
import org.eclipse.core.runtime.IStatus;<br>
import org.eclipse.core.runtime.OperationCanceledException;<br>
import org.eclipse.core.runtime.Status;<br>
+import org.eclipse.emf.common.command.AbortExecutionException;<br>
import org.eclipse.emf.common.command.BasicCommandStack;<br>
import org.eclipse.emf.common.command.Command;<br>
import org.eclipse.emf.transaction.ExceptionHandler;<br>
@@ -163,6 +164,8 @@<br>
if ((command != null) && command.canExecute()) {<br>
try {<br>
doExecute(command, options);<br>
<b> +   ; } catch (AbortExecutionException e) {<br>
+   ;   ; command.dispose();</b><br>
} catch (RollbackException e) {<br>
handleRollback(command, e);</small><br>
<br>
I suppose an attempt to rollback when there's nothing to do would be
harmless? (So it might be treated like OperationCanceledException.)<br>
<br>
<br>
<br>
Christian W. Damus wrote:
<blockquote cite="mid:nemoSat050308093914@news.eclipse.org" type="cite">
<pre wrap="">Hi, Dave,
Did you try this new exception with a TransactionalCommandStack? I
imagine that it will be caught and handled like any other run-time
exception, which probably isn't good (it results in a scary log
message). Obviously, roll-back would still be desired, but I wonder
what the exact logging etc. behaviour is that we want?
Thanks,
Christian
On Tuesday 04-29-2008 (06:44), Dave Steinberg wrote:
</pre>
<blockquote type="cite">
<pre wrap=""> Hi Eric, hi Ed,
</pre>
</blockquote>
<pre wrap=""><!---->
</pre>
<blockquote type="cite">
<pre wrap=""> The restructuring Ed mentioned is now done in bug 229498. This bug
also adds an AbortExecutionException to the framework and handles it
appropriately in BasicCommandStack.execute(). So you can just throw
that from your ChangeComand's doExecute().
</pre>
</blockquote>
<pre wrap=""><!---->
</pre>
<blockquote type="cite">
<pre wrap=""> You didn't even have to open the Bugzilla. ;)
</pre>
</blockquote>
<pre wrap=""><!---->
</pre>
<blockquote type="cite">
<pre wrap=""> Cheers,
Dave
</pre>
</blockquote>
<pre wrap=""><!---->
-----8<-----
</pre>
</blockquote>
<br>
</body>
</html>
--------------060608070205010201060309--
Ed Merks
Professional Support: https://www.macromodeling.com/
|
|
|
| Re: Determining when a ChangeCommand did not actually change anything [message #418942 is a reply to message #418938] |
Sun, 04 May 2008 14:56 |
| Eclipse User |
|
|
|
Originally posted by: give.a.damus.gmail.com
Hi, Ed,
Would you mind raising an enhancement request for EMF Transaction,
with this patch? Then I'll be able to test it. And, since this
exception is new in EMF 2.4, I'll have to double-check my plug-in
dependency lower bound :-)
Thanks,
Christian
On Sunday 05-04-2008 (06:14), Ed Merks wrote:
> Christian,
> This exception should be used if and only if the state of the model
> hasn't changed, i.e., if no further action needs to be taken. It
> should be handled silently (we don't call handleError for it) so that
> effectively it is treated as a command that returns canExecute false,
> but has decided too late...
> Probably this change is an appropriate way of deal with in in the
> transaction framework.
> ### Eclipse Workspace Patch 1.0
> #P org.eclipse.emf.transaction
> Index:
> src/org/eclipse/emf/transaction/impl/AbstractTransactionalCo mmandStack
> .java
> ============================================================ =======RCS
> file:
> /cvsroot/modeling/org.eclipse.emf/org.eclipse.emf.transactio n/plugins/
> org.eclipse.emf.transaction/src/org/eclipse/emf/transaction/ impl/Abstr
> actTransactionalCommandStack.java,vretrieving revision 1.5
> diff -u -r1.5 AbstractTransactionalCommandStack.java
> ---
> src/org/eclipse/emf/transaction/impl/AbstractTransactionalCo mmandStack
> .java 14 Nov 2007 18:14:00 -0000 1.5
> +++
> src/org/eclipse/emf/transaction/impl/AbstractTransactionalCo mmandStack
> .java 4 May 2008 10:09:14 -0000
> @@ -22,6 +22,7 @@
> import org.eclipse.core.runtime.IStatus;
> import org.eclipse.core.runtime.OperationCanceledException;
> import org.eclipse.core.runtime.Status;
> +import org.eclipse.emf.common.command.AbortExecutionException;
> import org.eclipse.emf.common.command.BasicCommandStack;
> import org.eclipse.emf.common.command.Command;
> import org.eclipse.emf.transaction.ExceptionHandler;
> @@ -163,6 +164,8 @@
> if ((command != null) && command.canExecute()) {
> try {
> doExecute(command, options);
> *+ } catch (AbortExecutionException e) {
> + command.dispose();*
> } catch (RollbackException e) {
> handleRollback(command, e);
>
> I suppose an attempt to rollback when there's nothing to do would be
> harmless? (So it might be treated like OperationCanceledException.)
> Christian W. Damus wrote:
>> Hi, Dave,
>> Did you try this new exception with a TransactionalCommandStack?
>> I imagine that it will be caught and handled like any other run-time
>> exception, which probably isn't good (it results in a scary log
>> message). Obviously, roll-back would still be desired, but I wonder
>> what the exact logging etc. behaviour is that we want?
>> Thanks,
>> Christian
-----8<-----
--
I'm trying a new usenet client for Mac, Nemo OS X.
You can download it at http://www.malcom-mac.com/nemo
|
|
|
Goto Forum:
Current Time: Wed Sep 25 12:24:01 GMT 2024
Powered by FUDForum. Page generated in 0.07113 seconds |
] 
Search
Help
Register
Login
Home
