Make menu keybindings take precedence over builtin ones, except for confirm/esc

This makes it possible to use 'j', 'k', 'H' or 'L' as menu item keybindings.

Author: stefanhaller

pkg/gui/context/menu_context.go

@@ -56,6 +56,7 @@ type MenuViewModel struct {
 	promptLines               []string
 	columnAlignment           []utils.Alignment
 	allowFilteringKeybindings bool
+	keybindingsTakePrecedence bool
 	*FilteredListViewModel[*types.MenuItem]
 }
 
@@ -117,6 +118,10 @@ func (self *MenuViewModel) SetAllowFilteringKeybindings(allow bool) {
 	self.allowFilteringKeybindings = allow
 }
 
+func (self *MenuViewModel) SetKeybindingsTakePrecedence(value bool) {
+	self.keybindingsTakePrecedence = value
+}
+
 // TODO: move into presentation package
 func (self *MenuViewModel) GetDisplayStrings(_ int, _ int) [][]string {
 	menuItems := self.FilteredListViewModel.GetItems()
@@ -205,10 +210,19 @@ func (self *MenuContext) GetKeybindings(opts types.KeybindingsOpts) []*types.Bin
 		}
 	})
 
-	// appending because that means the menu item bindings have lower precedence.
-	// So if a basic binding is to escape from the menu, we want that to still be
-	// what happens when you press escape. This matters when we're showing the menu
-	// for all keybindings of say the files context.
+	if self.keybindingsTakePrecedence {
+		// This is used for all normal menus except the keybindings menu. In this case we want the
+		// bindings of the menu items to have higher precedence than the builtin bindings; this
+		// allows assigning a keybinding to a menu item that overrides a non-essential binding such
+		// as 'j', 'k', 'H', 'L', etc. This is safe to do because the essential bindings such as
+		// confirm and return have already been removed from the menu items in this case.
+		return append(menuItemBindings, basicBindings...)
+	}
+
+	// For the keybindings menu we didn't remove the essential bindings from the menu items, because
+	// it is important to see all bindings (as a cheat sheet for what the keys are when the menu is
+	// not open). Therefore we want the essential bindings to have higher precedence than the menu
+	// item bindings.
 	return append(basicBindings, menuItemBindings...)
 }
 

pkg/gui/menu_panel.go

@@ -58,6 +58,7 @@ func (gui *Gui) createMenu(opts types.CreateMenuOptions) error {
 	gui.State.Contexts.Menu.SetMenuItems(opts.Items, opts.ColumnAlignment)
 	gui.State.Contexts.Menu.SetPrompt(opts.Prompt)
 	gui.State.Contexts.Menu.SetAllowFilteringKeybindings(opts.AllowFilteringKeybindings)
+	gui.State.Contexts.Menu.SetKeybindingsTakePrecedence(!opts.KeepConflictingKeybindings)
 	gui.State.Contexts.Menu.SetSelection(0)
 
 	gui.Views.Menu.Title = opts.Title

pkg/integration/tests/custom_commands/custom_commands_submenu_with_special_keybindings.go

@@ -60,25 +60,14 @@ var CustomCommandsSubmenuWithSpecialKeybindings = NewIntegrationTest(NewIntegrat
 						Contains("  echo down"),
 					)
 				t.GlobalPress("j")
-				/* EXPECTED:
 				t.ExpectPopup().Alert().Title(Equals("echo j")).Content(Equals("j")).Confirm()
-				ACTUAL: */
-				// The menu stays open; 'j' didn't trigger the command; instead, it selected the
-				// next item, which we can confirm by pressing enter:
-				t.GlobalPress(keys.Universal.ConfirmMenu)
-				t.ExpectPopup().Alert().Title(Equals("echo H")).Content(Equals("H")).Confirm()
 			}).
 			Press("x").
 			Tap(func() {
 				t.ExpectPopup().Menu().
 					Title(Equals("My Custom Commands"))
 				t.GlobalPress("H")
-				/* EXPECTED:
 				t.ExpectPopup().Alert().Title(Equals("echo H")).Content(Equals("H")).Confirm()
-				ACTUAL: */
-				// The menu stays open:
-				t.ExpectPopup().Menu().
-					Title(Equals("My Custom Commands"))
 			})
 	},
 })