From e19544a42ba1c6f4b4f907ccd57cc7d686418258 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 21 Dec 2025 12:36:24 +0100 Subject: [PATCH 1/4] Add a test that demonstrates problems with custom menu keybindings The test shows two problems: a keybinding is not removed from a menu item (the 'y' binding is removed though, which is correct), and keybindings such as 'j' and 'H' don't work. We will fix both of these separately in the following commits. --- ...mmands_submenu_with_special_keybindings.go | 90 +++++++++++++++++++ pkg/integration/tests/test_list.go | 1 + 2 files changed, 91 insertions(+) create mode 100644 pkg/integration/tests/custom_commands/custom_commands_submenu_with_special_keybindings.go diff --git a/pkg/integration/tests/custom_commands/custom_commands_submenu_with_special_keybindings.go b/pkg/integration/tests/custom_commands/custom_commands_submenu_with_special_keybindings.go new file mode 100644 index 00000000000..cc23b297dcf --- /dev/null +++ b/pkg/integration/tests/custom_commands/custom_commands_submenu_with_special_keybindings.go @@ -0,0 +1,90 @@ +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: "", + 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( + /* EXPECTED: + Contains("j echo j"), + Contains("H echo H"), + Contains(" echo y"), + Contains(" echo down"), + ACTUAL: */ + Contains("j echo j"), + Contains("H echo H"), + Contains(" echo y"), + 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")) + }) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 14072d866db..9c5d57ec97c 100644 --- a/pkg/integration/tests/test_list.go +++ b/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, From 7e3b24d496bef10b86b0a8bf73d33ef623746da8 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 21 Dec 2025 11:22:02 +0100 Subject: [PATCH 2/4] In menus, remove not just the confirm binding, but also esc and up/down This is not really super important because we are very unlikely to assign a key such as esc or up/down to a menu item. However, users might do this in a custom commands menu, and in that case it is important that the builtin keys still work; or they might remap those builtin commands to other keys, in which case they might conflict with single-letter keys in normal menus. --- pkg/gui/menu_panel.go | 13 ++++++++++--- ...tom_commands_submenu_with_special_keybindings.go | 6 ------ 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/pkg/gui/menu_panel.go b/pkg/gui/menu_panel.go index 324c171c0e4..da1f45d0949 100644 --- a/pkg/gui/menu_panel.go +++ b/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.KeepConfirmKeybindings && lo.Contains(essentialKeys, item.Key) { item.Key = nil } } diff --git a/pkg/integration/tests/custom_commands/custom_commands_submenu_with_special_keybindings.go b/pkg/integration/tests/custom_commands/custom_commands_submenu_with_special_keybindings.go index cc23b297dcf..7790a378567 100644 --- a/pkg/integration/tests/custom_commands/custom_commands_submenu_with_special_keybindings.go +++ b/pkg/integration/tests/custom_commands/custom_commands_submenu_with_special_keybindings.go @@ -54,16 +54,10 @@ var CustomCommandsSubmenuWithSpecialKeybindings = NewIntegrationTest(NewIntegrat t.ExpectPopup().Menu(). Title(Equals("My Custom Commands")). Lines( - /* EXPECTED: Contains("j echo j"), Contains("H echo H"), Contains(" echo y"), Contains(" echo down"), - ACTUAL: */ - Contains("j echo j"), - Contains("H echo H"), - Contains(" echo y"), - Contains(" echo down"), ) t.GlobalPress("j") /* EXPECTED: From f996e47199a66cd8a49dc2cc0a2f0197333090ed Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 21 Dec 2025 12:06:28 +0100 Subject: [PATCH 3/4] Rename KeepConfirmKeybindings to KeepConflictingKeybindings After the change in the previous commit this expresses better what it is about. --- pkg/gui/controllers/options_menu_action.go | 12 ++++++------ pkg/gui/menu_panel.go | 2 +- pkg/gui/types/common.go | 14 +++++++------- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/pkg/gui/controllers/options_menu_action.go b/pkg/gui/controllers/options_menu_action.go index 28819caba4e..e711f9df67b 100644 --- a/pkg/gui/controllers/options_menu_action.go +++ b/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, }) } diff --git a/pkg/gui/menu_panel.go b/pkg/gui/menu_panel.go index da1f45d0949..2a4b6acca3a 100644 --- a/pkg/gui/menu_panel.go +++ b/pkg/gui/menu_panel.go @@ -42,7 +42,7 @@ func (gui *Gui) createMenu(opts types.CreateMenuOptions) error { maxColumnSize = max(maxColumnSize, len(item.LabelColumns)) // Remove all item keybindings that are the same as one of the essential bindings - if !opts.KeepConfirmKeybindings && lo.Contains(essentialKeys, item.Key) { + if !opts.KeepConflictingKeybindings && lo.Contains(essentialKeys, item.Key) { item.Key = nil } } diff --git a/pkg/gui/types/common.go b/pkg/gui/types/common.go index 2afe4f7d3b8..4c892508d4d 100644 --- a/pkg/gui/types/common.go +++ b/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 { From 344d3866a669155f394544ed07828eed631cb4c0 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 21 Dec 2025 12:04:11 +0100 Subject: [PATCH 4/4] 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. --- pkg/gui/context/menu_context.go | 22 +++++++++++++++---- pkg/gui/menu_panel.go | 1 + ...mmands_submenu_with_special_keybindings.go | 11 ---------- 3 files changed, 19 insertions(+), 15 deletions(-) diff --git a/pkg/gui/context/menu_context.go b/pkg/gui/context/menu_context.go index 0d59f1fa9a4..09781261468 100644 --- a/pkg/gui/context/menu_context.go +++ b/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...) } diff --git a/pkg/gui/menu_panel.go b/pkg/gui/menu_panel.go index 2a4b6acca3a..8a681abbd57 100644 --- a/pkg/gui/menu_panel.go +++ b/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 diff --git a/pkg/integration/tests/custom_commands/custom_commands_submenu_with_special_keybindings.go b/pkg/integration/tests/custom_commands/custom_commands_submenu_with_special_keybindings.go index 7790a378567..a1f62aff6cf 100644 --- a/pkg/integration/tests/custom_commands/custom_commands_submenu_with_special_keybindings.go +++ b/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")) }) }, })