Allow using 'j', 'k', 'H', or 'L' as keybindings in custom command menus (#5131)

Previously they would be shown as keybindings in the menu, but they
didn't work because their builtin functionality (select next/prev line,
scroll view left/right) would take precedence.

This will allow us to revert #4934; doing that in a separate PR, see
#5132.

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/controllers/options_menu_action.go

@@ -46,12 +46,12 @@ func (self *OptionsMenuAction) Call() error {
 	appendBindings(navigation, &types.MenuSection{Title: self.c.Tr.KeybindingsMenuSectionNavigation, Column: 1})
 
 	return self.c.Menu(types.CreateMenuOptions{
-		Title:                     self.c.Tr.Keybindings,
-		Items:                     menuItems,
-		HideCancel:                true,
-		ColumnAlignment:           []utils.Alignment{utils.AlignRight, utils.AlignLeft},
-		AllowFilteringKeybindings: true,
-		KeepConfirmKeybindings:    true,
+		Title:                      self.c.Tr.Keybindings,
+		Items:                      menuItems,
+		HideCancel:                 true,
+		ColumnAlignment:            []utils.Alignment{utils.AlignRight, utils.AlignLeft},
+		AllowFilteringKeybindings:  true,
+		KeepConflictingKeybindings: true,
 	})
 }
 

pkg/gui/menu_panel.go

@@ -6,6 +6,7 @@ import (
 	"github.com/jesseduffield/lazygit/pkg/gui/keybindings"
 	"github.com/jesseduffield/lazygit/pkg/gui/types"
 	"github.com/jesseduffield/lazygit/pkg/theme"
+	"github.com/samber/lo"
 )
 
 // note: items option is mutated by this function
@@ -21,7 +22,13 @@ func (gui *Gui) createMenu(opts types.CreateMenuOptions) error {
 	}
 
 	maxColumnSize := 1
-	confirmKey := keybindings.GetKey(gui.c.UserConfig().Keybinding.Universal.ConfirmMenu)
+
+	essentialKeys := []types.Key{
+		keybindings.GetKey(gui.c.UserConfig().Keybinding.Universal.ConfirmMenu),
+		keybindings.GetKey(gui.c.UserConfig().Keybinding.Universal.Return),
+		keybindings.GetKey(gui.c.UserConfig().Keybinding.Universal.PrevItem),
+		keybindings.GetKey(gui.c.UserConfig().Keybinding.Universal.NextItem),
+	}
 
 	for _, item := range opts.Items {
 		if item.LabelColumns == nil {
@@ -34,8 +41,8 @@ func (gui *Gui) createMenu(opts types.CreateMenuOptions) error {
 
 		maxColumnSize = max(maxColumnSize, len(item.LabelColumns))
 
-		// Remove all item keybindings that are the same as the confirm binding
-		if item.Key == confirmKey && !opts.KeepConfirmKeybindings {
+		// Remove all item keybindings that are the same as one of the essential bindings
+		if !opts.KeepConflictingKeybindings && lo.Contains(essentialKeys, item.Key) {
 			item.Key = nil
 		}
 	}
@@ -51,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/gui/types/common.go

@@ -150,13 +150,13 @@ const (
 )
 
 type CreateMenuOptions struct {
-	Title                     string
-	Prompt                    string // a message that will be displayed above the menu options
-	Items                     []*MenuItem
-	HideCancel                bool
-	ColumnAlignment           []utils.Alignment
-	AllowFilteringKeybindings bool
-	KeepConfirmKeybindings    bool // if true, the keybindings that match the confirm binding will not be removed from menu items
+	Title                      string
+	Prompt                     string // a message that will be displayed above the menu options
+	Items                      []*MenuItem
+	HideCancel                 bool
+	ColumnAlignment            []utils.Alignment
+	AllowFilteringKeybindings  bool
+	KeepConflictingKeybindings bool // if true, the keybindings that match essential bindings such as confirm or return will not be removed from menu items
 }
 
 type CreatePopupPanelOpts struct {

pkg/integration/tests/custom_commands/custom_commands_submenu_with_special_keybindings.go

@@ -0,0 +1,73 @@
+package custom_commands
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var CustomCommandsSubmenuWithSpecialKeybindings = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "Using custom commands from a custom commands menu with keybindings that conflict with builtin ones",
+	ExtraCmdArgs: []string{},
+	Skip:         false,
+	SetupRepo:    func(shell *Shell) {},
+	SetupConfig: func(cfg *config.AppConfig) {
+		cfg.GetUserConfig().CustomCommands = []config.CustomCommand{
+			{
+				Key:         "x",
+				Description: "My Custom Commands",
+				CommandMenu: []config.CustomCommand{
+					{
+						Key:     "j",
+						Context: "global",
+						Command: "echo j",
+						Output:  "popup",
+					},
+					{
+						Key:     "H",
+						Context: "global",
+						Command: "echo H",
+						Output:  "popup",
+					},
+					{
+						Key:     "y",
+						Context: "global",
+						Command: "echo y",
+						Output:  "popup",
+					},
+					{
+						Key:     "<down>",
+						Context: "global",
+						Command: "echo down",
+						Output:  "popup",
+					},
+				},
+			},
+		}
+		cfg.GetUserConfig().Keybinding.Universal.ConfirmMenu = "y"
+	},
+	Run: func(t *TestDriver, keys config.KeybindingConfig) {
+		t.Views().Files().
+			Focus().
+			IsEmpty().
+			Press("x").
+			Tap(func() {
+				t.ExpectPopup().Menu().
+					Title(Equals("My Custom Commands")).
+					Lines(
+						Contains("j echo j"),
+						Contains("H echo H"),
+						Contains("  echo y"),
+						Contains("  echo down"),
+					)
+				t.GlobalPress("j")
+				t.ExpectPopup().Alert().Title(Equals("echo j")).Content(Equals("j")).Confirm()
+			}).
+			Press("x").
+			Tap(func() {
+				t.ExpectPopup().Menu().
+					Title(Equals("My Custom Commands"))
+				t.GlobalPress("H")
+				t.ExpectPopup().Alert().Title(Equals("echo H")).Content(Equals("H")).Confirm()
+			})
+	},
+})

pkg/integration/tests/test_list.go

@@ -170,6 +170,7 @@ var tests = []*components.IntegrationTest{
 	custom_commands.BasicCommand,
 	custom_commands.CheckForConflicts,
 	custom_commands.CustomCommandsSubmenu,
+	custom_commands.CustomCommandsSubmenuWithSpecialKeybindings,
 	custom_commands.FormPrompts,
 	custom_commands.GlobalContext,
 	custom_commands.MenuFromCommand,