gnustep-dev
[Top][All Lists]
Advanced

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

Re: What happened to the code freeze?


From: Doug Simons
Subject: Re: What happened to the code freeze?
Date: Thu, 22 Apr 2010 23:42:55 -0600



On Apr 22, 2010, at 1:41 AM, Fred Kiefer wrote:

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.

Okay, on looking at this more closely now, I think you are right about the call to setSupermenu: while decoding. I'm not sure of exactly what was happening (and don't have time to debug it at the moment), but the supermenu was nil in all of the submenus of our main menu (which is loaded from a nib file). The new call to setSupermenu: in insertItem:atIndex: seems to be enough to fix the problem for me. My guess now is that the root problem is probably in the nib loading code for the menu. But the fix as it stands now shouldn't be a problem I think.

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.

Good point. I've moved the call to menuChanged into all of the individual methods that note the changes.

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?

Moving the one call into [NSMenu setMain:] is reasonable. Unfortunately, the call to updateAllWindowsWithMenu: in _didFinishLaunching is necessary, now that it isn't being called all the time from [NSMenu update]. Without this call the menus never get built. I guess either setMain: isn't being called at launch, or it is called too early to be effective. 

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

Sorry, it looks consistent to me. I'm using Notepad++ on Windows. I guess I'll have to fiddle with the settings and see if I can get it to conform properly. Does anyone know the right settings to use?

Here is my updated version of NSMenu. See what you think.

Doug

Attachment: NSMenu.m
Description: Binary data



reply via email to

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