From 08a85b2b2b02cf7ddeb0e47b324c8358e65d92cf Mon Sep 17 00:00:00 2001 From: Yee Cheng Chin Date: Thu, 6 Oct 2022 18:27:11 -0700 Subject: [PATCH] Fix menu items like Edit.Cut/Copy not disabled in normal mode We are using auto-enabling of menu items, which require each target of macaction to implement validateMenuItem properly. It was done in MMTextView, but not MMCoreTextView. Also, as part of fixing this up, just add more comments, and make sure to call back to superclass etc to make the code more robust. --- src/MacVim/MMCoreTextView.h | 17 +++++++++++++++++ src/MacVim/MMCoreTextView.m | 17 +++++++++++++++++ src/MacVim/MMFullScreenWindow.h | 5 +++++ src/MacVim/MMFullScreenWindow.m | 9 ++++++--- src/MacVim/MMTextView.m | 2 +- src/MacVim/MMVimController.m | 13 +++++++++---- src/MacVim/MMWindow.h | 4 ++++ src/MacVim/MMWindow.m | 10 +++++++--- src/MacVim/MMWindowController.h | 13 ++++++++++++- src/MacVim/MMWindowController.m | 9 ++++----- 10 files changed, 82 insertions(+), 17 deletions(-) diff --git a/src/MacVim/MMCoreTextView.h b/src/MacVim/MMCoreTextView.h index 5c7c6e3f68..4959e04ce9 100644 --- a/src/MacVim/MMCoreTextView.h +++ b/src/MacVim/MMCoreTextView.h @@ -17,6 +17,7 @@ NSTextInput #if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_14 , NSFontChanging + , NSMenuItemValidation #endif > { @@ -62,6 +63,22 @@ // - (void)changeFont:(id)sender; +// +// NSMenuItemValidation +// +- (BOOL)validateMenuItem:(NSMenuItem *)item; + +// +// Public macaction's +// Note: New items here need to be handled in validateMenuItem: as well. +// +- (IBAction)cut:(id)sender; +- (IBAction)copy:(id)sender; +- (IBAction)paste:(id)sender; +- (IBAction)undo:(id)sender; +- (IBAction)redo:(id)sender; +- (IBAction)selectAll:(id)sender; + // // MMTextStorage methods // diff --git a/src/MacVim/MMCoreTextView.m b/src/MacVim/MMCoreTextView.m index a1089fb886..018cfc965b 100644 --- a/src/MacVim/MMCoreTextView.m +++ b/src/MacVim/MMCoreTextView.m @@ -1140,6 +1140,23 @@ - (void)changeFont:(id)sender } } +/// Specifies whether the menu item should be enabled/disabled. +- (BOOL)validateMenuItem:(NSMenuItem *)item +{ + if ([item action] == @selector(cut:) + || [item action] == @selector(copy:) + || [item action] == @selector(paste:) + || [item action] == @selector(undo:) + || [item action] == @selector(redo:) + || [item action] == @selector(selectAll:)) + return [item tag]; + + // This class should not have any special macOS menu itmes, so theoretically + // all of them should just return item.tag, but just in case macOS decides + // to inject some menu items to the parent NSView class without us knowing, + // we let the superclass handle this. + return YES; +} // // NOTE: The menu items cut/copy/paste/undo/redo/select all/... must be bound diff --git a/src/MacVim/MMFullScreenWindow.h b/src/MacVim/MMFullScreenWindow.h index 4dd870fd58..337c9a9e04 100644 --- a/src/MacVim/MMFullScreenWindow.h +++ b/src/MacVim/MMFullScreenWindow.h @@ -47,4 +47,9 @@ - (BOOL)canBecomeMainWindow; - (void)applicationDidChangeScreenParameters:(NSNotification *)notification; + +// Public macaction's. +// Note: New items here need to be handled in validateMenuItem: as well. +- (void)performClose:(id)sender; + @end diff --git a/src/MacVim/MMFullScreenWindow.m b/src/MacVim/MMFullScreenWindow.m index 4c36ffe2d7..274b4cc5d2 100644 --- a/src/MacVim/MMFullScreenWindow.m +++ b/src/MacVim/MMFullScreenWindow.m @@ -444,13 +444,16 @@ - (void)performClose:(id)sender [super performClose:sender]; } +/// Validates whether the menu item should be enabled or not. - (BOOL)validateMenuItem:(NSMenuItem *)item { - if ([item action] == @selector(vimMenuItemAction:) - || [item action] == @selector(performClose:)) + // This class only really have one action that's bound from Vim + if ([item action] == @selector(performClose:)) return [item tag]; - return YES; + // Since this is a subclass of NSWindow, it has a bunch of auto-populated + // menu from the OS. Just pass it off to the superclass to let it handle it. + return [super validateMenuItem:item]; } @end // MMFullScreenWindow diff --git a/src/MacVim/MMTextView.m b/src/MacVim/MMTextView.m index 4546787ed1..26cd20d8ac 100644 --- a/src/MacVim/MMTextView.m +++ b/src/MacVim/MMTextView.m @@ -906,7 +906,7 @@ - (BOOL)validateMenuItem:(NSMenuItem *)item || [item action] == @selector(selectAll:)) return [item tag]; - return YES; + return [super validateMenuItem:item]; } @end // MMTextView diff --git a/src/MacVim/MMVimController.m b/src/MacVim/MMVimController.m index 6faa7899c7..42097cb143 100644 --- a/src/MacVim/MMVimController.m +++ b/src/MacVim/MMVimController.m @@ -1447,10 +1447,15 @@ - (void)enableMenuItemWithDescriptor:(NSArray *)desc state:(BOOL)on return; } - // Use tag to set whether item is enabled or disabled instead of - // calling setEnabled:. This way the menus can autoenable themselves - // but at the same time Vim can set if a menu is enabled whenever it - // wants to. + // We are using auto-enabling of menu items, where instead of directly + // calling setEnabled:, we rely on validateMenuItem: callbacks in each + // target to handle whether they want each menu item to be enabled or not. + // This allows us to more easily control the enabled states of OS-injected + // menu items if we want to. To remember whether we want to enable/disable + // a Vim menu, we use item.tag to remember it. See each validateMenuItem: + // implementation for details. + // + // See https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/MenuList/Articles/EnablingMenuItems.html [[self menuItemForDescriptor:desc] setTag:on]; const BOOL isPopup = [MMVimController hasPopupPrefix:rootName]; diff --git a/src/MacVim/MMWindow.h b/src/MacVim/MMWindow.h index 9eb7be1cba..ce6fa39e57 100644 --- a/src/MacVim/MMWindow.h +++ b/src/MacVim/MMWindow.h @@ -33,4 +33,8 @@ - (IBAction)toggleFullScreen:(id)sender; - (IBAction)realToggleFullScreen:(id)sender; +// Public macaction's. +// Note: New items here need to be handled in validateMenuItem: as well. +- (void)performClose:(id)sender; + @end diff --git a/src/MacVim/MMWindow.m b/src/MacVim/MMWindow.m index 5fa0a02e4d..7c05880d18 100644 --- a/src/MacVim/MMWindow.m +++ b/src/MacVim/MMWindow.m @@ -184,13 +184,17 @@ - (void)performClose:(id)sender [super performClose:sender]; } +/// Validates whether the menu item should be enabled or not. - (BOOL)validateMenuItem:(NSMenuItem *)item { - if ([item action] == @selector(vimMenuItemAction:) - || [item action] == @selector(performClose:)) + // This class only really have one action that's bound from Vim + if ([item action] == @selector(performClose:)) { return [item tag]; + } - return YES; + // Since this is a subclass of NSWindow, it has a bunch of auto-populated + // menu from the OS. Just pass it off to the superclass to let it handle it. + return [super validateMenuItem:item]; } - (IBAction)zoom:(id)sender diff --git a/src/MacVim/MMWindowController.h b/src/MacVim/MMWindowController.h index ce39161a4d..e1e83176e2 100644 --- a/src/MacVim/MMWindowController.h +++ b/src/MacVim/MMWindowController.h @@ -17,7 +17,12 @@ @class MMVimController; @class MMVimView; -@interface MMWindowController : NSWindowController +@interface MMWindowController : NSWindowController< + NSWindowDelegate +#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_14 + , NSMenuItemValidation +#endif + > { MMVimController *vimController; MMVimView *vimView; @@ -104,6 +109,12 @@ - (BOOL)getDefaultTopLeft:(NSPoint*)pt; - (void)runAfterWindowPresentedUsingBlock:(void (^)(void))block; +// +// NSMenuItemValidation +// +- (BOOL)validateMenuItem:(NSMenuItem *)item; + +// Menu items / macactions - (IBAction)addNewTab:(id)sender; - (IBAction)toggleToolbar:(id)sender; - (IBAction)performClose:(id)sender; diff --git a/src/MacVim/MMWindowController.m b/src/MacVim/MMWindowController.m index 7d0f012c1c..8496e8d522 100644 --- a/src/MacVim/MMWindowController.m +++ b/src/MacVim/MMWindowController.m @@ -1181,11 +1181,10 @@ - (IBAction)findAndReplace:(id)sender - (BOOL)validateMenuItem:(NSMenuItem *)item { - if ([item action] == @selector(vimMenuItemAction:) - || [item action] == @selector(performClose:)) - return [item tag]; - - return YES; + // This class is a responsder class and this should get called when we have + // a specific action that we implement exposed as a menu. As such just return + // [item tag] and no need to worry about macOS-injected menus. + return [item tag]; } // -- NSWindow delegate ------------------------------------------------------