Merge pull request #1308 from ychin/menu-enabled-state-fix

Fix menu items like Edit.Cut/Copy not disabled in normal mode

Author: ychin

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
 //

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

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

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

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

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];

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

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

src/MacVim/MMWindowController.h

@@ -17,7 +17,12 @@
 @class MMVimController;
 @class MMVimView;
 
-@interface MMWindowController : NSWindowController<NSWindowDelegate>
+@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;

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 ------------------------------------------------------