ravynos icon indicating copy to clipboard operation
ravynos copied to clipboard

Transparently init app menu when not present

Open mszoek opened this issue 8 months ago • 14 comments

Menus are normally defined in the app's nib file. For a nibless app, the menus can be created from code, but the Application menu (the bold one with the app's name and the quit option) should always be created automatically if one is not provided.

Move something like this into NSMenu's init or NSApp's setMainMenu:

        // Hack up an application menu since we don't load a nib
        NSMenu *appMenu = [[NSMenu alloc] initApplicationMenu:@"Client Demo"];
        NSMenuItem *appMenuItem = [NSMenuItem new];
        [appMenuItem setTitle:@"Client Demo"];
        [appMenuItem setSubmenu:appMenu];
        [menu insertItem:appMenuItem atIndex:0];
        [NSApp setMainMenu:menu];

mszoek avatar Apr 23 '25 13:04 mszoek

Consider the following prototype:

if (!menu) {
    NSLog(@"no menu to add to is app headless?");
    return; // Handle the error appropriately
}

// inputs need to be fleshed out (perhaps a type alias pattern? NSString for now)
NSString *_input_AppDisplayName = @"Client Demo"; // might support locales in future here?

// Hack up an application menu since we don't load a nib
NSMenu *appMenu = [[NSMenu alloc] initWithTitle:_input_AppDisplayName];
if (!appMenu) {
    NSLog(@"Failed to allocate application menu without nib.");
    // TODO: review release/cleanup of _input_AppDisplayName to avoid memory leaks
    return; // Handle the error appropriately
}

// Create a menu item for the application menu
NSMenuItem *appMenuItem = [[NSMenuItem alloc] initWithTitle: _input_AppDisplayName action:nil keyEquivalent:@""];
if (!appMenuItem) {
    NSLog(@"Failed to create application menu item.");
    // TODO: review release/cleanup of _input_AppDisplayName, and appMenu to avoid memory leaks
    return; // Handle the error appropriately
}

// Set the submenu for the application menu item
[appMenuItem setSubmenu:appMenu];

// Insert the application menu item at the beginning of the main menu
[menu insertItem:appMenuItem atIndex:0];

// TODO: review release/cleanup of _input_AppDisplayName, appMenuItem, to avoid memory leaks

// Set the main menu for the application
[NSApp setMainMenu:menu];

// TODO: review release/cleanup of _input_AppDisplayName, appMenuItem, to avoid memory leaks

I'm still familiarizing myself with the project but this part looks interesting to me.

TODOs not mentioned in code: add menuItems expected:

  • localized "Quit" with command+Q key equivalent
  • localized "settings" with command+comma key equivalent
  • localized hide/show items (if show/hide supported)
  • dividers (de-duplicated) between sections
  • optional "about {name}" item to call about-panel forward

reactive-firewall avatar Aug 22 '25 00:08 reactive-firewall

It's a weird corner case for a GUI app to not have a .nib, which is what this issue covers. Normally any GUI app is created with Xcode and it automatically populates a nib with main menus. CLI apps of course don't have one, and they don't need it as they never become the active app in the menu bar (AFAIK) - they just run in a Terminal. But we don't have an Xcode yet and many of the ravynOS apps do not have nibs, so this helps remove some boilerplate code. And will help with running non-Cocoa GUI apps, I think.

BTW, ARC will take care of the release/dealloc of any ObjC objects here, so don't worry about memory leaks. They're destroyed once nobody holds a reference to them.

mszoek avatar Sep 09 '25 22:09 mszoek

BTW, ARC will take care of the release/dealloc of any ObjC objects here, so don't worry about memory leaks. They're destroyed once nobody holds a reference to them.

Oh, good to know, I hadn't yet found documentation on the topic, and I wasn't sure if Objective-C 2 ARC support was implemented, does that mean it is safe to use @autorelease {} syntax too?

But we don't have an Xcode yet and many of the ravynOS apps do not have nibs, so this helps remove some boilerplate code. And will help with running non-Cocoa GUI apps, I think.

The reason we use frameworks a libraries, indeed!

Simplicity does not precede complexity, but follows it. Alan Perlis

While I'm not sure about how to plug this boiler-plate code into the project, I'd like to take a crack at drafting an implementation based on the ravynOS's AppKit API (e.g., from the headers in "NSMenu.h" etc.)

I'll start from the previously suggested snippet, and draft a typical framework style header stub for the string constants too.

I typically use docc and markdown to document my code, but I'll try to stick to doxygen tagglets where I remember them (it's been a few years for me though)

reactive-firewall avatar Sep 11 '25 05:09 reactive-firewall

Oh, good to know, I hadn't yet found documentation on the topic, and I wasn't sure if Objective-C 2 ARC support was implemented, does that mean it is safe to use @autorelease {} syntax too?

Yes! I use that all the time. You can make sure ARC is enabled by using -fobjc-arc in your compiler flags, although it should be on by default. Test it by trying to call [obj release] - you should get a build error when ARC is on.

While I'm not sure about how to plug this boiler-plate code into the project, I'd like to take a crack at drafting an implementation based on the ravynOS's AppKit API (e.g., from the headers in "NSMenu.h" etc.)

I'll start from the previously suggested snippet, and draft a typical framework style header stub for the string constants too.

Sounds great! This should probably go in [NSApp setMainMenu]. That's what you call to attach the new menus to your app, and they should be fully formed when that is called. (Not true of [NSMenu init] which might be only part of the full set.)

I typically use docc and markdown to document my code, but I'll try to stick to doxygen tagglets where I remember them (it's been a few years for me though)

That's fine. We don't really have a doc generation system yet. Anything you see is inherited from other code. There aren't really tests either :) I'm still treating this mostly like a big POC.

mszoek avatar Sep 11 '25 13:09 mszoek

  • localized "Quit" with command+Q key equivalent

  • localized "settings" with command+comma key equivalent

  • localized hide/show items (if show/hide supported)

  • dividers (de-duplicated) between sections

  • optional "about {name}" item to call about-panel forward

Quick notes on these:

  • Please do use the standard Mac keys (Cmd-Q, Cmd-Comma, etc). We want the consistent feel.
  • AppKit gets Mac key symbols from WindowServer so you can use Command and Option. There's no weird mappings and the key event modifier flags will be set correctly for NSCommandKeyMask, NSShiftKeyMask, and so on. Command should be your keyboards "logo" key if you have one and Option should be the "Alt" or "Option" key. This isn't tested extensively on non-US keyboards or ones without logo keys so YMMV.
  • You'll also have to construct the About panel in code if there's no nib.

mszoek avatar Sep 11 '25 14:09 mszoek

Yes! I use that all the time. You can make sure ARC is enabled by using -fobjc-arc in your compiler flags, although it should be on by default. Test it by trying to call [obj release] - you should get a build error when ARC is on.

TL;DR - actually I have a shim for that

:thinking: I learned back when NeXT was still separate from Apple and got into the habbit of using retain release/autorelease before Objective-C v2+ so when I migrated to ARC support I shimmed my code like this:

/* Header snippet is too small to claim copyright, but for "PAK_" namespace you may consider it MIT - Mr. Walls (Author) */
#if !__has_feature(objc_arc)

#if !defined(PAK_RETAIN)
#define PAK_RETAIN(A)	([(A) retain])
#endif /* end PAK_RETAIN */
#if !defined(PAK_RELEASE)
#define PAK_RELEASE(A)	({ [(A) release]; })
#endif /* end PAK_RELEASE */
#if !defined(PAK_AUTORELEASE)
#define PAK_AUTORELEASE(A)	([(A) autorelease])
#endif /* end PAK_AUTORELEASE */

#else /* !__has_feature(objc_arc) */

#if !defined(PAK_RETAIN)
#define PAK_RETAIN(A) (A)
#endif /* end PAK_RETAIN */
#if !defined(PAK_RELEASE)
#define PAK_RELEASE(A)
#endif /* end PAK_RELEASE */
#if !defined(PAK_AUTORELEASE)
#define PAK_AUTORELEASE(A) (A)
#endif /* end PAK_AUTORELEASE */

#endif /* end __has_feature(objc_arc) */

Then one can just use it as normal in init functions, e.g., self = PAK_RETAIN([self init]); and both runtimes with/without are handled.

Sounds great! This should probably go in [NSApp setMainMenu]. That's what you call to attach the new menus to your app, and they should be fully formed when that is called. (Not true of [NSMenu init] which might be only part of the full set.)

:thinking: I concur; I'll aim to hook in there then, (ATM I'm thinking as a fallback on nil/nibless issues by calling helper from a new ravnOS NSApplication category extension)

Please do use the standard Mac keys (Cmd-Q, Cmd-Comma, etc). We want the consistent feel.

:+1: thanks for confirming

Command should be your keyboards "logo" key if you have one and Option should be the "Alt" or "Option" key. This isn't tested extensively on non-US keyboards or ones without logo keys so YMMV.

I'll make a comment noting the assumption that NSCommandKeyMask is to be fetched from a TBD future helper that maps meta-to-clover behavior for non-clover command type keyboards and leave that up-to the NSEvents codebase for now. (allowing us to side-step the issue for now, and support the targeted consistent use-case)

You'll also have to construct the About panel in code if there's no nib.

Expected, but I might not get to this, I'll at-least provide the hook-logic for the menu (but may have it just log a "not implemented" message and return for now) :shrug:

That's fine. We don't really have a doc generation system yet. Anything you see is inherited from other code. There aren't really tests either :) I'm still treating this mostly like a big POC.

:open_mouth: I admire your ambition here.


I'll ping you when I have a draft for you to review.

reactive-firewall avatar Sep 11 '25 22:09 reactive-firewall

All of that sounds great, and LOVE the story about the shim! Dead on about the future keymap prefs panel and all that. It's really great to have someone here who is strong in ObjC!

mszoek avatar Sep 11 '25 23:09 mszoek

Supplemental Update

(no response needed, just keeping you in the loop)

So after getting in deep in the code I found the NSMenu already had a helper for these kinds of menus:

https://github.com/ravynsoft/ravynos/blob/a1330219b84d2d3498bbadeb8c9322a0861e041d/Frameworks/AppKit/NSMenu.subproj/NSMenu.m#L80-L86

So I'll pivot my plan and integrate those kinds of helper functions (and expand this one) over in NSMenu before hooking in-to NSApplication setMainMenu (when passed a nil menu) as it seems more organized that way.

New Plan:

  • in NSApplication setMainMenu:

    • logic for default menu when given nil menu
    • set help menu call logic glue (will use NSApplication helpmenu bindings)
    • custodial comments about unimplemented locking/object ownership/graphics notifications/etc.
  • in NSMenu:

    • new category for helper functions:
      • app menu (deprecate as chain call & move/refactor in category for maintainability)
      • edit menu (partial implementation only, focusing on binding to NSApp (e.g., not font management))
      • view menu (partial implementation focused on show/hide/zoom bindings)
      • window menu helper for linking/stub (this menu is typically delegated anyway)
      • help menu helper (will use NSApplication helpmenu bindings as source)
    • etc. for keybinding helpers

See also the opportunistic GHI #518 I've opened to separately track some cleanup while I'm in this part of the code (albeit unrelated to this GHI's goals)


It's really great to have someone here who is strong in ObjC!

LOL :laughing: Likewise! The Objective-C runtime port is what first caught my eye on this project (though I have avoided looking at it because you mentioned somewhere it was derived from GNUStep's at some point); Most folks assume objective-C is Apple only or prefer GNUstep (and the GPL virus :speak_no_evil:)

I also admire the shear ambition of such a large under-taking.

reactive-firewall avatar Sep 12 '25 21:09 reactive-firewall

@mszoek Friendly Ping!

Please do use the standard Mac keys (Cmd-Q, Cmd-Comma, etc). We want the consistent feel.

Done and Ready for review of initial code changes in Pull-Request #520 when you have the time.

I just need some feedback on the code relevant to this task (but you are of-course welcome to review the rest of the changes from GHI #518)


TL;DR - misc.

I ended up fixing the NSMenu side of any GHI #288 while I was there resolving GHI #518. I resisted integrating any of my own shims into the code base as they are under apache-2.0 (over in my XCMBuild project) or bundled with my own closed-source apps. So I wrote a lot of the cross-compatibility from scratch and don't mind contributing these as MIT-0

reactive-firewall avatar Sep 16 '25 04:09 reactive-firewall

Awesome! I will have a look later today or tomorrow - just on a train at the moment heading home.

mszoek avatar Sep 17 '25 15:09 mszoek

@mszoek may I ask why PR #520 was closed without any feedback from you regarding NSMenu or the work on this GHI (e.g., transparent menu init helpers in NSMenu for App menues)?

:confused:

reactive-firewall avatar Oct 13 '25 19:10 reactive-firewall

I didn't close it... maybe an automated action or something. Let's see...

mszoek avatar Oct 13 '25 21:10 mszoek

Ahhhh, it was automatically closed when I force-pushed main with the new history because it has no common commits anymore. Sorry. You will need to rebase to the new staging branch and submit a new PR.

mszoek avatar Oct 13 '25 21:10 mszoek

I didn't close it... maybe an automated action or something. Let's see...

:speak_no_evil: Sorry, perhaps poor phrasing on my part; I did not mean to come across accusatory, (git blame will do that enough anyhow), I was just confused by the sudden closed PR, as I have been waiting on your feedback before proceeding. Again I apologize if my question came off in any-way rudely, I meant no offence.

Ahhhh, it was automatically closed when I force-pushed main with the new history because it has no common commits anymore.

Odd, that would indicate the commit that brought in the NSMenu.{h,m} files (e.g., from cocoatron, etc.) are missing after the force push (with a history re-write).

Sorry. You will need to rebase to the new staging branch and submit a new PR.

:thinking: hrmm, due to the size of the git index of this repo (2.4 GB at last check) I'm actually going to cherry-pick my changes into a standalone copy based on the version I've already right's cleared. I figure it's easier to let you continue with the momentum of your workflow and just provide a drop-in shim for the NSMenu based on the public API of Apple's public open-source (MIT-like) headers.

Regarding my outstanding question in the meantime:

Is the following implementation close to what you are wanting from the NSMenu helpers (to then be-called by NSApplication during startup):

NSMenu_ravynOS_stub.m:

@implementation NSMenu (MenuHelpers)

/*
 Circa 2025
 Modification provided by Mr. Walls under MIT-NFA
*/

#if !__has_feature(nullability)
+(NSMenu *)newMenuAsApplicationMenu:(NSString*)appName
#else
+(NSMenu * _Nonnull)newMenuAsApplicationMenu:(NSString*)appName
#endif
{
	//TODO: add localization logic
	NSMenu *aMenu = [NSMenu new];
	// legacy code:
	//[aMenu setValue:@"NSAppleMenu" forKey:@"name"]; // assuming KvP
	//FIXME: generate valid DBus
#if defined(DBusInstanceID)
	//FIXME: generate valid DBusItemID for this menu and assign it here
#endif
	[aMenu setTitle:appName];

	NSMenuItem *aboutItem = [[NSMenuItem alloc] initWithTitle:[NSString stringWithFormat:NSMENU_RAVYN_OS_PATTERN_APPMENU_ABOUT_TITLE, appName]
												action:@selector(orderFrontStandardAboutPanel:)
												keyEquivalent:@""];
	[aboutItem setTarget:[NSApplication sharedApplication]]; // FIXME: should be set to first-responder
	[aMenu addItem:aboutItem];

	[aMenu addItem:[NSMenuItem separatorItem]];

	// this stuff must be customized by the App Developer
	NSMenuItem *prefItem = [[NSMenuItem alloc] initWithTitle:NSMENU_RAVYN_OS_PATTERN_APPMENU_PREFERENCES_TITLE
												action:NULL
												keyEquivalent:@","];
	[prefItem setKeyEquivalentModifierMask:NSEventModifierFlagCommand]; // now it is command+comma
	[prefItem setTarget:nil]; // FIXME: should be set to custom target
	// consider default of [prefItem setEnabled:NO];
	[aMenu addItem:prefItem];

	[aMenu addItem:[NSMenuItem separatorItem]];

#if defined(NSMENU_RAVYN_OS_SUPPORTS_APP_SERVICES) && NSMENU_RAVYN_OS_SUPPORTS_APP_SERVICES
	//FIXME: handle services menu stuff
	NSMenuItem *servicesItem = [[NSMenuItem alloc] initWithTitle:@"Services"
													action:NULL
													keyEquivalent:@""];
	[servicesItem setTarget:nil]; // FIXME: should be set to system value
	[servicesItem setSubmenu:[NSMenu new]]; // FIXME: this is an empty placeholder for system services menu
	[aMenu addItem:servicesItem];

	[aMenu addItem:[NSMenuItem separatorItem]];
#endif

	NSMenuItem *hideSelfItem = [[NSMenuItem alloc] initWithTitle:[NSString stringWithFormat:NSMENU_RAVYN_OS_PATTERN_APPMENU_HIDE_SELF_TITLE, appName]
													action:@selector(hide:)
													keyEquivalent:@"h"];
	[hideSelfItem setKeyEquivalentModifierMask:NSEventModifierFlagCommand]; // now it is command+h
	[hideSelfItem setTarget:[NSApplication sharedApplication]]; // FIXME: should be set to first-responder
	[aMenu addItem:hideSelfItem];

	NSMenuItem *hideOthersItem = [[NSMenuItem alloc] initWithTitle:[NSString stringWithFormat:NSMENU_RAVYN_OS_PATTERN_APPMENU_HIDE_OTHERS_TITLE, appName]
													action:@selector(hideOtherApplications:)
													keyEquivalent:@"h"];
	[hideOthersItem setKeyEquivalentModifierMask:(NSEventModifierFlagCommand|NSEventModifierFlagOption)]; // now it is command+option/alt+h
	[hideOthersItem setTarget:[NSApplication sharedApplication]]; // FIXME: should be set to first-responder
	[aMenu addItem:hideOthersItem];

	NSMenuItem *showAllItem = [[NSMenuItem alloc] initWithTitle:[NSString stringWithFormat:NSMENU_RAVYN_OS_PATTERN_APPMENU_SHOW_ALL_TITLE, appName]
													action:@selector(unhideAllApplications:)
													keyEquivalent:@""];
	[showAllItem setTarget:[NSApplication sharedApplication]]; // FIXME: should be set to first-responder
	[aMenu addItem:showAllItem];

	[aMenu addItem:[NSMenuItem separatorItem]];

	// see GHI https://github.com/ravynsoft/ravynos/issues/288
	NSMenuItem *quitItem = [[NSMenuItem alloc] initWithTitle:[NSString stringWithFormat:NSMENU_RAVYN_OS_PATTERN_APPMENU_QUIT_TITLE, appName] action:@selector(terminate:) keyEquivalent:@"q"];
	[quitItem setKeyEquivalentModifierMask:NSEventModifierFlagCommand]; // now it is command+q
	[quitItem setTarget:[NSApplication sharedApplication]];
	[aMenu addItem:quitItem];

	return aMenu;
}

#endif /* !__RAVYNOS__ */

@end

(TL;DR; - yes I've implemented the rest of the NSMenu class so this code would work as written. See closed PR for more on that.)

If this part looks fine to you, I can start on the rest of the solution to this GHI #509. Please advise how you'd prefer to proceed.

:bow: Hope this helps.

reactive-firewall avatar Oct 13 '25 23:10 reactive-firewall