Re: [PATCH (revised)] -setTarget: and -setAction of NSImageView

[Alexander Malmberg]

Kazunobu Kuriyama wrote:
[snip]
> Could someone please check it and judge whether it is acceptable
> or not?

Well, having had time to think about it, I think the general idea is ok,
although the situation is unfortunate. Detailed comments:

> --- gui/Headers/AppKit/NSImageCell.h    2003-12-29 13:57:42.000000000 +0900
[snip]
> +  int _tag;
> +  id _target;
> +  SEL _action;
> +  id _control_view;

However, the general idea was to add target and action. Adding a tag and
a control view effectively turns NSImageCell completely into an
NSActionCell, it isn't required to get dragging working, and it isn't in
apple's docs (but then, target and action aren't, either).

However, I think adding them now is the right thing to do. The cost
(amount of code) is low, it makes NSImageCell more useful, and by adding
all the stuff at once, we won't have to change the ivar layout/encoding
format twice if it turns out that we want them later (eg. because apple
really did add them, or will).

> +// Target and Action
> +//
> +// N.B: These methods are overridden so that the instances doesn't raise an
> +//      exception when they receive a target or action message.

This comment is very misleading (in the same way that confused the
original discussion about this). The methods are overridden because
NSImageCell has a target and an action, unlike NSCell. The fact that the
methods don't throw exceptions is just a side-effect of this; it is not
the purpose of the change.

> +  [aCoder encodeConditionalObject: _control_view];

The control view shouldn't be encoded. (NSActionCell used to get this
wrong; I'm guessing that's where it's from.)

> --- gui/Source/NSImageView.m    2003-12-29 13:57:43.000000000 +0900

> +  if ([sender draggingSource] == self || ![self isEditable])

-prepareForDragOperation: already checks this, so checking it again here
is unnecessary.


Anyway, unless there are any other ideas about what to do about this,
I'll commit this (with the above issues fixed) soon.

- Alexander Malmberg