Setting undo action names where no undo action is recorded
-
I have an application structured in such a way that I often use [NSUndoManager setActionName:] in cases where I haven't actually recorded an undo action in the run loop. The reason for this that that my controller object is the one that uses setActionName: and it is my model object that actually records the undo action. There are cases where the controller object will tell the model object to do something (set a value, for instance), but the model object won't bother doing it, because the new value is the same as the old value. (This generally happens where the user is editing a text field, but doesn't change the value). In these cases, the controller object calls setActionName:, but the model object doesn't actually record an undo action.
The problem is this: the Undo menu is activated in these cases with the name specified in setActionName:, and it acts as though it is undoing something when you choose the menu item. Of course, nothing happens, as no undo operation is there. But it is confusing to have these "no-op" undos on the stack.
I can think of several ways of working around this. Essentially, you would have to ensure in some way that setActionName: does not get called if no undo action will be recorded. But all the methods I can think of are awkward in one way or another (they require either the controller object or the model object to know more than I'd like it to know). And it seems to me that NSUndoManager ought to be able to figure out what to do if setActionName: is called, but no undo action is recorded before the end of the run loop (i.e. just forget about it). -
on 12/30/00 4:14 PM, Ryan Rempel at <ryan.rempel...> wrote:
> I have an application structured in such a way that I often use [NSUndoManager
> setActionName:] in cases where I haven't actually recorded an undo action in
> the run loop. The reason for this that that my controller object is the one
> that uses setActionName: and it is my model object that actually records the
> undo action. There are cases where the controller object will tell the model
> object to do something (set a value, for instance), but the model object won't
> bother doing it, because the new value is the same as the old value. (This
> generally happens where the user is editing a text field, but doesn't change
> the value). In these cases, the controller object calls setActionName:, but
> the model object doesn't actually record an undo action.
My apps are set up the same as yours, except that my model object does
actually record the action. So the Undo command isn't a no-op. It actually
undoes the action back to its original (same) state. This is, after all, the
mirror image of what the user did in the first place when entering a (same)
value.
This behavior manifests itself not only on text fields, but also, for
example, when clicking a radio button that is already selected or choosing a
pop-up list item that is already chosen.
I would be interested in a cleaner way of handling this, too, but it doesn't
strike me as a particularly serious problem. In fact, a case could be made
that the user should be able to undo a repeat action. The user might not
(indeed, probably does not) realize that he/she chose an already-chosen
value, and the failure to be able to "undo" it might cause confusion or fear
that an error occurred.
Here's a potential solution for the text field case: capture the original
text in your action method before telling the model object to do anything
and before setting an action name. You can get the text field's original
value from the window's field editor when the user starts editing and
compare it with the user's final value. There is a delegate method for
learning when the user starts editing a text field, which your window
controller can implement. Actually, since the field editor's original value
still exists until the user tries to commit the new value, you could use the
control:textShouldEndEditing: delegate method, instead, and, if the original
and final values are identical, tell the control to abortEdit instead of
sending a message to the model object and setting an action name. Your
window controller doesn't have to know anything about the model object; it
can just read the incoming and outgoing states of the text field.
I'm not sure can handle the radio button and pop-up list cases without
reading the model object. If there is a way to obtain the original state
from the control, it could be handled by beeping at the user when the user
tries to select a value that is already selected, instead of talking to your
model object and setting an action name. Unfortunately, the state of these
user controls has already changed by the time your window controller's
action method gets invoked. To know which radio button in a multi-button
cluster was previously selected, you would have to read the data value from
your model object. But what's wrong with a window controller knowing where
the model object's data is located? -- that's one of the jobs committed to
the window controller, after all.
But, like I said, it doesn't seem like it's worth all the trouble, and I'm
not sure the behavior you seek is better than the behavior you avoid.
-
Bill Cheeseman, Quechee, Vermont <mailto:<cheeseb...>
The AppleScript Sourcebook
<http://www.AppleScriptSourcebook.com/>
Vermont Recipes-A Cocoa Cookbook
<http://www.stepwise.com/Articles/VermontRecipes/> -
You make several good points. I wonder whether my expectations as a programmer are different than what the user expects. It is quite possible that the user would want to be able to undo what I consider to be a no-op. In a sense, it is not a no-op from the user's point of view, since the user did do something (otherwise my controller wouldn't set an action name at all). So perhaps NSUndoManager is doing the right thing.
The case where I see this the most is in my NSTableViews where I inadvertently put a cell into edit mode, and then don't actually do any editing. I think that this particular case is in fact a no-op even from the user's point of view. But if the user did actually do some editing (for instance, type a character and then delete it), I would agree that that is not a no-op even though the value did not change.
You made several good suggestions as to how to work around this (should I still want to). I think the easiest thing is for my controller object to ask the model object what its current value is before setting a new value. That way, the controller object can decide whether to go ahead even if the two values are the same.
On Saturday, December 30, 2000, at 04:42 PM, Bill Cheeseman wrote:
> My apps are set up the same as yours, except that my model object does
> actually record the action. So the Undo command isn't a no-op. It actually
> undoes the action back to its original (same) state. This is, after all, the
> mirror image of what the user did in the first place when entering a (same)
> value.
>
> This behavior manifests itself not only on text fields, but also, for
> example, when clicking a radio button that is already selected or choosing a
> pop-up list item that is already chosen.
>
> I would be interested in a cleaner way of handling this, too, but it doesn't
> strike me as a particularly serious problem. In fact, a case could be made
> that the user should be able to undo a repeat action. The user might not
> (indeed, probably does not) realize that he/she chose an already-chosen
> value, and the failure to be able to "undo" it might cause confusion or fear
> that an error occurred.
-
Here is how I work around the problem of empty undo groups that get
created by calling -setActionName.
In my document subclass I implement -setActionName:, and register to
receive NSUndoManagerDidOpenUndoGroupNotification notifications:
@interface OODocument : NSDocument
{
NSString *actionName;
}
@end
@implementation OODocument
- (id)init;
{
...
[[NSNotificationCenter defaultCenter] addObserver:self
selector:@selector(undoManagerDidOpenUndoGroup:)
name: NSUndoManagerDidOpenUndoGroupNotification
object:[self undoManager]];
...
}
// Set the action name before you do something that might create an
undo group. If it does, this will set the action name.
// If it doesn't, you won't have set the action name at the end
which would have created an undo group anyway. You should
// set the action name back to nil at the end, in case an undo group
was not created.
- (void)setActionName:(NSString *)newActionName;
{
if (newActionName == actionName)
return;
[actionName release];
actionName = [newActionName retain];
}
- (void)undoManagerDidOpenUndoGroup:(NSNotification *)aNotification;
{
if (actionName != nil)
[[self undoManager] setActionName:actionName];
[self setActionName:nil];
}
My model methods still make sure something has really changed before
calling -prepareWithInvocationTarget:.
And finally in the controller code, I use my new -setActionName:
before making a change, and then set it back to nil afterward:
{
[[anItem document] setActionName:@"Change Title"];
[anItem setTitle:newTitle];
[[anItem document] setActionName:nil];
}
--
Steve -
Your approach got me thinking, and I came up with an alternative, which is to create a setConditionalActionName: method as a category on NSUndoManager. The method checks to see whether an undo group has been created yet. If so, it simply calls setActionName:. If not, it creates (and autoreleases) a helper object. The helper object listens to see whether the undoManager creates an undo group before run loop ends. If so, the helper calls setActionName:, and stops listening. If not, the helper goes away (since it was autoreleased).
This code is not totally robust in the face of manually created autorelease pools or nested undo groups. Manually created autorelease pools would only be a problem if you create an autorelease pool before calling setConditionalActionName: and then release the pool before the relevant undo group is created (in this case, the helper object would go away before doing its work). But autorelease pools created after calling setConditionalActionName: are not a problem.
There are some puzzles about how to handle cases where setConditionalActionName: is called twice in the same run-loop (for the same undoManager). This would occur where your code sets up nested undo groups. There is some possibility that the helper object might set the action name for the wrong undo group in this case. This could occur when setConditionalActionName: is called twice before any undo group is created, or where it is called a second time after an undo group has been created. I could probably be smarter about nested undo groups, but as I don't use them myself, I'll have to leave it for the moment.
Here is the code.
// MiscUndoConditionalActionName.h
//
// Ryan Rempel (<ryan.rempel...>)
// You may use this code as you like.
#import <Foundation/Foundation.h>
@interface NSUndoManager (MiscUndoConditionalActionName)
- (void) setConditionalActionName: (NSString *) aString;
@end
@interface MiscUndoConditionalActionName: NSObject {
NSString *actionName;
}
- (MiscUndoConditionalActionName *) initWithUndoManager:(NSUndoManager *) anUndoManager actionName:(NSString *)anActionName;
@end
// MiscUndoConditionalActionName.m
//
// Ryan Rempel (<ryan.rempel...>)
// You may use this code as you like.
#import "MiscUndoConditionalActionName.h"
@implementation NSUndoManager (MiscUndoConditionalActionName)
- (void) setConditionalActionName: (NSString *) aName
{
if ([self groupingLevel] > 0) {
[self setActionName:aName];
} else {
[[[MiscUndoConditionalActionName alloc] initWithUndoManager:self actionName:aName] autorelease];
}
}
@end
@implementation MiscUndoConditionalActionName
- (MiscUndoConditionalActionName *) initWithUndoManager:(NSUndoManager *) anUndoManager actionName:(NSString *) anActionName;
{
[super init];
actionName = [anActionName copy];
[[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(handleUndoGroupDidOpenNotification:) name:NSUndoManagerDidOpenUndoGroupNotification object:anUndoManager];
return self;
}
- (void) handleUndoGroupDidOpenNotification:(NSNotification *) aNotification
{
[[NSNotificationCenter defaultCenter] removeObserver:self];
[(NSUndoManager *) [aNotification object] setActionName:actionName];
}
- (void) dealloc
{
[[NSNotificationCenter defaultCenter] removeObserver:self];
[actionName release];
[super dealloc];
}
@end
On Sunday, December 31, 2000, at 11:06 AM, Steve Nygard wrote:
> Here is how I work around the problem of empty undo groups that get created by calling
> -setActionName.
>
> [snip some useful code]
