Re: [Gnustep-cvs] r27874 - in /libs/gui/trunk: ChangeLog Source/NSCell.m

[Fred Kiefer]

Gregory Casamento wrote:
> Author: gcasa
> Date: Mon Feb 16 01:31:23 2009
> New Revision: 27874
> 
> URL: http://svn.gna.org/viewcvs/gnustep?rev=27874&view=rev
> Log:
>       * Source/NSCell.m: Change to implement 10.3 and later behavior for
>       the method setStringValue: as documented in Apple's documentation
>       for the method.  This behavior was observed on Cocoa under 
>       Mac OS 10.5.
> 
> Modified:
>     libs/gui/trunk/ChangeLog
>     libs/gui/trunk/Source/NSCell.m
> 

I don't like this change and I would like to explain why. The first half
of your change, the bit in setObjectValue: is without any external
relevance:

  ///
  // If the thing that was assigned is not a string, but
  // responds to stringValue then get that.
  ///
  if([_object_value respondsToSelector: @selector(attributedStringValue)])
    {
      newContents = [_object_value attributedStringValue];
    }
  else if([_object_value respondsToSelector: @selector(stringValue)])
    {
      newContents = [_object_value stringValue];
    }
        
  newContents = [_object_value description];

Here you first get a value and then override it with the old default. We
really need to clean this up, as the basic idea is ok.

As for the second half of the change it may assign an attributed string
to the _contents ivar without setting
_cell.contents_is_attributed_string to YES. This could break things
horribly. I would suggest that when the formatter is nil we just call
setObjectValue: without looking at that value and try to resolve things
there.

If Apple really allows non-String parameters to setStringValue: this is
rather a bug then a feature. If you insist to add this to GNUstep, it is
fine for me, but at least we should not break working GNUstep code for this.

I make these changes and you can comment on them. OK?
Fred