Initial refactor for NSMenu for ARC and nullability (and a whole lot of TODO/FIXME comments)

[reactive-firewall]

Patch Notes

This code is still unpolished but now it should actually behave more like an NSMenu should.

... or it would if NSMenuItem was working 🤦 ... but let's take it one class at a time.

Impacted GHI

• Closes Custodial refactor of NSMenu #518

• Contributes to Transparently init app menu when not present #509

• Opens NSMenu needs NSMenuItem to be useful #519

---

Misc. Comments:

• note the plethora of TODOs and FIXMEs left for future iterations
• this should unblock GHI Transparently init app menu when not present #509 (at-least for the helper methods)

[github-actions]

Thank you for taking the time to contribute to FreeBSD!
There are a few issues that need to be fixed:

• Missing Signed-off-by lines^0c0f0b5
• Real email address is needed^0c0f0b5

Please review CONTRIBUTING.md, then update and push your branch again.

[reactive-firewall]

Changes in the Objective-C category MenuHelpers of NSMenu is focused on GHI #509

@mszoek please review around lines 1353-1440 (end)

• If this part looks good to you, I'll churn out the rest of the menu helpers in a cookie-cuter fashion for GHI Transparently init app menu when not present #509.

---

TL;DR - rest of changes are just custodial and do not result in major change yet.

Areas to focus on: how should the DBus IDs for RavynOS be generated, the previous code never initializes them and there is no equivilant of these in apple's public APIs of NSMenu so I'll leave that executive decision up to you, I did implement the accessor/modifiers for the property on systems that define __RAVYNOS__ but that means the helper functions need to set it if it is to be used by other components. I followed the typical Objective-C principle of using the overflow values for the invalid indexes so unset values can be identified (even without Foundation's NSNotFound defined 👀 )

I also found the code to be a mix of manual and ARC and even what looked like c89 code (seriously it wasn't even Objective-C code) so I stabilized that and prepared the code for future work.

[reactive-firewall]

Technical Summary

---

Overview

This PR introduces significant enhancements and modernizations to the NSMenu.h header file in the AppKit framework for ravynos. It adds platform-specific definitions, modern Objective-C attributes, improved nullability handling, more robust property/method declarations, and explicit support for the ravynos environment. The code is annotated with extensive comments and TODOs for future maintainability and compatibility.

---

Key Changes

1. Platform & Compatibility Macros

• New Defines for ravynos:
  Introduces NSMENU_RAVYN_OS_PATTERN_* macros for common menu string patterns. These facilitate the use of localized or standardized menu item titles for application and system menus under ravynos.
• Feature Detection Macros:
  Adds guards and feature checks (__has_feature(nullability), __has_feature(objc_arc), __has_feature(objc_protocol_qualifier_mangling)) for conditional compilation, improving cross-platform and cross-compiler compatibility.

2. Objective-C Attribute Support

• OBJC_DESIGNATED_INITIALIZER Macro:
  Conditional macro for marking designated initializers, providing compiler-checked correctness in modern Objective-C.

3. Class Interface Modernization

• Class Declaration:
  • Now conforms to both <NSCopying> and <NSCoding>, indicating support for copying and serialization.
  • Uses @protected and @private visibility for ivars, clarifying encapsulation.
• DBusInstanceID Typedef:
  Adds a typedef for DBusInstanceID, with conditional definition.
• Nullability Annotations:
  Properties and methods are now conditionally annotated for nullability, improving safety when used in Swift or modern Objective-C.

4. Properties and Methods

• Properties:
  • Declares properties for supermenu, title, itemArray, and delegate with appropriate memory management attributes (weak, retain, copy, assign), all conditionally chosen based on ARC and nullability support.
• Method Signatures:
  • All major API methods are updated to include nullability, and in some cases, new overloads are added for compatibility.
  • New methods for setting and getting itemArray.
  • Adds conditionally-compiled methods and properties for ravynos-specific features (e.g., _name, _menuWithName:).
• Designated Initializer:
  • Adds documentation and the OBJC_DESIGNATED_INITIALIZER attribute to -initWithTitle:.
• Menu Bar Height:
  • Adds method to get menu bar height if the proper macro is defined.

5. ravynos-Specific Enhancements

• Menu Patterns:
  • Macros and helpers for common menu item naming conventions.
• App Menu Construction Helper:
  • Adds a category method +newMenuAsApplicationMenu: to help construct an application menu based on the app name (ravynos only).
• Warnings:
  • Explicit warnings for unsupported features (like NSFont and NSScreen) in this implementation.

6. Improved Nullability and Type Safety

• Nullability Guards:
  • Extensive use of __nullable, __nonnull, __null_unspecified for pointers, making the API more robust for modern development.
• Type Annotations for NSArray:
  • Uses __kindof for arrays where appropriate.

7. Comments, Fixmes, and Documentation

• Rich Commenting:
  • Extensive FIXMEs, TODOs, and documentation for maintainers and future developers.
  • Licensing and authorship notes updated.

---

Technical Impact

• Modernizes API and increases safety for Objective-C and Swift clients.
• Improves cross-platform compatibility and prepares the code for better static analysis and compiler checking.
• Provides ravynos-specific enhancements for system and application menu conventions.
• Documents areas for future improvement and highlights current platform limitations.

---

Summary:
This PR is a major reworking of the NSMenu.h header, bringing it in line with modern Objective-C practices, increasing robustness, and adding ravynos-specific APIs and macros. It lays a strong foundation for future cross-platform and cross-language usability while documenting deliberate platform differences and pending work.

[reactive-firewall]

Technical Summary

---

The changes to NSMenu.m in this PR represent a substantial refactor and modernization of the Objective-C implementation for RavynOS. Key technical updates include:

• Major code expansion: The file grew by over 1,000 lines, adding detailed handling for menu structure, item management, and compatibility macros.
• Enhanced platform-specific support: Numerous sections are now guarded by RAVYNOS and related macros, enabling RavynOS-specific behaviors and patterns (e.g., menu naming conventions, pattern strings for standard menu items).
• Improved nullability annotations and Objective-C feature detection: Widespread use of __has_feature, OBJC_DESIGNATED_INITIALIZER, and other compile-time checks improves compatibility with modern Objective-C compilers and clarifies intent.
• Expanded menu API: The NSMenu class now fully supports copying, encoding/decoding (NSCoding), thread-safety preparations (locking placeholders), and more robust item and submenu lookup.
• Application menu helpers: A major addition is newMenuAsApplicationMenu:, which programmatically builds a standard application menu with About, Preferences, Services, Hide, Show, and Quit items, following platform conventions.
• More robust memory management: Retain/release logic is explicit, and items are removed or updated safely, with placeholder logic for thread safety.
• Extended validation and event handling: Key equivalent handling, click handling, and item enable/disable logic are more robust and modular.
• Extensive documentation and TODOs: The code is heavily commented with implementation notes, platform compatibility warnings, and developer guidance.

In summary, this PR transforms NSMenu.m from a minimal implementation into a comprehensive, modern, platform-adapted menu system suitable for RavynOS, with extensibility and future improvements in mind.

[reactive-firewall]

@mszoek,
I wanted to circle back this week on this; I would still love to get your opinion on these changes, when you get a chance.

[mszoek]

@mszoek, I wanted to circle back this week on this; I would still love to get your opinion on these changes, when you get a chance.

Hey! Sorry I haven't been able to review/merge this yet. I'm still trying to bisect the code to resolve some breakage from upstream, so can't really merge anything new until I get the branch back in shape. Soon I hope!

[reactive-firewall]

Hey! Sorry I haven't been able to review/merge this yet. I'm still trying to bisect the code to resolve some breakage from upstream, so can't really merge anything new until I get the branch back in shape. Soon I hope!

No worries; The Breakage is why I kept the changes to one file at a time. If you'd rather a fully working AppKit, that will take me quite a bit longer, but is part of my own interest here (I'm interested namely in a stable Foundation.framework, and UIKit.framework AND/OR AppKit.framework)

Warning

If a broken main is an issue (I assumed just a sideffect of W.I.P.) Please don't merge this to main, keep it on a feature branch. See #519 for why it will break.

Side Note TL;DR; Now that I've taken a look at the AppKit as a whole (it seems clearly a derivative of Cocotron from a while back) my plan is to piecewise refactor (e.g. one class at a time when I have the time) and modernize the codebase, focusing on core interfaces (leaving extensions for now). NSMenuItem is next on my list.

Important

What I namely need to get your opinion on is the RavynOS specific design:

ravynos/Frameworks/AppKit/NSMenu.subproj/NSMenu.m

Lines 1353 to 1440 in 0c0f0b5

	@implementation NSMenu (MenuHelpers)
	#endif
	/*
	Circa 2025
	Modification provided by Mr. Walls under MIT
	*/
	#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
	[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

e.g., does this look like what you feel we discussed in GHI #509

---

If you still want to get the upstream stuff working, first, I'll be patient, so no rush from me. 🙇 I hope this helps.