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

[Fred Kiefer]

Thank you that you finally took the time to inspect the real issue and fix it. 
In the meantime you surely noticed that the analysis you provided below was 
wrong. The NSMenu did copy the NSMenuItems. The bug was in the copy method of 
the item.

It would have been a lot better if you had looked into the issue when it first 
showed up, but everything should be fine now.

Fred


Am 03.06.2011 um 00:24 schrieb Gregory Casamento <address@hidden>:

> Fred,
> 
> It's actually pretty simple to see what's broken in the copyWithZone
> method in NSMenu.m.   That's what I was trying to explain in the
> "Revert".   I don't need to write a test program since it's plain to
> see on inspection that it's incorrect.
> 
> The method does a recursive copy of the menu and simply inserts the
> menu items from the menu being copied into the copy.   As I explained
> in my comment, this causes all of the menus to share all of their
> NSMenuItems, which is not correct.
> 
> It wasn't my intention to do a "silent revert" or anything of that
> nature.  I'm trying right now to fix some issues I'm seeing with the
> Gnome theme and had forgotten about the debate in March.  I'll try to
> fix this problem properly sometime this evening.
> 
> Thanks, GC
> 
> On Thu, Jun 2, 2011 at 5:47 PM, Fred Kiefer <address@hidden> wrote:
>> On 02.06.2011 22:22, Gregory Casamento wrote:
>>> 
>>> Author: gcasa
>>> Date: Thu Jun  2 22:22:39 2011
>>> New Revision: 33234
>>> 
>>> URL: http://svn.gna.org/viewcvs/gnustep?rev=33234&view=rev
>>> Log:
>>> Use the NSArchiver to copy the menu since the copy method does not do a
>>> deep enough copy.
>>> 
>>> Modified:
>>>     libs/gui/trunk/ChangeLog
>>>     libs/gui/trunk/Source/GSThemeMenu.m
>> 
>> Haven't we been through that before?
>> 
>> There was a long mail exchange in March why this code was incorrect and
>> causing trouble to German. At that point I suggested that you report back
>> with a short example program showing what is broken with the [NSMenu
>> -copyWithZone:] method. Instead you changed the code to use copy and now you
>> changed it back and added this comment:
>> 
>> /*
>> * NOTE: The reason the copy or copyWithZone method is not used here is
>> because
>> * it doesn't make a deep copy of the menu.  A deep copy is needed in order
>> to
>> * allow the individual menu items to be used in multiple menus at one time
>> without
>> * interfering with one another's state.
>> */
>> 
>> This doesn't explain anything, nor will it ever help to resolve the issue,
>> if there is any. It just breaks German's code again.
>> 
>> Don't you think that reopening the debate with what ever new evidence you
>> came up with would be a lot more productive then doing a silent revert?
>> 
>> _______________________________________________
>> Gnustep-dev mailing list
>> address@hidden
>> https://lists.gnu.org/mailman/listinfo/gnustep-dev
>> 
> 
> 
> 
> -- 
> Gregory Casamento - GNUstep Lead/Principal Consultant, OLC, Inc.
> yahoo/skype: greg_casamento, aol: gjcasa
> (240)274-9630 (Cell)
> 
> _______________________________________________
> Gnustep-dev mailing list
> address@hidden
> https://lists.gnu.org/mailman/listinfo/gnustep-dev