[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: What happened to the code freeze?

From: Fred Kiefer
Subject: Re: What happened to the code freeze?
Date: Thu, 22 Apr 2010 09:41:15 +0200
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; de; rv: Gecko/20100317 SUSE/3.0.4-1.1.1 Thunderbird/3.0.4

Great that you send this change to the list so we can discuss it in
advance. Anything that touches the system that deep should better be
analysed by many developers even when not in feature freeze mode.

The changes to NSMenuItem are so obvious, why didn't we do it that way
in the first place?

You change to set the super menu on a decoded submenu in NSMenu is most
likely wrong. This value already gets set in NSMenuItem +setSubmenu: and
before doing so the code checks that the old super menu is nil. I don't
see how your code could have ever worked.
If there was an issue with the super menu not being set correctly we
need to investigate it. I don't think this is the right fix for it.

I like all the simplifications you did for the menu update for the in
window menu. Why not hide all the details in the
setMenuChangedMessagesEnabled: method? That is have the change passed up
to the super menu and when we are the app menu, do what is needed there?
You will then miss out on some more menu changes, for example when the
main menu doesn't have autoenabled items. Looks like I need to think
about this some more.

The changes to NSApplication I don't like. Why should the application be
concerned about the display of the main menu at all? At least one of
these could be moved into NSMenu -setMain: and perhaps that change could
take care of the second call as well?

Your patch also seems to have an indentation problem are you using tabs?

Any changes that affect only the Windows backend aren't that
problematic. They wont be noticed by most people running GNUstep. :-)


Am 21.04.2010 21:26, schrieb Doug Simons:
> Okay, here are the files I updated for this fix to NSMenu. These are based on 
> r30210 if you want to do a diff to see what's changed. Or drop them in and 
> see 
> how it works for you (Eric, I agree that this should help with the scroll 
> wheel 
> in Ink. Let me know. Also, try typing very fast in a text view before and 
> after 
> applying this change).
> Fred and anyone else who takes a look: let me know in the next day or so if 
> you 
> see any issues here or have any questions. I'll hold off checking them in for 
> the moment.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]