From 9e64f7dd66ee2258d23becd254e98aa176a52c82 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 1 Jun 2025 15:54:11 +0200 Subject: [PATCH 1/4] Bump gocui and adapt lazygit code Adaptions are for this gocui commit: Cleanup: remove Is* error functions - Use errors.Is instead of quality comparisons. This is better because it matches wrapped errors as well, which we will need later in this branch. - Inline the errors.Is calls at the call sites. This is idiomatic go, we don't need helper functions for this. See https://go.dev/blog/go1.13-errors for more about this. --- go.mod | 2 +- go.sum | 4 +-- pkg/gui/layout.go | 6 ++-- pkg/gui/views.go | 3 +- pkg/integration/clients/tui.go | 9 ++--- vendor/github.com/jesseduffield/gocui/gui.go | 36 +++++++------------- vendor/modules.txt | 2 +- 7 files changed, 27 insertions(+), 35 deletions(-) diff --git a/go.mod b/go.mod index f5776edcb..aab10d119 100644 --- a/go.mod +++ b/go.mod @@ -15,7 +15,7 @@ require ( github.com/integrii/flaggy v1.4.0 github.com/jesseduffield/generics v0.0.0-20250517122708-b0b4a53a6f5c github.com/jesseduffield/go-git/v5 v5.14.1-0.20250407170251-e1a013310ccd - github.com/jesseduffield/gocui v0.3.1-0.20250529123049-319bd37ff248 + github.com/jesseduffield/gocui v0.3.1-0.20250605111917-fc5387961412 github.com/jesseduffield/kill v0.0.0-20250101124109-e216ddbe133a github.com/jesseduffield/lazycore v0.0.0-20221012050358-03d2e40243c5 github.com/jesseduffield/minimal/gitignore v0.3.3-0.20211018110810-9cde264e6b1e diff --git a/go.sum b/go.sum index acd5728cd..00fc8284b 100644 --- a/go.sum +++ b/go.sum @@ -194,8 +194,8 @@ github.com/jesseduffield/generics v0.0.0-20250517122708-b0b4a53a6f5c h1:tC2Paiis github.com/jesseduffield/generics v0.0.0-20250517122708-b0b4a53a6f5c/go.mod h1:F2fEBk0ddf6ixrBrJjY7phfQ3hL9rXG0uSjvwYe50bE= github.com/jesseduffield/go-git/v5 v5.14.1-0.20250407170251-e1a013310ccd h1:ViKj6qth8FgcIWizn9KiACWwPemWSymx62OPN0tHT+Q= github.com/jesseduffield/go-git/v5 v5.14.1-0.20250407170251-e1a013310ccd/go.mod h1:lRhCiBr6XjQrvcQVa+UYsy/99d3wMXn/a0nSQlhnhlA= -github.com/jesseduffield/gocui v0.3.1-0.20250529123049-319bd37ff248 h1:kFWTUOjkyuerq8L74MyTnpBvrBxPR4T5GpkOv/gr6/o= -github.com/jesseduffield/gocui v0.3.1-0.20250529123049-319bd37ff248/go.mod h1:sLIyZ2J42R6idGdtemZzsiR3xY5EF0KsvYEGh3dQv3s= +github.com/jesseduffield/gocui v0.3.1-0.20250605111917-fc5387961412 h1:8z1CpdCy9nzdj47lSLbDbCVmR5MgXsknYsuuHpzYk5M= +github.com/jesseduffield/gocui v0.3.1-0.20250605111917-fc5387961412/go.mod h1:sLIyZ2J42R6idGdtemZzsiR3xY5EF0KsvYEGh3dQv3s= github.com/jesseduffield/kill v0.0.0-20250101124109-e216ddbe133a h1:UDeJ3EBk04bXDLOPvuqM3on8HvyJfISw0+UMqW+0a4g= github.com/jesseduffield/kill v0.0.0-20250101124109-e216ddbe133a/go.mod h1:FSWDLKT0NQpntbDd1H3lbz51fhCVlMzy/J0S6nM727Q= github.com/jesseduffield/lazycore v0.0.0-20221012050358-03d2e40243c5 h1:CDuQmfOjAtb1Gms6a1p5L2P8RhbLUq5t8aL7PiQd2uY= diff --git a/pkg/gui/layout.go b/pkg/gui/layout.go index 53e824778..7ab7a88a0 100644 --- a/pkg/gui/layout.go +++ b/pkg/gui/layout.go @@ -1,6 +1,8 @@ package gui import ( + "errors" + "github.com/jesseduffield/gocui" "github.com/jesseduffield/lazygit/pkg/gui/types" "github.com/samber/lo" @@ -121,7 +123,7 @@ func (gui *Gui) layout(g *gocui.Gui) error { } _, err := setViewFromDimensions(context) - if err != nil && !gocui.IsUnknownView(err) { + if err != nil && !errors.Is(err, gocui.ErrUnknownView) { return err } } @@ -134,7 +136,7 @@ func (gui *Gui) layout(g *gocui.Gui) error { for _, context := range gui.transientContexts() { view, err := gui.g.View(context.GetViewName()) - if err != nil && !gocui.IsUnknownView(err) { + if err != nil && !errors.Is(err, gocui.ErrUnknownView) { return err } view.Visible = gui.helpers.Window.GetViewNameForWindow(context.GetWindowName()) == context.GetViewName() diff --git a/pkg/gui/views.go b/pkg/gui/views.go index bc7395fd6..6346b6709 100644 --- a/pkg/gui/views.go +++ b/pkg/gui/views.go @@ -1,6 +1,7 @@ package gui import ( + "errors" "fmt" "github.com/jesseduffield/gocui" @@ -78,7 +79,7 @@ func (gui *Gui) createAllViews() error { var err error for _, mapping := range gui.orderedViewNameMappings() { *mapping.viewPtr, err = gui.prepareView(mapping.name) - if err != nil && !gocui.IsUnknownView(err) { + if err != nil && !errors.Is(err, gocui.ErrUnknownView) { return err } } diff --git a/pkg/integration/clients/tui.go b/pkg/integration/clients/tui.go index af4432d60..f93644597 100644 --- a/pkg/integration/clients/tui.go +++ b/pkg/integration/clients/tui.go @@ -1,6 +1,7 @@ package clients import ( + "errors" "fmt" "log" "os" @@ -314,7 +315,7 @@ func (self *app) layout(g *gocui.Gui) error { g.FgColor = gocui.ColorGreen listView, err := g.SetView("list", 0, 0, maxX-1, maxY-descriptionViewHeight-keybindingsViewHeight-editorViewHeight-1, 0) if err != nil { - if !gocui.IsUnknownView(err) { + if !errors.Is(err, gocui.ErrUnknownView) { return err } @@ -334,7 +335,7 @@ func (self *app) layout(g *gocui.Gui) error { descriptionView, err := g.SetViewBeneath("description", "list", descriptionViewHeight) if err != nil { - if !gocui.IsUnknownView(err) { + if !errors.Is(err, gocui.ErrUnknownView) { return err } descriptionView.Title = "Test description" @@ -344,7 +345,7 @@ func (self *app) layout(g *gocui.Gui) error { keybindingsView, err := g.SetViewBeneath("keybindings", "description", keybindingsViewHeight) if err != nil { - if !gocui.IsUnknownView(err) { + if !errors.Is(err, gocui.ErrUnknownView) { return err } keybindingsView.Title = "Keybindings" @@ -355,7 +356,7 @@ func (self *app) layout(g *gocui.Gui) error { editorView, err := g.SetViewBeneath("editor", "keybindings", editorViewHeight) if err != nil { - if !gocui.IsUnknownView(err) { + if !errors.Is(err, gocui.ErrUnknownView) { return err } diff --git a/vendor/github.com/jesseduffield/gocui/gui.go b/vendor/github.com/jesseduffield/gocui/gui.go index 14f16ee71..c3dddd590 100644 --- a/vendor/github.com/jesseduffield/gocui/gui.go +++ b/vendor/github.com/jesseduffield/gocui/gui.go @@ -786,7 +786,7 @@ func (g *Gui) MainLoop() error { } func (g *Gui) handleError(err error) error { - if err != nil && !IsQuit(err) && g.ErrorHandler != nil { + if err != nil && !standardErrors.Is(err, ErrQuit) && g.ErrorHandler != nil { return g.ErrorHandler(err) } @@ -1498,6 +1498,8 @@ func (g *Gui) execKeybindings(v *View, ev *GocuiEvent) error { } } + var err error + for _, kb := range g.keybindings { if kb.handler == nil { continue @@ -1506,13 +1508,13 @@ func (g *Gui) execKeybindings(v *View, ev *GocuiEvent) error { continue } if g.matchView(v, kb) { - err := g.execKeybinding(v, kb) - if IsKeybindingNotHandled(err) { - matchingParentViewKb = nil - break - } else { + err = g.execKeybinding(v, kb) + if !errors.Is(err, ErrKeybindingNotHandled) { return err } + + matchingParentViewKb = nil + break } if v != nil && g.matchView(v.ParentView, kb) { matchingParentViewKb = kb @@ -1522,8 +1524,8 @@ func (g *Gui) execKeybindings(v *View, ev *GocuiEvent) error { } } if matchingParentViewKb != nil { - err := g.execKeybinding(v.ParentView, matchingParentViewKb) - if !IsKeybindingNotHandled(err) { + err = g.execKeybinding(v.ParentView, matchingParentViewKb) + if !errors.Is(err, ErrKeybindingNotHandled) { return err } } @@ -1536,9 +1538,9 @@ func (g *Gui) execKeybindings(v *View, ev *GocuiEvent) error { } if globalKb != nil { - return g.execKeybinding(v, globalKb) + err = g.execKeybinding(v, globalKb) } - return nil + return err } // execKeybinding executes a given keybinding @@ -1602,20 +1604,6 @@ func (g *Gui) isBlacklisted(k Key) bool { return false } -// IsUnknownView reports whether the contents of an error is "unknown view". -func IsUnknownView(err error) bool { - return err != nil && err.Error() == ErrUnknownView.Error() -} - -// IsQuit reports whether the contents of an error is "quit". -func IsQuit(err error) bool { - return err != nil && err.Error() == ErrQuit.Error() -} - -func IsKeybindingNotHandled(err error) bool { - return err != nil && err.Error() == ErrKeybindingNotHandled.Error() -} - func (g *Gui) Suspend() error { g.suspendedMutex.Lock() defer g.suspendedMutex.Unlock() diff --git a/vendor/modules.txt b/vendor/modules.txt index f29156732..e8fd23521 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -227,7 +227,7 @@ github.com/jesseduffield/go-git/v5/utils/merkletrie/internal/frame github.com/jesseduffield/go-git/v5/utils/merkletrie/noder github.com/jesseduffield/go-git/v5/utils/sync github.com/jesseduffield/go-git/v5/utils/trace -# github.com/jesseduffield/gocui v0.3.1-0.20250529123049-319bd37ff248 +# github.com/jesseduffield/gocui v0.3.1-0.20250605111917-fc5387961412 ## explicit; go 1.12 github.com/jesseduffield/gocui # github.com/jesseduffield/kill v0.0.0-20250101124109-e216ddbe133a From 37b118f4fbb4f82c072a00b6f7e66d73f03e0d55 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 1 Jun 2025 18:12:12 +0200 Subject: [PATCH 2/4] Cleanup: restructure code for clarity There was no reason to declare a variable for disabledReason, assign it inside the "if binding.GetDisabledReason != nil" statement, and then check its value again after that if statement. Move all that code inside the first if statement to make the control flow easier to understand. --- pkg/gui/keybindings.go | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/pkg/gui/keybindings.go b/pkg/gui/keybindings.go index e52f9a815..fe3b88ef3 100644 --- a/pkg/gui/keybindings.go +++ b/pkg/gui/keybindings.go @@ -523,19 +523,18 @@ func (gui *Gui) SetMouseKeybinding(binding *gocui.ViewMouseBinding) error { } func (gui *Gui) callKeybindingHandler(binding *types.Binding) error { - var disabledReason *types.DisabledReason if binding.GetDisabledReason != nil { - disabledReason = binding.GetDisabledReason() - } - if disabledReason != nil { - if disabledReason.ShowErrorInPanel { - return errors.New(disabledReason.Text) - } + if disabledReason := binding.GetDisabledReason(); disabledReason != nil { + if disabledReason.ShowErrorInPanel { + return errors.New(disabledReason.Text) + } - if len(disabledReason.Text) > 0 { - gui.c.ErrorToast(gui.Tr.DisabledMenuItemPrefix + disabledReason.Text) + if len(disabledReason.Text) > 0 { + gui.c.ErrorToast(gui.Tr.DisabledMenuItemPrefix + disabledReason.Text) + } + return nil } - return nil } + return binding.Handler() } From 3e26be9845f06439970ee1b4af02682295dc0f99 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 1 Jun 2025 16:11:20 +0200 Subject: [PATCH 3/4] Optionally pass disabled commands on to next handler If a DisabledReason has its AllowFurtherDispatching flag set, it is returned as a ErrKeybindingNotHandled error, instead of shown as a toast right away. This allows gocui to continue to dispatch the keybinding, and we can unwrap the error at the other end (in our global ErrorHandler) and display it then. This allows having keybindings for the same key at the local and global levels, and they will continue to be dispatched even if the first one returns a DisabledReason. It is opt-in, so we only use it for cases where we know that a local and a global handler share the same (default) keybinding. --- pkg/gui/keybindings.go | 4 ++++ pkg/gui/popup/popup_handler.go | 11 +++++++++++ pkg/gui/types/common.go | 4 ++++ pkg/gui/types/keybindings.go | 12 ++++++++++++ 4 files changed, 31 insertions(+) diff --git a/pkg/gui/keybindings.go b/pkg/gui/keybindings.go index fe3b88ef3..63d25b622 100644 --- a/pkg/gui/keybindings.go +++ b/pkg/gui/keybindings.go @@ -525,6 +525,10 @@ func (gui *Gui) SetMouseKeybinding(binding *gocui.ViewMouseBinding) error { func (gui *Gui) callKeybindingHandler(binding *types.Binding) error { if binding.GetDisabledReason != nil { if disabledReason := binding.GetDisabledReason(); disabledReason != nil { + if disabledReason.AllowFurtherDispatching { + return &types.ErrKeybindingNotHandled{DisabledReason: disabledReason} + } + if disabledReason.ShowErrorInPanel { return errors.New(disabledReason.Text) } diff --git a/pkg/gui/popup/popup_handler.go b/pkg/gui/popup/popup_handler.go index 8ea50de2b..7fe77a8da 100644 --- a/pkg/gui/popup/popup_handler.go +++ b/pkg/gui/popup/popup_handler.go @@ -2,6 +2,7 @@ package popup import ( "context" + "errors" "strings" "github.com/jesseduffield/gocui" @@ -80,6 +81,16 @@ func (self *PopupHandler) WithWaitingStatusSync(message string, f func() error) } func (self *PopupHandler) ErrorHandler(err error) error { + var notHandledError *types.ErrKeybindingNotHandled + if errors.As(err, ¬HandledError) { + if !notHandledError.DisabledReason.ShowErrorInPanel { + if msg := notHandledError.DisabledReason.Text; len(msg) > 0 { + self.ErrorToast(self.Tr.DisabledMenuItemPrefix + msg) + } + return nil + } + } + // Need to set bold here explicitly; otherwise it gets cancelled by the red colouring. coloredMessage := style.FgRed.SetBold().Sprint(strings.TrimSpace(err.Error())) if err := self.onErrorFn(); err != nil { diff --git a/pkg/gui/types/common.go b/pkg/gui/types/common.go index 096f95f02..2ca016825 100644 --- a/pkg/gui/types/common.go +++ b/pkg/gui/types/common.go @@ -202,6 +202,10 @@ type DisabledReason struct { // error panel instead. This is useful if the text is very long, or if it is // important enough to show it more prominently, or both. ShowErrorInPanel bool + + // If true, the keybinding dispatch mechanism will continue to look for + // other handlers for the keypress. + AllowFurtherDispatching bool } type MenuWidget int diff --git a/pkg/gui/types/keybindings.go b/pkg/gui/types/keybindings.go index db21e7bc9..e0e6af58a 100644 --- a/pkg/gui/types/keybindings.go +++ b/pkg/gui/types/keybindings.go @@ -55,3 +55,15 @@ type KeybindingGuards struct { OutsideFilterMode Guard NoPopupPanel Guard } + +type ErrKeybindingNotHandled struct { + DisabledReason *DisabledReason +} + +func (e ErrKeybindingNotHandled) Error() string { + return e.DisabledReason.Text +} + +func (e ErrKeybindingNotHandled) Unwrap() error { + return gocui.ErrKeybindingNotHandled +} From c752f3529b97a88f3a3ecd6afc2dfb6a8c37ab31 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 1 Jun 2025 17:25:38 +0200 Subject: [PATCH 4/4] Remove the pick vs. pull hack Previously we would call pullFiles() from the pick() handler if we were not in a rebase, assuming that the default keybinding for both is "p". This needn't be the case of course, if the user has remapped one or the other. The consequence of this was that swapping the keybindings for "pullFiles" and "pushFiles" would work in all panels except the Commits panel (unless "pick" was also remapped in the same way). Fix this by using the new AllowFurtherDispatching mechanism of DisabledReasons to pass the keybinding on to the next handler. --- pkg/gui/controllers/local_commits_controller.go | 15 ++------------- pkg/i18n/english.go | 2 ++ 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 2b6f3d5c0..e9cd9cbc0 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -144,14 +144,6 @@ func (self *LocalCommitsController) GetKeybindings(opts types.KeybindingsOpts) [ ), Description: self.c.Tr.Pick, Tooltip: self.c.Tr.PickCommitTooltip, - // Not displaying this because we only want to display it when a TODO commit - // is selected. A keybinding is displayed in the options view if Display is true, - // and if it's not disabled, but if we disable it whenever a non-TODO commit is - // selected, we'll be preventing pulls from happening within the commits view - // (given they both use the 'p' key). Some approaches that come to mind: - // * Allow a disabled keybinding to conditionally fallback to a global keybinding - // * Allow a separate way of deciding whether a keybinding is displayed in the options view - DisplayOnScreen: false, }, { Key: opts.GetKey(opts.Config.Commits.CreateFixupCommit), @@ -623,9 +615,7 @@ func (self *LocalCommitsController) pick(selectedCommits []*models.Commit) error return self.updateTodos(todo.Pick, selectedCommits) } - // at this point we aren't actually rebasing so we will interpret this as an - // attempt to pull. We might revoke this later after enabling configurable keybindings - return self.pullFiles() + panic("should be disabled when not rebasing") } func (self *LocalCommitsController) interactiveRebase(action todo.TodoCommand, startIdx int, endIdx int) error { @@ -1476,8 +1466,7 @@ func (self *LocalCommitsController) pickEnabled(selectedCommits []*models.Commit } if !self.isRebasing() { - // if not rebasing, we're going to do a pull so we don't care about the selection - return nil + return &types.DisabledReason{Text: self.c.Tr.PickIsOnlyAllowedDuringRebase, AllowFurtherDispatching: true} } return self.midRebaseCommandEnabled(selectedCommits, startIdx, endIdx) diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index 468231f9b..36a119c36 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -369,6 +369,7 @@ type TranslationSet struct { RewordNotSupported string ChangingThisActionIsNotAllowed string NotAllowedMidCherryPickOrRevert string + PickIsOnlyAllowedDuringRebase string DroppingMergeRequiresSingleSelection string CherryPickCopy string CherryPickCopyTooltip string @@ -1459,6 +1460,7 @@ func EnglishTranslationSet() *TranslationSet { RewordNotSupported: "Rewording commits while interactively rebasing is not currently supported", ChangingThisActionIsNotAllowed: "Changing this kind of rebase todo entry is not allowed", NotAllowedMidCherryPickOrRevert: "This action is not allowed while cherry-picking or reverting", + PickIsOnlyAllowedDuringRebase: "This action is only allowed while rebasing", DroppingMergeRequiresSingleSelection: "Dropping a merge commit requires a single selected item", CherryPickCopy: "Copy (cherry-pick)", CherryPickCopyTooltip: "Mark commit as copied. Then, within the local commits view, you can press `{{.paste}}` to paste (cherry-pick) the copied commit(s) into your checked out branch. At any time you can press `{{.escape}}` to cancel the selection.",