Skip to content

Commit 08a85b2

Browse files
committed
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.
1 parent 2fbd8bf commit 08a85b2

10 files changed

+82
-17
lines changed

src/MacVim/MMCoreTextView.h

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
NSTextInput
1818
#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_14
1919
, NSFontChanging
20+
, NSMenuItemValidation
2021
#endif
2122
>
2223
{
@@ -62,6 +63,22 @@
6263
//
6364
- (void)changeFont:(id)sender;
6465

66+
//
67+
// NSMenuItemValidation
68+
//
69+
- (BOOL)validateMenuItem:(NSMenuItem *)item;
70+
71+
//
72+
// Public macaction's
73+
// Note: New items here need to be handled in validateMenuItem: as well.
74+
//
75+
- (IBAction)cut:(id)sender;
76+
- (IBAction)copy:(id)sender;
77+
- (IBAction)paste:(id)sender;
78+
- (IBAction)undo:(id)sender;
79+
- (IBAction)redo:(id)sender;
80+
- (IBAction)selectAll:(id)sender;
81+
6582
//
6683
// MMTextStorage methods
6784
//

src/MacVim/MMCoreTextView.m

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1140,6 +1140,23 @@ - (void)changeFont:(id)sender
11401140
}
11411141
}
11421142

1143+
/// Specifies whether the menu item should be enabled/disabled.
1144+
- (BOOL)validateMenuItem:(NSMenuItem *)item
1145+
{
1146+
if ([item action] == @selector(cut:)
1147+
|| [item action] == @selector(copy:)
1148+
|| [item action] == @selector(paste:)
1149+
|| [item action] == @selector(undo:)
1150+
|| [item action] == @selector(redo:)
1151+
|| [item action] == @selector(selectAll:))
1152+
return [item tag];
1153+
1154+
// This class should not have any special macOS menu itmes, so theoretically
1155+
// all of them should just return item.tag, but just in case macOS decides
1156+
// to inject some menu items to the parent NSView class without us knowing,
1157+
// we let the superclass handle this.
1158+
return YES;
1159+
}
11431160

11441161
//
11451162
// NOTE: The menu items cut/copy/paste/undo/redo/select all/... must be bound

src/MacVim/MMFullScreenWindow.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,4 +47,9 @@
4747
- (BOOL)canBecomeMainWindow;
4848

4949
- (void)applicationDidChangeScreenParameters:(NSNotification *)notification;
50+
51+
// Public macaction's.
52+
// Note: New items here need to be handled in validateMenuItem: as well.
53+
- (void)performClose:(id)sender;
54+
5055
@end

src/MacVim/MMFullScreenWindow.m

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -444,13 +444,16 @@ - (void)performClose:(id)sender
444444
[super performClose:sender];
445445
}
446446

447+
/// Validates whether the menu item should be enabled or not.
447448
- (BOOL)validateMenuItem:(NSMenuItem *)item
448449
{
449-
if ([item action] == @selector(vimMenuItemAction:)
450-
|| [item action] == @selector(performClose:))
450+
// This class only really have one action that's bound from Vim
451+
if ([item action] == @selector(performClose:))
451452
return [item tag];
452453

453-
return YES;
454+
// Since this is a subclass of NSWindow, it has a bunch of auto-populated
455+
// menu from the OS. Just pass it off to the superclass to let it handle it.
456+
return [super validateMenuItem:item];
454457
}
455458

456459
@end // MMFullScreenWindow

src/MacVim/MMTextView.m

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -906,7 +906,7 @@ - (BOOL)validateMenuItem:(NSMenuItem *)item
906906
|| [item action] == @selector(selectAll:))
907907
return [item tag];
908908

909-
return YES;
909+
return [super validateMenuItem:item];
910910
}
911911

912912
@end // MMTextView

src/MacVim/MMVimController.m

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1447,10 +1447,15 @@ - (void)enableMenuItemWithDescriptor:(NSArray *)desc state:(BOOL)on
14471447
return;
14481448
}
14491449

1450-
// Use tag to set whether item is enabled or disabled instead of
1451-
// calling setEnabled:. This way the menus can autoenable themselves
1452-
// but at the same time Vim can set if a menu is enabled whenever it
1453-
// wants to.
1450+
// We are using auto-enabling of menu items, where instead of directly
1451+
// calling setEnabled:, we rely on validateMenuItem: callbacks in each
1452+
// target to handle whether they want each menu item to be enabled or not.
1453+
// This allows us to more easily control the enabled states of OS-injected
1454+
// menu items if we want to. To remember whether we want to enable/disable
1455+
// a Vim menu, we use item.tag to remember it. See each validateMenuItem:
1456+
// implementation for details.
1457+
//
1458+
// See https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/MenuList/Articles/EnablingMenuItems.html
14541459
[[self menuItemForDescriptor:desc] setTag:on];
14551460

14561461
const BOOL isPopup = [MMVimController hasPopupPrefix:rootName];

src/MacVim/MMWindow.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,4 +33,8 @@
3333
- (IBAction)toggleFullScreen:(id)sender;
3434
- (IBAction)realToggleFullScreen:(id)sender;
3535

36+
// Public macaction's.
37+
// Note: New items here need to be handled in validateMenuItem: as well.
38+
- (void)performClose:(id)sender;
39+
3640
@end

src/MacVim/MMWindow.m

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -184,13 +184,17 @@ - (void)performClose:(id)sender
184184
[super performClose:sender];
185185
}
186186

187+
/// Validates whether the menu item should be enabled or not.
187188
- (BOOL)validateMenuItem:(NSMenuItem *)item
188189
{
189-
if ([item action] == @selector(vimMenuItemAction:)
190-
|| [item action] == @selector(performClose:))
190+
// This class only really have one action that's bound from Vim
191+
if ([item action] == @selector(performClose:)) {
191192
return [item tag];
193+
}
192194

193-
return YES;
195+
// Since this is a subclass of NSWindow, it has a bunch of auto-populated
196+
// menu from the OS. Just pass it off to the superclass to let it handle it.
197+
return [super validateMenuItem:item];
194198
}
195199

196200
- (IBAction)zoom:(id)sender

src/MacVim/MMWindowController.h

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,12 @@
1717
@class MMVimController;
1818
@class MMVimView;
1919

20-
@interface MMWindowController : NSWindowController<NSWindowDelegate>
20+
@interface MMWindowController : NSWindowController<
21+
NSWindowDelegate
22+
#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_14
23+
, NSMenuItemValidation
24+
#endif
25+
>
2126
{
2227
MMVimController *vimController;
2328
MMVimView *vimView;
@@ -104,6 +109,12 @@
104109
- (BOOL)getDefaultTopLeft:(NSPoint*)pt;
105110
- (void)runAfterWindowPresentedUsingBlock:(void (^)(void))block;
106111

112+
//
113+
// NSMenuItemValidation
114+
//
115+
- (BOOL)validateMenuItem:(NSMenuItem *)item;
116+
117+
// Menu items / macactions
107118
- (IBAction)addNewTab:(id)sender;
108119
- (IBAction)toggleToolbar:(id)sender;
109120
- (IBAction)performClose:(id)sender;

src/MacVim/MMWindowController.m

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1181,11 +1181,10 @@ - (IBAction)findAndReplace:(id)sender
11811181

11821182
- (BOOL)validateMenuItem:(NSMenuItem *)item
11831183
{
1184-
if ([item action] == @selector(vimMenuItemAction:)
1185-
|| [item action] == @selector(performClose:))
1186-
return [item tag];
1187-
1188-
return YES;
1184+
// This class is a responsder class and this should get called when we have
1185+
// a specific action that we implement exposed as a menu. As such just return
1186+
// [item tag] and no need to worry about macOS-injected menus.
1187+
return [item tag];
11891188
}
11901189

11911190
// -- NSWindow delegate ------------------------------------------------------

0 commit comments

Comments
 (0)