Fix tmenu in MacVim and submenu handling in TouchBar

ychin · ychin · commit 15eadd75e16b · 2020-09-09T01:14:48.000-07:00
Fix `tmenu` to work when it's called after the menu has already been created, by implementing `gui_mch_menu_set_tip`. This works for menus, tool bar, and Touch Bar (see #1084 which added tmenu support for TouchBar). Also, fix Touch Bar's submenu handling to reuse more common shared function. Previously enabling/disabling/removing Touch Bar submenu items didn't work as it had a hard-coded length check.
diff --git a/src/MacVim/MMVimController.m b/src/MacVim/MMVimController.m
@@ -132,6 +132,7 @@ - (void)addMenuItemWithDescriptor:(NSArray *)desc
                       isAlternate:(BOOL)isAlternate;
 - (void)removeMenuItemWithDescriptor:(NSArray *)desc;
 - (void)enableMenuItemWithDescriptor:(NSArray *)desc state:(BOOL)on;
+- (void)updateMenuItemTooltipWithDescriptor:(NSArray *)desc tip:(NSString *)tip;
 - (void)addToolbarItemToDictionaryWithLabel:(NSString *)title
         toolTip:(NSString *)tip icon:(NSString *)icon;
 - (void)addToolbarItemWithLabel:(NSString *)label
@@ -144,6 +145,12 @@ - (void)addTouchbarItemWithLabel:(NSString *)label
                        isSubMenu:(BOOL)submenu
                             desc:(NSArray *)desc
                       atTouchBar:(MMTouchBarInfo *)touchbarInfo;
+- (void)updateTouchbarItemLabel:(NSString *)label
+                            tip:(NSString *)tip
+                 atTouchBarItem:(MMTouchBarItemInfo*)item;
+- (BOOL)touchBarItemForDescriptor:(NSArray *)desc
+                         touchBar:(MMTouchBarInfo **)touchBarPtr
+                     touchBarItem:(MMTouchBarItemInfo **)touchBarItemPtr;
 - (void)popupMenuWithDescriptor:(NSArray *)desc
                           atRow:(NSNumber *)row
                          column:(NSNumber *)col;
@@ -752,6 +759,10 @@ - (void)handleMessage:(int)msgid data:(NSData *)data
         NSDictionary *attrs = [NSDictionary dictionaryWithData:data];
         [self enableMenuItemWithDescriptor:[attrs objectForKey:@"descriptor"]
                 state:[[attrs objectForKey:@"enable"] boolValue]];
+    } else if (UpdateMenuItemTooltipMsgID == msgid) {
+        NSDictionary *attrs = [NSDictionary dictionaryWithData:data];
+        [self updateMenuItemTooltipWithDescriptor:[attrs objectForKey:@"descriptor"]
+                                              tip:[attrs objectForKey:@"tip"]];
     } else if (ShowToolbarMsgID == msgid) {
         const void *bytes = [data bytes];
         int enable = *((int*)bytes);  bytes += sizeof(int);
@@ -1025,10 +1036,11 @@ - (void)savePanelDidEnd:(NSSavePanel *)panel code:(int)code
 {
     NSString *path = nil;
 #if (MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_10)
-    if (code == NSModalResponseOK) {
+    if (code == NSModalResponseOK)
 #else
-    if (code == NSOKButton) {
+    if (code == NSOKButton)
 #endif
+    {
         NSURL *url = [panel URL];
         if ([url isFileURL])
             path = [url path];
@@ -1214,16 +1226,9 @@ - (void)addMenuWithDescriptor:(NSArray *)desc atIndex:(int)idx
             if ([desc count] >= 3) // Unfortunately currently Apple does not support nested popover's so we can only do one level nesting
                 return;
 
-            MMTouchBarInfo *submenuTouchbar = touchbarInfo;
-            for (int i = 1; i < [desc count] - 1; i++) {
-                NSString *submenuName = [desc objectAtIndex:i];
-                MMTouchBarItemInfo *submenu = [[submenuTouchbar itemDict] objectForKey:submenuName];
-                if ([submenu childTouchbar]) {
-                    submenuTouchbar = [submenu childTouchbar];
-                } else {
-                    ASLogWarn(@"No Touch Bar submenu with id '%@'", submenuName);
-                    return;
-                }
+            MMTouchBarInfo *submenuTouchbar = nil;
+            if (![self touchBarItemForDescriptor:desc touchBar:&submenuTouchbar touchBarItem:nil]) {
+                return;
             }
             // Icon is not supported for Touch Bar submenu for now, as "amenu" does not have a way of specifying "icon=<icon_path>" for submenus.
             NSString *title = [desc lastObject];
@@ -1289,17 +1294,11 @@ - (void)addMenuItemWithDescriptor:(NSArray *)desc
             return;
 
         if (NSClassFromString(@"NSTouchBar")) {
-            MMTouchBarInfo *submenuTouchbar = touchbarInfo;
-            for (int i = 1; i < [desc count] - 1; i++) {
-                NSString *submenuName = [desc objectAtIndex:i];
-                MMTouchBarItemInfo *submenu = [[submenuTouchbar itemDict] objectForKey:submenuName];
-                if ([submenu childTouchbar]) {
-                    submenuTouchbar = [submenu childTouchbar];
-                } else {
-                    ASLogWarn(@"No Touch Bar submenu with id '%@'", submenuName);
-                    return;
-                }
+            MMTouchBarInfo *submenuTouchbar = nil;
+            if (![self touchBarItemForDescriptor:desc touchBar:&submenuTouchbar touchBarItem:nil]) {
+                return;
             }
+
             [self addTouchbarItemWithLabel:title icon:icon tip:tip atIndex:idx isSubMenu:NO desc:desc atTouchBar:submenuTouchbar];
         }
 #endif
@@ -1370,11 +1369,14 @@ - (void)removeMenuItemWithDescriptor:(NSArray *)desc
     if ([rootName isEqual:MMTouchbarMenuName]){
 #if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_12_2
         if (NSClassFromString(@"NSTouchBar")) {
-            if ([desc count] == 2) {
-                [[touchbarInfo itemOrder] removeObject:title];
-                [[touchbarInfo itemDict] removeObjectForKey:title];
-                [windowController setTouchBar:nil];
+            MMTouchBarInfo *submenuTouchbar = nil;
+            if (![self touchBarItemForDescriptor:desc touchBar:&submenuTouchbar touchBarItem:nil]) {
+                return;
             }
+
+            [[submenuTouchbar itemOrder] removeObject:title];
+            [[submenuTouchbar itemDict] removeObjectForKey:title];
+            [windowController setTouchBar:nil];
         }
 #endif
         return;
@@ -1417,11 +1419,12 @@ - (void)enableMenuItemWithDescriptor:(NSArray *)desc state:(BOOL)on
     if ([rootName isEqual:MMTouchbarMenuName]) {
 #if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_12_2
         if (NSClassFromString(@"NSTouchBar")) {
-            if ([desc count] == 2) {
-                NSString *title = [desc lastObject];
-                [[touchbarInfo.itemDict objectForKey:title] setEnabled:on];
-                [windowController setTouchBar:nil];
+            MMTouchBarItemInfo *touchbarItem = nil;
+            if (![self touchBarItemForDescriptor:desc touchBar:nil touchBarItem:&touchbarItem]) {
+                return;
             }
+            [touchbarItem setEnabled:on];
+            [windowController setTouchBar:nil];
         }
 #endif
         return;
@@ -1433,6 +1436,38 @@ - (void)enableMenuItemWithDescriptor:(NSArray *)desc state:(BOOL)on
     // wants to.
     [[self menuItemForDescriptor:desc] setTag:on];
 }
+    
+- (void)updateMenuItemTooltipWithDescriptor:(NSArray *)desc
+                                        tip:(NSString *)tip
+{
+    if (!(desc && [desc count] > 0)) return;
+    
+    NSString *rootName = [desc objectAtIndex:0];
+    if ([rootName isEqual:MMToolbarMenuName]) {
+        if (toolbar && [desc count] == 2) {
+            NSString *title = [desc lastObject];
+            [[toolbar itemWithItemIdentifier:title] setToolTip:tip];
+        }
+        return;
+    }
+
+    if ([rootName isEqual:MMTouchbarMenuName]) {
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_12_2
+        if (NSClassFromString(@"NSTouchBar")) {
+            MMTouchBarItemInfo *touchbarItem = nil;
+            if (![self touchBarItemForDescriptor:desc touchBar:nil touchBarItem:&touchbarItem]) {
+                return;
+            }
+            NSString *title = [desc lastObject];
+            [self updateTouchbarItemLabel:title tip:tip atTouchBarItem:touchbarItem];
+            [windowController setTouchBar:nil];
+        }
+#endif
+        return;
+    }
+
+    [[self menuItemForDescriptor:desc] setToolTip:tip];
+}
 
 - (void)addToolbarItemToDictionaryWithLabel:(NSString *)title
                                     toolTip:(NSString *)tip
@@ -1575,6 +1610,50 @@ - (void)addTouchbarItemWithLabel:(NSString *)label
 
     [windowController setTouchBar:nil];
 }
+
+- (void)updateTouchbarItemLabel:(NSString *)label
+                            tip:(NSString *)tip
+                 atTouchBarItem:(MMTouchBarItemInfo*)item
+{
+    // The logic here should match that in addTouchbarItemWithLabel: as otherwise we will
+    // get weird results when adding/removing tooltips.
+    BOOL useTip = tip && [tip length] != 0;
+    NSString *buttonTitle = useTip ? tip : label;
+    NSCustomTouchBarItem *touchbarItem = (NSCustomTouchBarItem*)item.touchbarItem;
+    MMTouchBarButton *button = (MMTouchBarButton*)touchbarItem.view;
+    [button setTitle:buttonTitle];
+    if (button.image) {
+        if (useTip) {
+            [button setImagePosition:NSImageLeft];
+        } else {
+            [button setImagePosition:NSImageOnly];
+        }
+    } else {
+        [button setImagePosition:NSNoImage];
+    }
+}
+
+- (BOOL)touchBarItemForDescriptor:(NSArray *)desc
+                         touchBar:(MMTouchBarInfo **)touchBarPtr
+                     touchBarItem:(MMTouchBarItemInfo **)touchBarItemPtr
+{
+    MMTouchBarInfo *submenuTouchbar = touchbarInfo;
+    for (int i = 1; i < [desc count] - 1; i++) {
+        NSString *submenuName = [desc objectAtIndex:i];
+        MMTouchBarItemInfo *submenu = [[submenuTouchbar itemDict] objectForKey:submenuName];
+        if ([submenu childTouchbar]) {
+            submenuTouchbar = [submenu childTouchbar];
+        } else {
+            ASLogWarn(@"No Touch Bar submenu with id '%@'", submenuName);
+            return NO;
+        }
+    }
+    if (touchBarPtr)
+        *touchBarPtr = submenuTouchbar;
+    if (touchBarItemPtr)
+        *touchBarItemPtr = [[submenuTouchbar itemDict] objectForKey:[desc lastObject]];
+    return YES;
+}
 #endif
 - (void)popupMenuWithDescriptor:(NSArray *)desc
                           atRow:(NSNumber *)row
diff --git a/src/MacVim/MacVim.h b/src/MacVim/MacVim.h
@@ -203,6 +203,7 @@ extern const char * const MMVimMsgIDStrings[];
     MSG(RemoveMenuItemMsgID) \
     MSG(EnableMenuItemMsgID) \
     MSG(ExecuteMenuMsgID) \
+    MSG(UpdateMenuItemTooltipMsgID) \
     MSG(ShowToolbarMsgID) \
     MSG(ToggleToolbarMsgID) \
     MSG(CreateScrollbarMsgID) \
diff --git a/src/MacVim/gui_macvim.m b/src/MacVim/gui_macvim.m
@@ -894,6 +894,22 @@
 }
 
 
+/*
+ * Update a menu's tooltip.
+ */
+    void
+gui_mch_menu_set_tip(vimmenu_T *menu)
+{
+    char_u *tip = menu->strings[MENU_INDEX_TIP];
+    NSArray *desc = descriptor_for_menu(menu);
+    [[MMBackend sharedInstance] queueMessage:UpdateMenuItemTooltipMsgID properties:
+        [NSDictionary dictionaryWithObjectsAndKeys:
+            desc, @"descriptor",
+            [NSString stringWithVimString:tip], @"tip",
+            nil]];
+}
+
+
 /*
  * This is called when a :popup command is executed.
  */
diff --git a/src/menu.c b/src/menu.c
@@ -814,8 +814,7 @@ add_menu_path(
 	    }
 	}
 #if defined(FEAT_TOOLBAR) && !defined(FEAT_GUI_MSWIN) \
-	&& !defined(FEAT_GUI_MACVIM) \
-	&& (defined(FEAT_BEVAL_GUI) || defined(FEAT_GUI_GTK))
+	&& (defined(FEAT_BEVAL_GUI) || defined(FEAT_GUI_GTK) || defined(FEAT_GUI_MACVIM))
 	// Need to update the menu tip.
 	if (modes & MENU_TIP_MODE)
 	    gui_mch_menu_set_tip(menu);
@@ -1010,8 +1009,7 @@ remove_menu(
 	{
 	    free_menu_string(menu, MENU_INDEX_TIP);
 #if defined(FEAT_TOOLBAR) && !defined(FEAT_GUI_MSWIN) \
-	    && !defined(FEAT_GUI_MACVIM) \
-	    && (defined(FEAT_BEVAL_GUI) || defined(FEAT_GUI_GTK))
+	    && (defined(FEAT_BEVAL_GUI) || defined(FEAT_GUI_GTK) || defined(FEAT_GUI_MACVIM))
 	    // Need to update the menu tip.
 	    if (gui.in_use)
 		gui_mch_menu_set_tip(menu);

Original file line number	Diff line number	Diff line change
`@@ -814,8 +814,7 @@ add_menu_path(`
`814`	`814`	`}`
`815`	`815`	`}`
`816`	`816`	`#if defined(FEAT_TOOLBAR) && !defined(FEAT_GUI_MSWIN) \`
`817`		`- && !defined(FEAT_GUI_MACVIM) \`
`818`		`- && (defined(FEAT_BEVAL_GUI) \|\| defined(FEAT_GUI_GTK))`
	`817`	`+ && (defined(FEAT_BEVAL_GUI) \|\| defined(FEAT_GUI_GTK) \|\| defined(FEAT_GUI_MACVIM))`
`819`	`818`	`// Need to update the menu tip.`
`820`	`819`	`if (modes & MENU_TIP_MODE)`
`821`	`820`	`gui_mch_menu_set_tip(menu);`
`@@ -1010,8 +1009,7 @@ remove_menu(`
`1010`	`1009`	`{`
`1011`	`1010`	`free_menu_string(menu, MENU_INDEX_TIP);`
`1012`	`1011`	`#if defined(FEAT_TOOLBAR) && !defined(FEAT_GUI_MSWIN) \`
`1013`		`- && !defined(FEAT_GUI_MACVIM) \`
`1014`		`- && (defined(FEAT_BEVAL_GUI) \|\| defined(FEAT_GUI_GTK))`
	`1012`	`+ && (defined(FEAT_BEVAL_GUI) \|\| defined(FEAT_GUI_GTK) \|\| defined(FEAT_GUI_MACVIM))`
`1015`	`1013`	`// Need to update the menu tip.`
`1016`	`1014`	`if (gui.in_use)`
`1017`	`1015`	`gui_mch_menu_set_tip(menu);`