From af0cdf6d6e4d77f26b1d1b4f0086cc8c31d43e1e Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 6 May 2026 18:44:59 +0200 Subject: [PATCH 01/28] Fix the check_for_fixups.sh script again Our most recent change to the script (58309b02a900) broke it because the anchored regex's no longer match the beginning of the subject. Fix this by omitting the hash, which is a bit unfortunate but probably acceptable (I rarely look at the output of the script anyway). --- scripts/check_for_fixups.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/check_for_fixups.sh b/scripts/check_for_fixups.sh index d2e8cb08e..5184d4820 100755 --- a/scripts/check_for_fixups.sh +++ b/scripts/check_for_fixups.sh @@ -2,7 +2,7 @@ # We will have only done a shallow clone, so the git log will consist only of # commits on the current PR -commits=$(git log --format="%h %s" | egrep '(^fixup!|^squash!|^amend!|WIP|DROPME)') +commits=$(git log --format="%s" | egrep '(^fixup!|^squash!|^amend!|WIP|DROPME)') if [ -z "$commits" ]; then echo "No fixup commits found." From 164f7da33dc82610bd910b71a566e15170414fe4 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 4 May 2026 18:39:54 +0200 Subject: [PATCH 02/28] Fix missing layout call after switching repos This didn't cause a bug so far because switching repos always happens from within an OnWaitingStatus, so the spinner would take care of calling layout and draw. However, later in this branch we are going to optimize the spinner so that it no longer calls layout, at which point this would break, so make sure we rerender at the point where it's needed. --- pkg/gui/gui.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/gui/gui.go b/pkg/gui/gui.go index ce22f4240..1f49b857a 100644 --- a/pkg/gui/gui.go +++ b/pkg/gui/gui.go @@ -428,6 +428,8 @@ func (gui *Gui) onNewRepo(startArgs appTypes.StartArgs, contextKey types.Context gui.c.Context().Push(contextToPush, types.OnFocusOpts{}) + gui.render() + return nil } From 79727f1780db4ba5d9c627930d08fb47ea98dab3 Mon Sep 17 00:00:00 2001 From: Antoine Gaudreau Simard Date: Mon, 4 May 2026 14:53:33 -0400 Subject: [PATCH 03/28] Honor the spinner rate configurate with sync spinner --- pkg/gui/controllers/helpers/app_status_helper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/gui/controllers/helpers/app_status_helper.go b/pkg/gui/controllers/helpers/app_status_helper.go index 89de221b8..e7a75f17c 100644 --- a/pkg/gui/controllers/helpers/app_status_helper.go +++ b/pkg/gui/controllers/helpers/app_status_helper.go @@ -111,7 +111,7 @@ func (self *AppStatusHelper) renderAppStatus() { func (self *AppStatusHelper) renderAppStatusSync(stop chan struct{}) { go func() { - ticker := time.NewTicker(time.Millisecond * 50) + ticker := time.NewTicker(time.Millisecond * time.Duration(self.c.UserConfig().Gui.Spinner.Rate)) defer ticker.Stop() // Forcing a re-layout and redraw after we added the waiting status; From 9c3e7dac888394543513b93cc0cd4b84c19ff481 Mon Sep 17 00:00:00 2001 From: Antoine Gaudreau Simard Date: Tue, 5 May 2026 18:34:51 -0400 Subject: [PATCH 04/28] Support to request a content-only UI refresh This skips the whole-UI layout calculations, and lets tcell's dirty cell handling redraw only changed cells. --- pkg/gocui/flush_test.go | 202 ++++++++++++++++++++++++++++++++++++++++ pkg/gocui/gui.go | 54 +++++++++-- pkg/gui/gui.go | 6 ++ pkg/gui/gui_common.go | 4 + pkg/gui/types/common.go | 4 + 5 files changed, 261 insertions(+), 9 deletions(-) create mode 100644 pkg/gocui/flush_test.go diff --git a/pkg/gocui/flush_test.go b/pkg/gocui/flush_test.go new file mode 100644 index 000000000..2447901f5 --- /dev/null +++ b/pkg/gocui/flush_test.go @@ -0,0 +1,202 @@ +package gocui + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func newTestGui(t *testing.T) *Gui { + t.Helper() + g, err := NewGui(NewGuiOpts{ + OutputMode: OutputNormal, + Headless: true, + Width: 80, + Height: 24, + }) + assert.NoError(t, err) + t.Cleanup(func() { g.Close() }) + return g +} + +// setupViews creates a few views and does an initial full flush so all views +// start in a clean (non-tainted) state. +func setupViews(t *testing.T, g *Gui) (*View, *View) { + t.Helper() + + status, _ := g.SetView("status", 0, 22, 40, 24, 0) + status.Frame = false + main, _ := g.SetView("main", 0, 0, 80, 22, 0) + + // Initial content + status.SetContent("Ready") + main.SetContent("hello world") + + // Full flush to draw everything and clear tainted flags + assert.NoError(t, g.flush()) + + return status, main +} + +// pushContentOnly pushes a content-only event directly to the channel +// (synchronous, deterministic — unlike Update which spawns a goroutine). +func pushContentOnly(g *Gui, f func(*Gui) error) { + g.userEvents <- userEvent{f: f, task: g.NewTask(), contentOnly: true} +} + +// pushRegular pushes a regular event directly to the channel. +func pushRegular(g *Gui, f func(*Gui) error) { + g.userEvents <- userEvent{f: f, task: g.NewTask(), contentOnly: false} +} + +func TestFlushContentOnly_SkipsUntaintedViews(t *testing.T) { + g := newTestGui(t) + status, main := setupViews(t, g) + + // After initial flush, both views should be untainted + assert.False(t, status.IsTainted(), "status view should not be tainted after flush") + assert.False(t, main.IsTainted(), "main view should not be tainted after flush") + + // Modify only the status view + status.SetContent("Fetching /") + + assert.True(t, status.IsTainted(), "status view should be tainted after SetContent") + assert.False(t, main.IsTainted(), "main view should not be tainted (was not modified)") + + // flushContentOnly should succeed and clear status tainted flag + assert.NoError(t, g.flushContentOnly()) + + assert.False(t, status.IsTainted(), "status view should not be tainted after flushContentOnly") + assert.False(t, main.IsTainted(), "main view should not be tainted after flushContentOnly") +} + +func TestFlushContentOnly_WritesCorrectContent(t *testing.T) { + g := newTestGui(t) + status, _ := setupViews(t, g) + + status.SetContent("Fetching |") + assert.NoError(t, g.flushContentOnly()) + + assert.Equal(t, "Fetching |", status.Buffer()) +} + +func TestProcessEvent_ContentOnlyEvent_SkipsTaintedCheck(t *testing.T) { + g := newTestGui(t) + status, main := setupViews(t, g) + + // Send a content-only event that modifies only the status view + pushContentOnly(g, func(gui *Gui) error { + status.SetContent("Fetching /") + return nil + }) + + assert.NoError(t, g.processEvent()) + + // status was modified and drawn → tainted cleared + assert.False(t, status.IsTainted(), "status should not be tainted after processEvent with contentOnly") + // main was NOT modified → should still be untainted + assert.False(t, main.IsTainted(), "main should not be tainted after processEvent with contentOnly") +} + +func TestProcessEvent_RegularEvent_UsesFullFlush(t *testing.T) { + g := newTestGui(t) + status, _ := setupViews(t, g) + + // Regular event (not content-only) should trigger full flush + pushRegular(g, func(gui *Gui) error { + status.SetContent("Fetching \\") + return nil + }) + + assert.NoError(t, g.processEvent()) + + assert.False(t, status.IsTainted(), "status should not be tainted after full flush") +} + +func TestProcessEvent_MixedBatch_UsesFullFlush(t *testing.T) { + g := newTestGui(t) + status, main := setupViews(t, g) + + // Queue a content-only event followed by a regular event. + // processEvent picks up the first; processRemainingEvents picks up + // the second. Since the second is not contentOnly, full flush runs. + pushContentOnly(g, func(gui *Gui) error { + status.SetContent("Fetching -") + return nil + }) + pushRegular(g, func(gui *Gui) error { + main.SetContent("updated main") + return nil + }) + + assert.NoError(t, g.processEvent()) + + // Both views were modified and should have been drawn by full flush + assert.False(t, status.IsTainted(), "status should not be tainted after full flush") + assert.False(t, main.IsTainted(), "main should not be tainted after full flush") +} + +func TestProcessEvent_RegularThenContentOnly_UsesFullFlush(t *testing.T) { + g := newTestGui(t) + status, main := setupViews(t, g) + + // Even if a regular event comes first and the remaining are contentOnly, + // the batch must use full flush. + pushRegular(g, func(gui *Gui) error { + main.SetContent("new main content") + return nil + }) + pushContentOnly(g, func(gui *Gui) error { + status.SetContent("Fetching |") + return nil + }) + + assert.NoError(t, g.processEvent()) + + assert.False(t, status.IsTainted(), "status should not be tainted after full flush") + assert.False(t, main.IsTainted(), "main should not be tainted after full flush") +} + +func TestProcessRemainingEvents_AllContentOnly_ReturnsTrue(t *testing.T) { + g := newTestGui(t) + status, _ := setupViews(t, g) + + pushContentOnly(g, func(gui *Gui) error { + status.SetContent("a") + return nil + }) + pushContentOnly(g, func(gui *Gui) error { + status.SetContent("b") + return nil + }) + + contentOnly, err := g.processRemainingEvents() + assert.NoError(t, err) + assert.True(t, contentOnly, "should return true when all events are contentOnly") +} + +func TestProcessRemainingEvents_MixedEvents_ReturnsFalse(t *testing.T) { + g := newTestGui(t) + status, _ := setupViews(t, g) + + pushContentOnly(g, func(gui *Gui) error { + status.SetContent("a") + return nil + }) + pushRegular(g, func(gui *Gui) error { + status.SetContent("b") + return nil + }) + + contentOnly, err := g.processRemainingEvents() + assert.NoError(t, err) + assert.False(t, contentOnly, "should return false when any event is not contentOnly") +} + +func TestProcessRemainingEvents_EmptyQueue_ReturnsTrue(t *testing.T) { + g := newTestGui(t) + + contentOnly, err := g.processRemainingEvents() + assert.NoError(t, err) + assert.True(t, contentOnly, "should return true when no events are queued") +} diff --git a/pkg/gocui/gui.go b/pkg/gocui/gui.go index dded5dd4a..73e358f55 100644 --- a/pkg/gocui/gui.go +++ b/pkg/gocui/gui.go @@ -604,6 +604,10 @@ func (g *Gui) SetRenderSearchStatusFunc(renderSearchStatusFunc func(*View, int, type userEvent struct { f func(*Gui) error task Task + // Signals that this event only modifies view content (e.g. SetContent). + // When all events in a batch are contentOnly, processEvent + // can skip the expensive layout() call in flush(). + contentOnly bool } // Update executes the passed function. This method can be called safely from a @@ -630,6 +634,12 @@ func (g *Gui) updateAsyncAux(f func(*Gui) error, task Task) { g.userEvents <- userEvent{f: f, task: task} } +// Like Update, but signals that the callback only modifies content. +func (g *Gui) UpdateContentOnly(f func(*Gui) error) { + task := g.NewTask() + g.userEvents <- userEvent{f: f, task: task, contentOnly: true} +} + // Calls a function in a goroutine. Handles panics gracefully and tracks // number of background tasks. // Always use this when you want to spawn a goroutine and you want lazygit to @@ -743,6 +753,8 @@ func (g *Gui) handleError(err error) error { } func (g *Gui) processEvent() error { + contentOnly := false + select { case ev := <-g.gEvents: task := g.NewTask() @@ -752,6 +764,7 @@ func (g *Gui) processEvent() error { return err } case ev := <-g.userEvents: + contentOnly = ev.contentOnly defer func() { ev.task.Done() }() if err := g.handleError(ev.f(g)); err != nil { @@ -759,32 +772,38 @@ func (g *Gui) processEvent() error { } } - if err := g.processRemainingEvents(); err != nil { - return err - } - if err := g.flush(); err != nil { + remainingContentOnly, err := g.processRemainingEvents() + if err != nil { return err } + contentOnly = contentOnly && remainingContentOnly - return nil + if contentOnly { + return g.flushContentOnly() + } + return g.flush() } // processRemainingEvents handles the remaining events in the events pool. -func (g *Gui) processRemainingEvents() error { +// Returns true if all processed events were content-only. +func (g *Gui) processRemainingEvents() (bool, error) { + contentOnly := true for { select { case ev := <-g.gEvents: + contentOnly = false if err := g.handleError(g.handleEvent(&ev)); err != nil { - return err + return false, err } case ev := <-g.userEvents: + contentOnly = ev.contentOnly && contentOnly err := g.handleError(ev.f(g)) ev.task.Done() if err != nil { - return err + return false, err } default: - return nil + return contentOnly, nil } } } @@ -1169,6 +1188,23 @@ func (g *Gui) ForceRedrawViews(views ...*View) error { return nil } +// Redraws only tainted views and skips the layout pass. +// tcell's cell-level dirty tracking ensures only +// actually-changed cells are emitted to the terminal. +func (g *Gui) flushContentOnly() error { + for _, v := range g.views { + if !v.tainted { + continue + } + if err := g.draw(v); err != nil { + return err + } + } + + Screen.Show() + return nil +} + // draw manages the cursor and calls the draw function of a view. func (g *Gui) draw(v *View) error { if g.suspended { diff --git a/pkg/gui/gui.go b/pkg/gui/gui.go index 1f49b857a..458e33804 100644 --- a/pkg/gui/gui.go +++ b/pkg/gui/gui.go @@ -1188,6 +1188,12 @@ func (gui *Gui) onUIThread(f func() error) { }) } +func (gui *Gui) onUIThreadContentOnly(f func() error) { + gui.g.UpdateContentOnly(func(*gocui.Gui) error { + return f() + }) +} + func (gui *Gui) onWorker(f func(gocui.Task) error) { gui.g.OnWorker(f) } diff --git a/pkg/gui/gui_common.go b/pkg/gui/gui_common.go index 13f49c46b..07945c350 100644 --- a/pkg/gui/gui_common.go +++ b/pkg/gui/gui_common.go @@ -120,6 +120,10 @@ func (self *guiCommon) OnUIThread(f func() error) { self.gui.onUIThread(f) } +func (self *guiCommon) OnUIThreadContentOnly(f func() error) { + self.gui.onUIThreadContentOnly(f) +} + func (self *guiCommon) OnWorker(f func(gocui.Task) error) { self.gui.onWorker(f) } diff --git a/pkg/gui/types/common.go b/pkg/gui/types/common.go index 0afbd5ed8..5856bb5e0 100644 --- a/pkg/gui/types/common.go +++ b/pkg/gui/types/common.go @@ -71,6 +71,10 @@ type IGuiCommon interface { // Only necessary to call if you're not already on the UI thread i.e. you're inside a goroutine. // All controller handlers are executed on the UI thread. OnUIThread(f func() error) + // Like OnUIThread, but signals that the callback only modifies view + // content (e.g. spinner), allows the event loop to skip + // the expensive layout recalculation when only content changed. + OnUIThreadContentOnly(f func() error) // Runs a function in a goroutine. Use this whenever you want to run a goroutine and keep track of the fact // that lazygit is still busy. See docs/dev/Busy.md OnWorker(f func(gocui.Task) error) From 0d195077e4a32a4241b19b1c29939a2f274fc3dd Mon Sep 17 00:00:00 2001 From: Antoine Gaudreau Simard Date: Tue, 5 May 2026 18:35:50 -0400 Subject: [PATCH 05/28] Improve performance when drawing the spinner on background events --- pkg/gui/controllers/helpers/app_status_helper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/gui/controllers/helpers/app_status_helper.go b/pkg/gui/controllers/helpers/app_status_helper.go index e7a75f17c..4a6e7726e 100644 --- a/pkg/gui/controllers/helpers/app_status_helper.go +++ b/pkg/gui/controllers/helpers/app_status_helper.go @@ -96,7 +96,7 @@ func (self *AppStatusHelper) renderAppStatus() { for range ticker.C { appStatus, color := self.statusMgr().GetStatusString(self.c.UserConfig()) self.c.Views().AppStatus.FgColor = color - self.c.OnUIThread(func() error { + self.c.OnUIThreadContentOnly(func() error { self.c.SetViewContent(self.c.Views().AppStatus, appStatus) return nil }) From 5dcc93e8cc67b22e5493fedb7fbb0c59cd1689df Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 3 May 2026 14:14:23 +0200 Subject: [PATCH 06/28] Validate that gui.spinner.frames must all have the same width The spinner looks weird if they don't. While we're at it, validate that frames must not be empty, which would have crashed with a division by zero. --- pkg/config/user_config_validation.go | 19 ++++++++++++++ pkg/config/user_config_validation_test.go | 30 +++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/pkg/config/user_config_validation.go b/pkg/config/user_config_validation.go index 23215eab6..7eeab32dd 100644 --- a/pkg/config/user_config_validation.go +++ b/pkg/config/user_config_validation.go @@ -1,6 +1,7 @@ package config import ( + "errors" "fmt" "log" "reflect" @@ -8,6 +9,8 @@ import ( "strings" "github.com/jesseduffield/lazygit/pkg/constants" + "github.com/jesseduffield/lazygit/pkg/utils" + "github.com/samber/lo" ) func (config *UserConfig) Validate() error { @@ -49,6 +52,22 @@ func (config *UserConfig) Validate() error { if err := validateCustomCommands(config.CustomCommands); err != nil { return err } + if err := validateSpinner(config.Gui.Spinner); err != nil { + return err + } + return nil +} + +func validateSpinner(spinner SpinnerConfig) error { + if len(spinner.Frames) == 0 { + return errors.New("gui.spinner.frames must not be empty.") + } + firstWidth := utils.StringWidth(spinner.Frames[0]) + if lo.SomeBy(spinner.Frames, func(frame string) bool { + return utils.StringWidth(frame) != firstWidth + }) { + return errors.New("All gui.spinner.frames entries must have the same width.") + } return nil } diff --git a/pkg/config/user_config_validation_test.go b/pkg/config/user_config_validation_test.go index 7fbfd00d4..a2841684d 100644 --- a/pkg/config/user_config_validation_test.go +++ b/pkg/config/user_config_validation_test.go @@ -289,3 +289,33 @@ func TestUserConfigValidate_enums(t *testing.T) { }) } } + +func TestUserConfigValidate_spinnerFrames(t *testing.T) { + scenarios := []struct { + name string + frames []string + valid bool + }{ + {name: "empty", frames: []string{}, valid: false}, + {name: "single frame", frames: []string{"|"}, valid: true}, + {name: "all same width", frames: []string{"|", "/", "-", "\\"}, valid: true}, + {name: "all same width, multi-char", frames: []string{". ", ".. ", "..."}, valid: true}, + {name: "all same width, wide runes", frames: []string{"⠋", "⠙", "⠹"}, valid: true}, + {name: "differing widths", frames: []string{"|", "//"}, valid: false}, + {name: "first differs from rest", frames: []string{"||", "/", "-"}, valid: false}, + } + + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + config := GetDefaultConfig() + config.Gui.Spinner.Frames = s.frames + err := config.Validate() + + if s.valid { + assert.NoError(t, err) + } else { + assert.Error(t, err) + } + }) + } +} From b7edcbad3a805990dbc4198c44e4066543121bb4 Mon Sep 17 00:00:00 2001 From: Antoine Gaudreau Simard Date: Tue, 5 May 2026 18:39:07 -0400 Subject: [PATCH 07/28] Improve performance of spinner in Synchronized events Instead of redrawing the two views on each tick, only redraw the spinner --- pkg/gocui/flush_test.go | 4 +- pkg/gocui/gui.go | 38 +++++++------------ .../controllers/helpers/app_status_helper.go | 2 +- 3 files changed, 17 insertions(+), 27 deletions(-) diff --git a/pkg/gocui/flush_test.go b/pkg/gocui/flush_test.go index 2447901f5..a27943d7c 100644 --- a/pkg/gocui/flush_test.go +++ b/pkg/gocui/flush_test.go @@ -64,7 +64,7 @@ func TestFlushContentOnly_SkipsUntaintedViews(t *testing.T) { assert.False(t, main.IsTainted(), "main view should not be tainted (was not modified)") // flushContentOnly should succeed and clear status tainted flag - assert.NoError(t, g.flushContentOnly()) + assert.NoError(t, g.flushContentOnly(g.views)) assert.False(t, status.IsTainted(), "status view should not be tainted after flushContentOnly") assert.False(t, main.IsTainted(), "main view should not be tainted after flushContentOnly") @@ -75,7 +75,7 @@ func TestFlushContentOnly_WritesCorrectContent(t *testing.T) { status, _ := setupViews(t, g) status.SetContent("Fetching |") - assert.NoError(t, g.flushContentOnly()) + assert.NoError(t, g.flushContentOnly(g.views)) assert.Equal(t, "Fetching |", status.Buffer()) } diff --git a/pkg/gocui/gui.go b/pkg/gocui/gui.go index 73e358f55..dac7295a3 100644 --- a/pkg/gocui/gui.go +++ b/pkg/gocui/gui.go @@ -779,7 +779,7 @@ func (g *Gui) processEvent() error { contentOnly = contentOnly && remainingContentOnly if contentOnly { - return g.flushContentOnly() + return g.flushContentOnly(g.views) } return g.flush() } @@ -1167,32 +1167,11 @@ func (g *Gui) flush() error { return nil } -func (g *Gui) ForceLayoutAndRedraw() error { - return g.flush() -} - -// force redrawing one or more views outside of the normal main loop. Useful during longer -// operations that block the main thread, to update a spinner in a status view. -func (g *Gui) ForceRedrawViews(views ...*View) error { - for _, m := range g.managers { - if err := m.Layout(g); err != nil { - return err - } - } - - for _, v := range views { - v.draw() - } - - Screen.Show() - return nil -} - // Redraws only tainted views and skips the layout pass. // tcell's cell-level dirty tracking ensures only // actually-changed cells are emitted to the terminal. -func (g *Gui) flushContentOnly() error { - for _, v := range g.views { +func (g *Gui) flushContentOnly(views []*View) error { + for _, v := range views { if !v.tainted { continue } @@ -1205,6 +1184,17 @@ func (g *Gui) flushContentOnly() error { return nil } +func (g *Gui) ForceLayoutAndRedraw() error { + return g.flush() +} + +// Redraws only tainted views outside of the normal main +// loop, without a layout pass. Useful during longer operations that block the +// main thread, e.g. to update a spinner in a status view. +func (g *Gui) ForceFlushViewsContentOnly(views []*View) error { + return g.flushContentOnly(views) +} + // draw manages the cursor and calls the draw function of a view. func (g *Gui) draw(v *View) error { if g.suspended { diff --git a/pkg/gui/controllers/helpers/app_status_helper.go b/pkg/gui/controllers/helpers/app_status_helper.go index 4a6e7726e..d71faeb65 100644 --- a/pkg/gui/controllers/helpers/app_status_helper.go +++ b/pkg/gui/controllers/helpers/app_status_helper.go @@ -136,7 +136,7 @@ func (self *AppStatusHelper) renderAppStatusSync(stop chan struct{}) { self.c.Views().AppStatus, self.c.Views().Options, self.c.Views().Information, self.c.Views().StatusSpacer1, self.c.Views().StatusSpacer2, } - _ = self.c.GocuiGui().ForceRedrawViews(bottomLineViews...) + _ = self.c.GocuiGui().ForceFlushViewsContentOnly(bottomLineViews) case <-stop: break outer } From 4f0393f97b06e4586c74d18b260f67c472c06119 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 8 May 2026 22:37:00 +0200 Subject: [PATCH 08/28] Fix visual glitch with status bar spinner at very slow spinner rates With a very slow spinner rate (seconds), you can see that at the beginning of an operation the bottom line leaves a gap for where the status will go, but the status (and spinner) is only drawn the first time the spinner ticks. The reason is that layout looks at GetStatusString to decide how much room to leave, rather than at the actual content of the AppStatus view; the view content is only set by renderAppStatus the first time the spinner ticks. Fix this by making layout look at the actual content of the view so that the layout is in sync with what is drawn. This also avoids flicker if an operation is so fast that it finishes before the spinner ticks for the first time, especially for users who set gui.showBottomLine to false. --- pkg/gui/layout.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/gui/layout.go b/pkg/gui/layout.go index 21d83ca04..dacd93f68 100644 --- a/pkg/gui/layout.go +++ b/pkg/gui/layout.go @@ -5,6 +5,7 @@ import ( "github.com/jesseduffield/lazygit/pkg/gocui" "github.com/jesseduffield/lazygit/pkg/gui/types" + "github.com/jesseduffield/lazygit/pkg/utils" "github.com/samber/lo" ) @@ -23,7 +24,11 @@ func (gui *Gui) layout(g *gocui.Gui) error { informationStr := gui.informationStr() - appStatus := gui.helpers.AppStatus.GetStatusString() + var appStatus string + appStatusView, err := g.View("appStatus") + if err == nil { + appStatus = utils.Decolorise(appStatusView.Buffer()) + } viewDimensions := gui.getWindowDimensions(informationStr, appStatus) From b3deef31ad7ec14ab315807a2bc75c13c2ce3901 Mon Sep 17 00:00:00 2001 From: Antoine Gaudreau Simard Date: Thu, 7 May 2026 21:49:00 -0400 Subject: [PATCH 09/28] When drawing tainted views in ForceFlushViewsContentOnly, also draw views that overlap them This prevents views from drawing over higher z-order views. Currently this is not an issue in practice, because we use ForceFlushViewsContentOnly only for the bottom line status spinner, and there are never views on top of it. However, later in the branch we will use the mechanism to redraw the inline spinners in panels (e.g. the "Pushing..." status next to a branch name), and there could be a popup on top of it. Co-authored-by: Stefan Haller --- pkg/gocui/flush_test.go | 90 +++++++++++++++++++++++++++++++++++++++++ pkg/gocui/gui.go | 38 +++++++++++++++-- 2 files changed, 124 insertions(+), 4 deletions(-) diff --git a/pkg/gocui/flush_test.go b/pkg/gocui/flush_test.go index a27943d7c..59bae427c 100644 --- a/pkg/gocui/flush_test.go +++ b/pkg/gocui/flush_test.go @@ -1,6 +1,7 @@ package gocui import ( + "strings" "testing" "github.com/stretchr/testify/assert" @@ -200,3 +201,92 @@ func TestProcessRemainingEvents_EmptyQueue_ReturnsTrue(t *testing.T) { assert.NoError(t, err) assert.True(t, contentOnly, "should return true when no events are queued") } + +// Ensure an overlapping view that is not tainted does not get overdrawn +func TestFlushContentOnly_DoesNotOverdrawHigherZViews(t *testing.T) { + g := newTestGui(t) + + // Base view + list, _ := g.SetView("list", 0, 0, 79, 23, 0) + list.Frame = false + list.SetContent(strings.Repeat("LIST LINE FILLER FILLER FILLER FILLER FILLER FILLER FILLER FILLER FILLER\n", 22)) + + // Overlapping 'popup' + popup, _ := g.SetView("popup", 20, 8, 60, 16, 0) + popup.Frame = false + popupLine := strings.Repeat("P", 60) + popup.SetContent(strings.Repeat(popupLine+"\n", 16)) + + // Full flush — popup ends up on top. + assert.NoError(t, g.flush()) + + cellAt := func(x, y int) string { + s, _, _ := g.screen.Get(x, y) + return s + } + + // Taint only the list view + list.SetContent(strings.Repeat(strings.Repeat("X", 80)+"\n", 22)) + assert.True(t, list.IsTainted(), "list should be tainted after SetContent") + assert.False(t, popup.IsTainted(), "popup should not be tainted") + + // flushContentOnly is what spinner ticks ultimately invoke. + assert.NoError(t, g.flushContentOnly(g.views)) + + assert.Equal(t, "P", cellAt(21, 9), + "popup region must still show popup content after flushContentOnly; "+ + "if this fails the popup-overdraw bug is present") + + // Additional checks to be sure + assert.Equal(t, "P", cellAt(40, 11), "interior popup cell should still show popup content") + assert.Equal(t, "P", cellAt(58, 14), "near-edge popup cell should still show popup content") + + // Ensure tainted view was updated + assert.Equal(t, "X", cellAt(5, 5), "list cell outside popup should show new list content") + assert.Equal(t, "X", cellAt(70, 20), "list cell outside popup should show new list content") +} + +// Ensure transitive overlap: with views in z-order [a, b, c] where b overlaps a +// and c overlaps b but c does NOT overlap a, tainting a must redraw all three — +// otherwise b's redraw paints over c. +func TestFlushContentOnly_RedrawsTransitivelyOverlappingViews(t *testing.T) { + g := newTestGui(t) + + // Geometry: b straddles a and c; a and c are disjoint. + // a: (0,0)-(40,10) b: (30,5)-(60,15) c: (50,12)-(75,20) + a, _ := g.SetView("a", 0, 0, 40, 10, 0) + a.Frame = false + a.SetContent(strings.Repeat(strings.Repeat("A", 60)+"\n", 20)) + + b, _ := g.SetView("b", 30, 5, 60, 15, 0) + b.Frame = false + b.SetContent(strings.Repeat(strings.Repeat("B", 60)+"\n", 20)) + + c, _ := g.SetView("c", 50, 12, 75, 20, 0) + c.Frame = false + c.SetContent(strings.Repeat(strings.Repeat("C", 60)+"\n", 20)) + + assert.NoError(t, g.flush()) + + cellAt := func(x, y int) string { + s, _, _ := g.screen.Get(x, y) + return s + } + + // Taint only a. + a.SetContent(strings.Repeat(strings.Repeat("X", 60)+"\n", 20)) + assert.True(t, a.IsTainted()) + assert.False(t, b.IsTainted()) + assert.False(t, c.IsTainted()) + + assert.NoError(t, g.flushContentOnly(g.views)) + + // a redrawn (direct). + assert.Equal(t, "X", cellAt(5, 5), "a should be redrawn (tainted)") + // b redrawn (overlaps a). + assert.Equal(t, "B", cellAt(45, 7), "b should be redrawn (overlaps a)") + // c redrawn transitively (overlaps b, which overlaps a). Without the + // transitive case, b's redraw would paint over c at this cell. + assert.Equal(t, "C", cellAt(55, 14), + "c should be redrawn transitively; if 'B' here, b's redraw painted over c") +} diff --git a/pkg/gocui/gui.go b/pkg/gocui/gui.go index dac7295a3..645329d6b 100644 --- a/pkg/gocui/gui.go +++ b/pkg/gocui/gui.go @@ -13,7 +13,9 @@ import ( "github.com/gdamore/tcell/v3" "github.com/go-errors/errors" + "github.com/jesseduffield/generics/set" "github.com/rivo/uniseg" + "github.com/samber/lo" ) // OutputMode represents an output mode, which determines how colors @@ -1170,11 +1172,9 @@ func (g *Gui) flush() error { // Redraws only tainted views and skips the layout pass. // tcell's cell-level dirty tracking ensures only // actually-changed cells are emitted to the terminal. +// Will also redraw any views that overlap tainted views func (g *Gui) flushContentOnly(views []*View) error { - for _, v := range views { - if !v.tainted { - continue - } + for _, v := range viewsToRedrawContentOnly(views) { if err := g.draw(v); err != nil { return err } @@ -1184,6 +1184,36 @@ func (g *Gui) flushContentOnly(views []*View) error { return nil } +func viewsToRedrawContentOnly(views []*View) []*View { + redrawIndexes := set.New[int]() + + for i, v := range views { + if !v.tainted && !redrawIndexes.Includes(i) { + continue + } + + redrawIndexes.Add(i) + + for j, above := range views[i+1:] { + aboveIndex := i + 1 + j + if !redrawIndexes.Includes(aboveIndex) && rectsOverlap(v, above) { + redrawIndexes.Add(aboveIndex) + } + } + } + + return lo.FilterMap(views, func(view *View, i int) (*View, bool) { + return view, redrawIndexes.Includes(i) + }) +} + +// Reports whether two views' rectangles share at least one cell. +func rectsOverlap(a, b *View) bool { + ax0, ay0, ax1, ay1 := a.Dimensions() + bx0, by0, bx1, by1 := b.Dimensions() + return ax0 <= bx1 && ax1 >= bx0 && ay0 <= by1 && ay1 >= by0 +} + func (g *Gui) ForceLayoutAndRedraw() error { return g.flush() } From 670565c175b0eaec0d0c0ae28b5eba2086668f30 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 7 May 2026 14:11:57 +0200 Subject: [PATCH 10/28] Have renderAppStatus trigger a full layout when the appStatus width changes In 0d195077e4a3 we improved the performance of the status bar spinner by avoiding a layout. This is fine from one spinner tick to the next, but it's a problem when spinning starts or ends (or in the hypothetical case that the status text changes in the middle of the operation, which we never do in lazygit, but theoretically could). In this case a layout is needed so that the rest of the status bar gets pushed over appropriately (or moves back to the left when the spinner ends), and also so that the bottom line is shown or hidden properly for users who set gui.showBottomLine to false. To fix this, keep track of the status string width and force a layout whenever it changes. This includes the beginning and end of an operation when it changes from empty to non-empty or vice versa. There is currently no observable misbehavior from this bug, but that's only because we must have a HandleRender call somewhere that forces a full layout when an operation starts or ends. We will remove the Render() call from HandleRender at the end of this branch, at which point the misbehavior would be visible if we didn't fix it here. --- pkg/gui/controllers/helpers/app_status_helper.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/pkg/gui/controllers/helpers/app_status_helper.go b/pkg/gui/controllers/helpers/app_status_helper.go index d71faeb65..83500d3a7 100644 --- a/pkg/gui/controllers/helpers/app_status_helper.go +++ b/pkg/gui/controllers/helpers/app_status_helper.go @@ -6,6 +6,7 @@ import ( "github.com/jesseduffield/lazygit/pkg/gocui" "github.com/jesseduffield/lazygit/pkg/gui/status" "github.com/jesseduffield/lazygit/pkg/gui/types" + "github.com/jesseduffield/lazygit/pkg/utils" ) type AppStatusHelper struct { @@ -93,13 +94,24 @@ func (self *AppStatusHelper) renderAppStatus() { self.c.OnWorker(func(_ gocui.Task) error { ticker := time.NewTicker(time.Millisecond * time.Duration(self.c.UserConfig().Gui.Spinner.Rate)) defer ticker.Stop() + prevAppStatus := "" for range ticker.C { appStatus, color := self.statusMgr().GetStatusString(self.c.UserConfig()) self.c.Views().AppStatus.FgColor = color - self.c.OnUIThreadContentOnly(func() error { + + update := self.c.OnUIThreadContentOnly + if utils.StringWidth(appStatus) != utils.StringWidth(prevAppStatus) { + // Need a full layout whenever the width of the status string changes. This can't + // happen during normal spinning because we validate that all spinner frames have + // the same width, so typically this will only be triggered at the beginning and end + // of a status, or if the status string changes midway for some reason. + update = self.c.OnUIThread + } + update(func() error { self.c.SetViewContent(self.c.Views().AppStatus, appStatus) return nil }) + prevAppStatus = appStatus if appStatus == "" { break From 79440c0945263fce7373efcd25cf2fbf710d4911 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 7 May 2026 20:40:09 +0200 Subject: [PATCH 11/28] Set the view color on the UI thread too Unrelated to this branch, just because we're touching this code: there's little reason to set the color on the background thread but the text on the UI thread. Set them both together on the UI thread. Avoids a data race (unlikely to be a problem in practice, we're talking about a 64-bit int, but still). --- pkg/gui/controllers/helpers/app_status_helper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/gui/controllers/helpers/app_status_helper.go b/pkg/gui/controllers/helpers/app_status_helper.go index 83500d3a7..fa402962c 100644 --- a/pkg/gui/controllers/helpers/app_status_helper.go +++ b/pkg/gui/controllers/helpers/app_status_helper.go @@ -97,7 +97,6 @@ func (self *AppStatusHelper) renderAppStatus() { prevAppStatus := "" for range ticker.C { appStatus, color := self.statusMgr().GetStatusString(self.c.UserConfig()) - self.c.Views().AppStatus.FgColor = color update := self.c.OnUIThreadContentOnly if utils.StringWidth(appStatus) != utils.StringWidth(prevAppStatus) { @@ -108,6 +107,7 @@ func (self *AppStatusHelper) renderAppStatus() { update = self.c.OnUIThread } update(func() error { + self.c.Views().AppStatus.FgColor = color self.c.SetViewContent(self.c.Views().AppStatus, appStatus) return nil }) From 0d1caf5c22c17e785fb625dd04ac5231f706e89b Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 7 May 2026 08:28:50 +0200 Subject: [PATCH 12/28] Bounce refreshView to the UI thread An async refresh dispatches refreshXyz on a worker goroutine, which then calls refreshView -> PostRefreshUpdate -> HandleRender. Today the final self.c.Render() inside ListContextTrait.HandleRender is what triggers a UI flush from the worker. We're going to remove that Render() call, so prepare by wrapping refreshView's body in OnUIThread. This moves the entire rendering of the view (and the ReApplyFilter/ReApplySearch stuff) to the UI thread, not just the layout. I don't expect this to make a difference in practice, and it is already one step towards my long-term goal of moving all view rendering to the UI thread (see https://github.com/jesseduffield/lazygit/issues/2974#issuecomment-1729154768). --- pkg/gui/controllers/helpers/refresh_helper.go | 33 +++++++++++-------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index 4a61bde18..6638d150c 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -780,22 +780,27 @@ func (self *RefreshHelper) refForLog() string { } func (self *RefreshHelper) refreshView(context types.Context) { - // Re-applying the filter must be done before re-rendering the view, so that - // the filtered list model is up to date for rendering. - self.searchHelper.ReApplyFilter(context) + // refreshView is called from the worker goroutine that drives async + // refreshes, so bounce to the UI thread before mutating view content. + self.c.OnUIThread(func() error { + // Re-applying the filter must be done before re-rendering the view, so that + // the filtered list model is up to date for rendering. + self.searchHelper.ReApplyFilter(context) - self.c.PostRefreshUpdate(context) + self.c.PostRefreshUpdate(context) - self.c.AfterLayout(func() error { - // Re-applying the search must be done after re-rendering the view though, - // so that the "x of y" status is shown correctly. - // - // Also, it must be done after layout, because otherwise FocusPoint - // hasn't been called yet (see ListContextTrait.FocusLine), which means - // that the scroll position might be such that the entire visible - // content is outside the viewport. And this would cause problems in - // searchModelCommits. - self.searchHelper.ReApplySearch(context) + self.c.AfterLayout(func() error { + // Re-applying the search must be done after re-rendering the view though, + // so that the "x of y" status is shown correctly. + // + // Also, it must be done after layout, because otherwise FocusPoint + // hasn't been called yet (see ListContextTrait.FocusLine), which means + // that the scroll position might be such that the entire visible + // content is outside the viewport. And this would cause problems in + // searchModelCommits. + self.searchHelper.ReApplySearch(context) + return nil + }) return nil }) } From a364a8d75ca5e584d5dfd678fb856a1d9c062657 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 7 May 2026 08:30:43 +0200 Subject: [PATCH 13/28] Bounce explicit LocalCommits render in refreshBranches to UI thread refreshBranches runs on a worker goroutine and re-renders the commits view directly to refresh the branch-head visualization. As with refreshView, this currently only flushes because HandleRender ends with self.c.Render(). Wrap the explicit HandleRender call (along with the LocalCommitsMutex pair around it) in OnUIThread so it keeps working once Render() is removed. --- pkg/gui/controllers/helpers/refresh_helper.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index 6638d150c..cb17ffd43 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -532,9 +532,12 @@ func (self *RefreshHelper) refreshBranches(refreshWorktrees bool, keepBranchSele // Need to re-render the commits view because the visualization of local // branch heads might have changed - self.c.Mutexes().LocalCommitsMutex.Lock() - self.c.Contexts().LocalCommits.HandleRender() - self.c.Mutexes().LocalCommitsMutex.Unlock() + self.c.OnUIThread(func() error { + self.c.Mutexes().LocalCommitsMutex.Lock() + self.c.Contexts().LocalCommits.HandleRender() + self.c.Mutexes().LocalCommitsMutex.Unlock() + return nil + }) self.refreshStatus() } From 3d324ed7fbfec2fb15f107c34dc975221db290aa Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 7 May 2026 08:31:03 +0200 Subject: [PATCH 14/28] Bounce SuggestionsContext.SetSuggestions to UI thread SetSuggestions has two callers: prepareConfirmationPanel calls it directly on the UI thread, while editors.promptEditor and SuggestionsContext.RefreshSuggestions call it via AsyncHandler, which runs the result closure on a worker goroutine. The worker path currently relies on HandleRender's self.c.Render() to flush the view update. Wrap the body in OnUIThread so the worker path stays correct when Render() is removed; for the UI-thread caller the extra bounce is harmless. --- pkg/gui/context/suggestions_context.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/pkg/gui/context/suggestions_context.go b/pkg/gui/context/suggestions_context.go index eafe7fb7c..fb69b34d9 100644 --- a/pkg/gui/context/suggestions_context.go +++ b/pkg/gui/context/suggestions_context.go @@ -67,10 +67,17 @@ func NewSuggestionsContext( } func (self *SuggestionsContext) SetSuggestions(suggestions []*types.Suggestion) { - self.State.Suggestions = suggestions - self.SetSelection(0) - self.c.ResetViewOrigin(self.GetView()) - self.HandleRender() + // SetSuggestions is invoked from AsyncHandler (a worker goroutine) when + // the prompt input changes, as well as from prepareConfirmationPanel on + // the UI thread. Bounce to the UI thread either way so the worker path + // keeps flushing once HandleRender stops calling Render() itself. + self.c.OnUIThread(func() error { + self.State.Suggestions = suggestions + self.SetSelection(0) + self.c.ResetViewOrigin(self.GetView()) + self.HandleRender() + return nil + }) } func (self *SuggestionsContext) RefreshSuggestions() { From 225bcaa619c4986fcd879d6aa6dca3b6727863c7 Mon Sep 17 00:00:00 2001 From: Antoine Gaudreau Simard Date: Thu, 7 May 2026 19:52:46 -0400 Subject: [PATCH 15/28] Bounce commit files selection render to UI thread The redraw of the selection color (using ) would be tied to the spinner drawing. To reproduce, having a high spinner refresh rate and toggling a file would see a delay equivalent to the time spinner refresh rate. --- pkg/gui/controllers/commits_files_controller.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/gui/controllers/commits_files_controller.go b/pkg/gui/controllers/commits_files_controller.go index 7cde6c15d..b12819c39 100644 --- a/pkg/gui/controllers/commits_files_controller.go +++ b/pkg/gui/controllers/commits_files_controller.go @@ -476,7 +476,11 @@ func (self *CommitFilesController) toggleForPatch(selectedNodes []*filetree.Comm self.c.Git().Patch.PatchBuilder.Reset() } - self.c.PostRefreshUpdate(self.context()) + self.c.OnUIThread(func() error { + self.c.PostRefreshUpdate(self.context()) + return nil + }) + return nil }) } From 86758622289bafb2780844f0c40acdd734e9ed1a Mon Sep 17 00:00:00 2001 From: Antoine Gaudreau Simard Date: Thu, 7 May 2026 20:55:00 -0400 Subject: [PATCH 16/28] Bounce setGithubPullRequests' Branches render to UI thread Fixes an issue mostly noticeable using the go -race mode, this resolves them. --- pkg/gui/controllers/helpers/refresh_helper.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index cb17ffd43..77de9ca4a 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -949,7 +949,11 @@ func (self *RefreshHelper) setGithubPullRequests(authToken string, baseRemote *m self.savePullRequestsToCache(prs) self.rebuildPullRequestsMap() - self.c.PostRefreshUpdate(self.c.Contexts().Branches) + self.c.OnUIThread(func() error { + self.c.PostRefreshUpdate(self.c.Contexts().Branches) + return nil + }) + return nil } From 76211eea68fd45c78180108198688cf76d6ffa0b Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 7 May 2026 08:43:10 +0200 Subject: [PATCH 17/28] Remove Render() from ListContextTrait.HandleRender self.c.Render() at the end of HandleRender was there to schedule a gocui Update tick so the view content modified above would actually get drawn. For UI-thread callers (the great majority -- keybinding handlers, the layout function itself, popup resize, etc.) this was unnecessary work, since gocui already runs a layout/redraw cycle after every event. SimpleContext.HandleRender doesn't call Render() either, so this aligns the two implementations. The few callers that drove HandleRender from a worker goroutine and relied on Render() for the flush were wrapped in OnUIThread in the preceding commits, so the implicit Render is no longer needed. Also, Render() being called *before* setFooter() looks like it might have been a theoretical race; this is no longer an issue now. --- pkg/gui/context/list_context_trait.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/gui/context/list_context_trait.go b/pkg/gui/context/list_context_trait.go index 98833fdb2..597fc99df 100644 --- a/pkg/gui/context/list_context_trait.go +++ b/pkg/gui/context/list_context_trait.go @@ -124,7 +124,6 @@ func (self *ListContextTrait) HandleRender() { content := self.renderLines(-1, -1) self.GetViewTrait().SetContent(content) } - self.c.Render() self.setFooter() } From 4734bab896ad8bfe9791bb0ed32a00495c179200 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 7 May 2026 19:28:20 +0200 Subject: [PATCH 18/28] Improve performance of inline status spinner Now that HandleRender no longer does an implicit Render(), we can use it inside OnUIThreadContentOnly to save performance, on the assumption that rerendering a view that contains an inline spinner never changes the layout. --- pkg/gui/controllers/helpers/inline_status_helper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/gui/controllers/helpers/inline_status_helper.go b/pkg/gui/controllers/helpers/inline_status_helper.go index 13c112811..02afcdd50 100644 --- a/pkg/gui/controllers/helpers/inline_status_helper.go +++ b/pkg/gui/controllers/helpers/inline_status_helper.go @@ -149,7 +149,7 @@ func (self *InlineStatusHelper) stop(opts InlineStatusOpts) { } func (self *InlineStatusHelper) renderContext(contextKey types.ContextKey) { - self.c.OnUIThread(func() error { + self.c.OnUIThreadContentOnly(func() error { self.c.ContextForKey(contextKey).HandleRender() return nil }) From 1cab15eba863f9619c5a98c10fe74d9fdfed4e6d Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 4 May 2026 20:54:22 +0200 Subject: [PATCH 19/28] Some additions to AGENTS.md --- AGENTS.md | 31 +++++++++++++++++++++++++++++++ CLAUDE.md | 1 + 2 files changed, 32 insertions(+) create mode 100644 CLAUDE.md diff --git a/AGENTS.md b/AGENTS.md index 399a86718..476d6e03a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -27,6 +27,8 @@ while still being meaningful and self-contained. - **Every commit must compile and pass all tests.** No "WIP" commits, no commits that leave the tree broken and rely on a follow-up to fix it. +- **Every commit must be `gofumpt`-formatted.** Run `make format` before + committing. - **Commit messages explain _why_, not _what_.** The diff already shows what changed; the message should capture the motivation, the constraint, or the bug being fixed. If the reason is obvious from a one-line subject, no body @@ -38,6 +40,28 @@ while still being meaningful and self-contained. - **Do not use conventional commits** (no `feat:`/`fix:`/`chore:` prefixes). Match the plain English imperative style of the existing history. +## Iterate with `fixup!` commits + +When refining work that's already committed — adjusting an approach, +incorporating an idea from elsewhere, fixing something that belongs to the +same logical unit — create a fixup against the target commit +(`git commit --fixup=`) so the history collapses cleanly under +`git rebase --autosquash`. Don't pile follow-up commits on top with the +intent of squashing them later. + +If the changes don't map cleanly onto existing commits — say they cut +across several of them, or restructure something at a different layer +than any existing commit naturally owns — stop and ask the user how to +proceed. Resetting the branch and redoing the work is sometimes the right +call, but it's the user's call to make. + +After writing a fixup, re-read the target commit's message. If anything in +that message has become inaccurate or misleading because of the fixup, use +an `amend!` commit instead (its subject is `amend! ` and +its body becomes the target's new full message after autosquash). A plain +`fixup!` keeps the original message verbatim, so message drift stays in +unless you explicitly correct it. + ## Prefer the cleaner design over the smaller diff When a task could be implemented either by tacking onto existing code or by @@ -88,3 +112,10 @@ Use this pattern only where it makes sense; don't apply it by default. Prefer `assert.Equal` (and friends) over hand-rolled `if` checks. The failure messages are more useful and the intent is clearer at a glance. + +## Don't search outside the working tree + +Never run `find` (or similar) from `/` or other paths outside the project. All +third-party code we use is vendored under `vendor/`, so dependency sources are +reachable from inside the working tree — search there instead of the host +filesystem. diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 000000000..3f1ed7b4c --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1 @@ +Before doing anything else, read AGENTS.md and follow it. From f264d43a1a2fa8eb10b629c4d47446dfb9f32784 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 9 May 2026 08:07:48 +0200 Subject: [PATCH 20/28] Add test for missing commits when range-copying after a paste After a successful paste, the "X commits copied" indicator hides and DidPaste is set, but the buffer is not cleared. If the user then range-selects multiple commits and presses shift+C, every Add() call rebuilds the set via SelectedHashSet(), which returns empty while DidPaste is true; each iteration of the copy loop therefore overwrites the previous one and only the last commit in the range survives. --- .../cherry_pick_range_after_paste.go | 119 ++++++++++++++++++ pkg/integration/tests/test_list.go | 1 + 2 files changed, 120 insertions(+) create mode 100644 pkg/integration/tests/cherry_pick/cherry_pick_range_after_paste.go diff --git a/pkg/integration/tests/cherry_pick/cherry_pick_range_after_paste.go b/pkg/integration/tests/cherry_pick/cherry_pick_range_after_paste.go new file mode 100644 index 000000000..9441c0596 --- /dev/null +++ b/pkg/integration/tests/cherry_pick/cherry_pick_range_after_paste.go @@ -0,0 +1,119 @@ +package cherry_pick + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var CherryPickRangeAfterPaste = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Regression test: range-copy multiple commits after a previous paste", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Git.LocalBranchSortOrder = "recency" + }, + SetupRepo: func(shell *Shell) { + shell. + EmptyCommit("base"). + NewBranch("target"). + NewBranch("source"). + EmptyCommit("one"). + EmptyCommit("two"). + EmptyCommit("three"). + EmptyCommit("four"). + EmptyCommit("five"). + Checkout("target") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Branches(). + Focus(). + Lines( + Contains("target").IsSelected(), + Contains("source"), + Contains("master"), + ). + SelectNextItem(). + PressEnter() + + t.Views().SubCommits(). + IsFocused(). + Lines( + Contains("five").IsSelected(), + Contains("four"), + Contains("three"), + Contains("two"), + Contains("one"), + Contains("base"), + ). + Press(keys.Commits.CherryPickCopy) + + t.Views().Commits(). + Focus(). + Lines( + Contains("base").IsSelected(), + ). + Press(keys.Commits.PasteCommits). + Tap(func() { + t.ExpectPopup().Alert(). + Title(Equals("Cherry-pick")). + Content(Equals("Are you sure you want to cherry-pick the 1 copied commit(s) onto this branch?")). + Confirm() + }). + Lines( + Contains("five"), + Contains("base").IsSelected(), + ). + Tap(func() { + // After paste, CherryPicking.DidPaste is true, so it looks to the user as if no + // commits are copied: + t.Views().Information().Content(DoesNotContain("commits copied")) + }) + + t.Views().Branches(). + Focus(). + NavigateToLine(Contains("source")). + PressEnter() + + t.Views().SubCommits(). + IsFocused(). + NavigateToLine(Contains("four")). + Press(keys.Universal.RangeSelectDown). + Press(keys.Universal.RangeSelectDown). + Press(keys.Commits.CherryPickCopy). + Tap(func() { + /* EXPECTED: + t.Views().Information().Content(Contains("3 commits copied")) + ACTUAL: */ + t.Views().Information().Content(Contains("1 commit copied")) + }) + + t.Views().Commits(). + Focus(). + NavigateToLine(Contains("base")). + Press(keys.Commits.PasteCommits). + Tap(func() { + t.ExpectPopup().Alert(). + Title(Equals("Cherry-pick")). + /* EXPECTED: + Content(Equals("Are you sure you want to cherry-pick the 3 copied commit(s) onto this branch?")). + ACTUAL: */ + Content(Equals("Are you sure you want to cherry-pick the 1 copied commit(s) onto this branch?")). + Confirm() + }) + + /* EXPECTED: + t.Views().Commits().Lines( + Contains("four"), + Contains("three"), + Contains("two"), + Contains("five"), + Contains("base").IsSelected(), + ) + ACTUAL: */ + t.Views().Commits().Lines( + Contains("two"), + Contains("five"), + Contains("base").IsSelected(), + ) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 09d487852..4b96c4d42 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -97,6 +97,7 @@ var tests = []*components.IntegrationTest{ cherry_pick.CherryPickDuringRebase, cherry_pick.CherryPickMerge, cherry_pick.CherryPickRange, + cherry_pick.CherryPickRangeAfterPaste, commit.AddCoAuthor, commit.AddCoAuthorRange, commit.AddCoAuthorWhileCommitting, From 7a2d1ab544c702e1ca51be25730ad8a30bb6a19c Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 9 May 2026 08:10:06 +0200 Subject: [PATCH 21/28] Clear cherry-pick buffer when copying after a paste After a successful paste DidPaste is true, hiding the "X commits copied" indicator but leaving the buffer populated. From the user's perspective this looks like a clean slate, so a new shift+C should start fresh. It is important to reset DidPaste first, before populating the buffer with the new commits, because otherwise each loop iteration would overwrite the previous one since Add() rebuilds the set via SelectedHashSet() which returns empty while DidPaste is set. --- pkg/gui/controllers/helpers/cherry_pick_helper.go | 10 ++++++++-- .../cherry_pick/cherry_pick_range_after_paste.go | 13 ------------- 2 files changed, 8 insertions(+), 15 deletions(-) diff --git a/pkg/gui/controllers/helpers/cherry_pick_helper.go b/pkg/gui/controllers/helpers/cherry_pick_helper.go index 359d1cbc2..079fdedcf 100644 --- a/pkg/gui/controllers/helpers/cherry_pick_helper.go +++ b/pkg/gui/controllers/helpers/cherry_pick_helper.go @@ -40,6 +40,14 @@ func (self *CherryPickHelper) CopyRange(commitsList []*models.Commit, context ty return err } + // After a paste the buffer is hidden but not cleared, so the user + // thinks they're starting fresh. Clear it before adding so the new + // copy replaces the old one. + if self.getData().DidPaste { + self.getData().CherryPickedCommits = nil + self.getData().DidPaste = false + } + commitSet := self.getData().SelectedHashSet() allCommitsCopied := lo.EveryBy(commitsList[startIdx:endIdx+1], func(commit *models.Commit) bool { @@ -59,8 +67,6 @@ func (self *CherryPickHelper) CopyRange(commitsList []*models.Commit, context ty } } - self.getData().DidPaste = false - self.rerender() return nil } diff --git a/pkg/integration/tests/cherry_pick/cherry_pick_range_after_paste.go b/pkg/integration/tests/cherry_pick/cherry_pick_range_after_paste.go index 9441c0596..4f1cf180a 100644 --- a/pkg/integration/tests/cherry_pick/cherry_pick_range_after_paste.go +++ b/pkg/integration/tests/cherry_pick/cherry_pick_range_after_paste.go @@ -81,10 +81,7 @@ var CherryPickRangeAfterPaste = NewIntegrationTest(NewIntegrationTestArgs{ Press(keys.Universal.RangeSelectDown). Press(keys.Commits.CherryPickCopy). Tap(func() { - /* EXPECTED: t.Views().Information().Content(Contains("3 commits copied")) - ACTUAL: */ - t.Views().Information().Content(Contains("1 commit copied")) }) t.Views().Commits(). @@ -94,14 +91,10 @@ var CherryPickRangeAfterPaste = NewIntegrationTest(NewIntegrationTestArgs{ Tap(func() { t.ExpectPopup().Alert(). Title(Equals("Cherry-pick")). - /* EXPECTED: Content(Equals("Are you sure you want to cherry-pick the 3 copied commit(s) onto this branch?")). - ACTUAL: */ - Content(Equals("Are you sure you want to cherry-pick the 1 copied commit(s) onto this branch?")). Confirm() }) - /* EXPECTED: t.Views().Commits().Lines( Contains("four"), Contains("three"), @@ -109,11 +102,5 @@ var CherryPickRangeAfterPaste = NewIntegrationTest(NewIntegrationTestArgs{ Contains("five"), Contains("base").IsSelected(), ) - ACTUAL: */ - t.Views().Commits().Lines( - Contains("two"), - Contains("five"), - Contains("base").IsSelected(), - ) }, }) From 21d58085f4e5d60a18d161da81fb0d9298dbe4ed Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 7 May 2026 22:49:13 +0200 Subject: [PATCH 22/28] Expose service info (provider, host, owner, repo) from the hosting service The github pull-request fetcher needs to know whether a given remote is a github-type service, which host its API lives on, and which owner/repo to query against. Today the fetcher hardcodes the first two ("does the URL contain github.com" and "https://api.github.com/graphql") and re-derives owner/repo from the remote, which precludes GitHub Enterprise and makes the fetch entry point take more arguments than it needs. Add an accessor on the hosting service manager that exposes the already- resolved service domain together with the parsed owner/repo, so callers can answer all of these questions without reaching into the manager's internals. Co-Authored-By: Claude Opus 4.7 (1M context) --- pkg/commands/git_commands/hosting_service.go | 4 + .../hosting_service/hosting_service.go | 36 ++++++ .../hosting_service/hosting_service_test.go | 104 ++++++++++++++++++ 3 files changed, 144 insertions(+) diff --git a/pkg/commands/git_commands/hosting_service.go b/pkg/commands/git_commands/hosting_service.go index 7d9772127..5deceea8e 100644 --- a/pkg/commands/git_commands/hosting_service.go +++ b/pkg/commands/git_commands/hosting_service.go @@ -25,6 +25,10 @@ func (self *HostingService) GetRepoNameFromRemoteURL(remoteURL string) (string, return self.getHostingServiceMgr(remoteURL).GetRepoName() } +func (self *HostingService) GetServiceInfo(remoteURL string) (hosting_service.ServiceInfo, error) { + return self.getHostingServiceMgr(remoteURL).GetServiceInfo() +} + // getting this on every request rather than storing it in state in case our remoteURL changes // from one invocation to the next. Note however that we're currently caching config // results so we might want to invalidate the cache here if it becomes a problem. diff --git a/pkg/commands/hosting_service/hosting_service.go b/pkg/commands/hosting_service/hosting_service.go index 620d0d0a7..ad1f072c5 100644 --- a/pkg/commands/hosting_service/hosting_service.go +++ b/pkg/commands/hosting_service/hosting_service.go @@ -73,6 +73,42 @@ func (self *HostingServiceMgr) GetRepoName() (string, error) { return repoName, nil } +// ServiceInfo holds the resolved hosting service for a remote URL. Owner +// comes from the "owner" named regex capture, which only exists for +// owner/repo-shaped providers (github, gitlab, bitbucket, gitea, codeberg); +// it's empty for azuredevops and bitbucketServer, whose URLs are organised +// differently. Repository is populated for every provider, but RepoName may +// have more than two segments (e.g. "org/project/repo" for azuredevops). +type ServiceInfo struct { + Provider string // e.g. "github" + WebDomain string // e.g. "github.com", or "git.acme.com" for an on-prem instance + Owner string // e.g. "jesseduffield" + Repository string // e.g. "lazygit" + RepoName string // e.g. "jesseduffield/lazygit" +} + +// GetServiceInfo identifies which hosting service the configured remote URL +// belongs to and returns enough information to talk to its web/API host. +func (self *HostingServiceMgr) GetServiceInfo() (ServiceInfo, error) { + serviceDomain, err := self.getServiceDomain(self.remoteURL) + if err != nil { + return ServiceInfo{}, err + } + + matches, err := serviceDomain.serviceDefinition.parseRemoteUrl(self.remoteURL) + if err != nil { + return ServiceInfo{}, err + } + + return ServiceInfo{ + Provider: serviceDomain.serviceDefinition.provider, + WebDomain: serviceDomain.webDomain, + Owner: matches["owner"], + Repository: matches["repo"], + RepoName: utils.ResolvePlaceholderString(serviceDomain.serviceDefinition.repoNameTemplate, matches), + }, nil +} + func (self *HostingServiceMgr) getService() (*Service, error) { serviceDomain, err := self.getServiceDomain(self.remoteURL) if err != nil { diff --git a/pkg/commands/hosting_service/hosting_service_test.go b/pkg/commands/hosting_service/hosting_service_test.go index c2fabcd0d..f150f22eb 100644 --- a/pkg/commands/hosting_service/hosting_service_test.go +++ b/pkg/commands/hosting_service/hosting_service_test.go @@ -577,3 +577,107 @@ func TestGetPullRequestURL(t *testing.T) { }) } } + +func TestGetServiceInfo(t *testing.T) { + scenarios := []struct { + name string + remoteURL string + configServiceDomains map[string]string + expected ServiceInfo + }{ + { + name: "github.com SSH", + remoteURL: "git@github.com:jesseduffield/lazygit.git", + expected: ServiceInfo{ + Provider: "github", + WebDomain: "github.com", + Owner: "jesseduffield", + Repository: "lazygit", + RepoName: "jesseduffield/lazygit", + }, + }, + { + name: "github enterprise with same git and web host", + remoteURL: "git@github.example.com:my-org/my-repo.git", + configServiceDomains: map[string]string{ + "github.example.com": "github:github.example.com", + }, + expected: ServiceInfo{ + Provider: "github", + WebDomain: "github.example.com", + Owner: "my-org", + Repository: "my-repo", + RepoName: "my-org/my-repo", + }, + }, + { + name: "github enterprise with distinct git and web hosts", + remoteURL: "git@git.example.com:my-org/my-repo.git", + configServiceDomains: map[string]string{ + "git.example.com": "github:ghe.example.com", + }, + expected: ServiceInfo{ + Provider: "github", + WebDomain: "ghe.example.com", + Owner: "my-org", + Repository: "my-repo", + RepoName: "my-org/my-repo", + }, + }, + { + name: "github enterprise with web host port", + remoteURL: "git@git.example.com:my-org/my-repo.git", + configServiceDomains: map[string]string{ + "git.example.com": "github:ghe.example.com:8443", + }, + expected: ServiceInfo{ + Provider: "github", + WebDomain: "ghe.example.com:8443", + Owner: "my-org", + Repository: "my-repo", + RepoName: "my-org/my-repo", + }, + }, + { + // azuredevops uses org/project/repo named captures rather than + // owner/repo, so Owner is unpopulated and RepoName has three + // segments rather than the usual two. + name: "azuredevops", + remoteURL: "https://myorg@dev.azure.com/myorg/myproject/_git/myrepo", + expected: ServiceInfo{ + Provider: "azuredevops", + WebDomain: "dev.azure.com", + Repository: "myrepo", + RepoName: "myorg/myproject/myrepo", + }, + }, + { + // bitbucketServer uses project/repo named captures, so Owner is + // unpopulated and RepoName is project/repo rather than owner/repo. + name: "bitbucketServer", + remoteURL: "https://mycompany.bitbucket.com/scm/myproject/myrepo.git", + configServiceDomains: map[string]string{ + "mycompany.bitbucket.com": "bitbucketServer:mycompany.bitbucket.com", + }, + expected: ServiceInfo{ + Provider: "bitbucketServer", + WebDomain: "mycompany.bitbucket.com", + Repository: "myrepo", + RepoName: "myproject/myrepo", + }, + }, + } + + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + tr := i18n.EnglishTranslationSet() + log := &fakes.FakeFieldLogger{} + mgr := NewHostingServiceMgr(log, tr, s.remoteURL, s.configServiceDomains) + + info, err := mgr.GetServiceInfo() + + assert.NoError(t, err) + assert.Equal(t, s.expected, info) + }) + } +} From 1c79fe24d0bf1a4b1727dd7a782288f9d8b07666 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 8 May 2026 06:59:10 +0200 Subject: [PATCH 23/28] Support GitHub Enterprise for the pull-requests feature MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The branches-panel PR icons only worked for github.com remotes. There was no fundamental reason — the auth library we already vendor (cli/go-gh) supports enterprise tokens out of the box (GH_ENTERPRISE_TOKEN, gh auth's keyring), and the user-facing 'services' config has long been the documented way to tell lazygit "this domain is a github service" for the View-PR-URL feature. The fetcher just hardcoded github.com in three places: - a substring check on the remote URL to decide we're "in a github repo", - the GraphQL endpoint (always api.github.com/graphql), and - the auth lookup (always against the default host). Plumb the resolved web domain through instead. Detection now goes through the hosting_service ("is this remote's provider 'github'?"), which means a user with services: { 'git.acme.com': 'github:git.acme.com' } configured gets PR icons on their GHE remotes too. Replacing the substring check with a provider check also tightens a latent bug in getGithubRemotes: it previously accepted any remote whose URL parsed with the default regex, including gitlab and bitbucket — masked today only by the InGithubRepo gate, but exposed once the gate goes away. Co-Authored-By: Claude Opus 4.7 (1M context) --- pkg/commands/git_commands/github.go | 35 +++++---- pkg/commands/git_commands/github_test.go | 19 +++++ pkg/gui/controllers/helpers/refresh_helper.go | 76 +++++++++++-------- .../helpers/refresh_helper_test.go | 55 +++++++++++++- 4 files changed, 139 insertions(+), 46 deletions(-) diff --git a/pkg/commands/git_commands/github.go b/pkg/commands/git_commands/github.go index 85893615d..02bbebdc2 100644 --- a/pkg/commands/git_commands/github.go +++ b/pkg/commands/git_commands/github.go @@ -138,19 +138,16 @@ func fetchPullRequestsQuery(branches []string, owner string, repo string) (strin return queryString, variables } -func (self *GitHubCommands) GetAuthToken() string { - defaultHost, _ := auth.DefaultHost() - token, _ := auth.TokenForHost(defaultHost) +func (self *GitHubCommands) GetAuthToken(host string) string { + token, _ := auth.TokenForHost(host) return token } -// FetchRecentPRs fetches recent pull requests using GraphQL. -func (self *GitHubCommands) FetchRecentPRs(branches []string, baseRemote *models.Remote, token string) ([]*models.GithubPullRequest, error) { - repoOwner, repoName, err := self.GetBaseRepoOwnerAndName(baseRemote) - if err != nil { - return nil, err - } - +// FetchRecentPRs fetches recent pull requests using GraphQL. serviceInfo +// identifies the GitHub instance (github.com or a GitHub Enterprise Server) +// and the owner/repo to query against. +func (self *GitHubCommands) FetchRecentPRs(branches []string, serviceInfo *hosting_service.ServiceInfo, token string) ([]*models.GithubPullRequest, error) { + endpoint := graphQLEndpoint(serviceInfo.WebDomain) t := time.Now() var g errgroup.Group @@ -171,7 +168,7 @@ func (self *GitHubCommands) FetchRecentPRs(branches []string, baseRemote *models // Launch a goroutine for each chunk of branches g.Go(func() error { - prs, err := self.fetchRecentPRsAux(repoOwner, repoName, branchChunk, token) + prs, err := self.fetchRecentPRsAux(endpoint, serviceInfo.Owner, serviceInfo.Repository, branchChunk, token) if err != nil { return err } @@ -181,7 +178,7 @@ func (self *GitHubCommands) FetchRecentPRs(branches []string, baseRemote *models } // Wait for all goroutines, then close the channel so the range loop exits - err = g.Wait() + err := g.Wait() close(results) if err != nil { return nil, err @@ -198,14 +195,14 @@ func (self *GitHubCommands) FetchRecentPRs(branches []string, baseRemote *models return allPRs, nil } -func (self *GitHubCommands) fetchRecentPRsAux(repoOwner string, repoName string, branches []string, token string) ([]*models.GithubPullRequest, error) { +func (self *GitHubCommands) fetchRecentPRsAux(endpoint string, repoOwner string, repoName string, branches []string, token string) ([]*models.GithubPullRequest, error) { queryString, variables := fetchPullRequestsQuery(branches, repoOwner, repoName) bodyBytes, err := json.Marshal(graphQLRequest{Query: queryString, Variables: variables}) if err != nil { return nil, err } - req, err := http.NewRequest("POST", "https://api.github.com/graphql", bytes.NewBuffer(bodyBytes)) + req, err := http.NewRequest("POST", endpoint, bytes.NewBuffer(bodyBytes)) if err != nil { return nil, err } @@ -336,6 +333,16 @@ func getRemotesToOwnersMap(remotes []*models.Remote) map[string]string { return res } +// graphQLEndpoint returns the GraphQL API URL for a GitHub host. github.com +// uses a dedicated api. subdomain; GitHub Enterprise Server hangs the API off +// the web host under /api/graphql. +func graphQLEndpoint(host string) string { + if auth.NormalizeHostname(host) == "github.com" { + return "https://api.github.com/graphql" + } + return "https://" + host + "/api/graphql" +} + func (self *GitHubCommands) InGithubRepo(remotes []*models.Remote) bool { if len(remotes) == 0 { return false diff --git a/pkg/commands/git_commands/github_test.go b/pkg/commands/git_commands/github_test.go index d9d55ffd1..b332ba12a 100644 --- a/pkg/commands/git_commands/github_test.go +++ b/pkg/commands/git_commands/github_test.go @@ -57,6 +57,25 @@ func TestGetRepoInfoFromURL(t *testing.T) { } } +func TestGraphQLEndpoint(t *testing.T) { + cases := []struct { + host string + expected string + }{ + {"github.com", "https://api.github.com/graphql"}, + {"www.github.com", "https://api.github.com/graphql"}, + {"GITHUB.com", "https://api.github.com/graphql"}, + {"ghe.example.com", "https://ghe.example.com/api/graphql"}, + {"ghe.example.com:8443", "https://ghe.example.com:8443/api/graphql"}, + } + + for _, c := range cases { + t.Run(c.host, func(t *testing.T) { + assert.Equal(t, c.expected, graphQLEndpoint(c.host)) + }) + } +} + func TestGenerateGithubPullRequestMap(t *testing.T) { cases := []struct { name string diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index 77de9ca4a..6c554deff 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -8,6 +8,7 @@ import ( "github.com/jesseduffield/generics/set" "github.com/jesseduffield/lazygit/pkg/commands/git_commands" + "github.com/jesseduffield/lazygit/pkg/commands/hosting_service" "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/config" "github.com/jesseduffield/lazygit/pkg/gocui" @@ -812,39 +813,33 @@ func (self *RefreshHelper) refreshGithubPullRequests() { self.c.Mutexes().RefreshingPullRequestsMutex.Lock() defer self.c.Mutexes().RefreshingPullRequestsMutex.Unlock() - if !self.c.Git().GitHub.InGithubRepo(self.c.Model().Remotes) { + githubRemotes := getAuthenticatedGithubRemotes(self.getGithubRemotes(), self.c.Git().GitHub.GetAuthToken) + if len(githubRemotes) == 0 { self.c.Model().PullRequests = nil self.c.Model().PullRequestsMap = nil return } - authToken := self.c.Git().GitHub.GetAuthToken() - if authToken == "" { - self.c.Model().PullRequests = nil - self.c.Model().PullRequestsMap = nil - return - } - - githubRemotes := self.getGithubRemotes() - baseRemote := getGithubBaseRemote(githubRemotes, self.c.Git().GitHub.ConfiguredBaseRemoteName()) - if baseRemote == nil { + baseInfo := getGithubBaseRemote(githubRemotes, self.c.Git().GitHub.ConfiguredBaseRemoteName()) + if baseInfo == nil { self.c.Model().PullRequests = nil self.c.Model().PullRequestsMap = nil - if len(githubRemotes) > 0 && !self.githubBaseRemotePromptDismissed[self.c.Git().RepoPaths.RepoPath()] { - self.promptForBaseGithubRepo(authToken, githubRemotes) + if !self.githubBaseRemotePromptDismissed[self.c.Git().RepoPaths.RepoPath()] { + self.promptForBaseGithubRepo(githubRemotes) } return } - if err := self.setGithubPullRequests(authToken, baseRemote); err != nil { + if err := self.setGithubPullRequests(baseInfo); err != nil { self.c.LogAction(fmt.Sprintf("Error fetching pull requests from GitHub: %s", err.Error())) } } type githubRemoteInfo struct { - remote *models.Remote - repoName string + remote *models.Remote + serviceInfo hosting_service.ServiceInfo + authToken string } func (self *RefreshHelper) getGithubRemotes() []githubRemoteInfo { @@ -852,23 +847,44 @@ func (self *RefreshHelper) getGithubRemotes() []githubRemoteInfo { if len(remote.Urls) == 0 { return githubRemoteInfo{}, false } - repoName, err := self.c.Git().HostingService.GetRepoNameFromRemoteURL(remote.Urls[0]) - if err != nil { + serviceInfo, err := self.c.Git().HostingService.GetServiceInfo(remote.Urls[0]) + if err != nil || serviceInfo.Provider != "github" { return githubRemoteInfo{}, false } - return githubRemoteInfo{remote: remote, repoName: repoName}, true + return githubRemoteInfo{remote: remote, serviceInfo: serviceInfo}, true }) } -func getGithubBaseRemote(githubRemotes []githubRemoteInfo, configuredRemoteName string) *models.Remote { - findRemoteByName := func(name string) *models.Remote { +// getAuthenticatedGithubRemotes drops remotes for which no auth token is +// available and attaches the resolved token to the rest. Token lookups are +// cached by host so that multiple remotes pointing at the same instance +// (e.g. origin + a fork on github.com) only trigger one lookup. +func getAuthenticatedGithubRemotes(githubRemotes []githubRemoteInfo, getAuthToken func(host string) string) []githubRemoteInfo { + tokensByHost := map[string]string{} + return lo.FilterMap(githubRemotes, func(info githubRemoteInfo, _ int) (githubRemoteInfo, bool) { + host := info.serviceInfo.WebDomain + token, cached := tokensByHost[host] + if !cached { + token = getAuthToken(host) + tokensByHost[host] = token + } + if token == "" { + return githubRemoteInfo{}, false + } + info.authToken = token + return info, true + }) +} + +func getGithubBaseRemote(githubRemotes []githubRemoteInfo, configuredRemoteName string) *githubRemoteInfo { + findRemoteByName := func(name string) *githubRemoteInfo { info, ok := lo.Find(githubRemotes, func(info githubRemoteInfo) bool { return info.remote.Name == name }) if !ok { return nil } - return info.remote + return &info } if configuredRemoteName != "" { @@ -876,29 +892,29 @@ func getGithubBaseRemote(githubRemotes []githubRemoteInfo, configuredRemoteName } if len(githubRemotes) == 1 { - return githubRemotes[0].remote + return &githubRemotes[0] } // Not sure if "upstream" is really a common convention for the name of the remote that PRs are // made against, but if it exists it's pretty likely to be the one we want. - if remote := findRemoteByName("upstream"); remote != nil { - return remote + if info := findRemoteByName("upstream"); info != nil { + return info } return nil } -func (self *RefreshHelper) promptForBaseGithubRepo(authToken string, githubRemotes []githubRemoteInfo) { +func (self *RefreshHelper) promptForBaseGithubRepo(githubRemotes []githubRemoteInfo) { menuItems := lo.Map(githubRemotes, func(info githubRemoteInfo, _ int) *types.MenuItem { return &types.MenuItem{ - LabelColumns: []string{info.remote.Name, style.FgCyan.Sprint(info.repoName)}, + LabelColumns: []string{info.remote.Name, style.FgCyan.Sprint(info.serviceInfo.RepoName)}, OnPress: func() error { return self.c.WithWaitingStatus(self.c.Tr.FetchingPullRequests, func(gocui.Task) error { if err := self.c.Git().GitHub.SetConfiguredBaseRemoteName(info.remote.Name); err != nil { self.c.Log.Error(err) } - if err := self.setGithubPullRequests(authToken, info.remote); err != nil { + if err := self.setGithubPullRequests(&info); err != nil { self.c.LogAction(fmt.Sprintf("Error fetching pull requests from GitHub: %s", err.Error())) } return nil @@ -928,7 +944,7 @@ func (self *RefreshHelper) rebuildPullRequestsMap() { ) } -func (self *RefreshHelper) setGithubPullRequests(authToken string, baseRemote *models.Remote) error { +func (self *RefreshHelper) setGithubPullRequests(baseInfo *githubRemoteInfo) error { if len(self.c.Model().Branches) == 0 { return nil } @@ -940,7 +956,7 @@ func (self *RefreshHelper) setGithubPullRequests(authToken string, baseRemote *m return branch.UpstreamBranch }) - prs, err := self.c.Git().GitHub.FetchRecentPRs(branchNames, baseRemote, authToken) + prs, err := self.c.Git().GitHub.FetchRecentPRs(branchNames, &baseInfo.serviceInfo, baseInfo.authToken) if err != nil { return err } diff --git a/pkg/gui/controllers/helpers/refresh_helper_test.go b/pkg/gui/controllers/helpers/refresh_helper_test.go index dea3b8b81..cebd044c4 100644 --- a/pkg/gui/controllers/helpers/refresh_helper_test.go +++ b/pkg/gui/controllers/helpers/refresh_helper_test.go @@ -3,6 +3,7 @@ package helpers import ( "testing" + "github.com/jesseduffield/lazygit/pkg/commands/hosting_service" "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/samber/lo" "github.com/stretchr/testify/assert" @@ -60,14 +61,64 @@ func TestGetGithubBaseRemote(t *testing.T) { assert.Nil(t, result) } else { assert.NotNil(t, result) - assert.Equal(t, c.expected, result.Name) + assert.Equal(t, c.expected, result.remote.Name) } }) } } +func TestGetAuthenticatedGithubRemotes(t *testing.T) { + githubRemotes := []githubRemoteInfo{ + makeGithubRemoteInfo("origin", "github.com"), + makeGithubRemoteInfo("fork", "github.com"), + makeGithubRemoteInfo("enterprise", "ghe.example.com"), + makeGithubRemoteInfo("missing-auth", "no-token.example.com"), + } + + callsByHost := map[string]int{} + result := getAuthenticatedGithubRemotes(githubRemotes, func(host string) string { + callsByHost[host]++ + switch host { + case "github.com": + return "github-token" + case "ghe.example.com": + return "ghe-token" + default: + return "" + } + }) + + assert.Equal(t, []githubRemoteInfo{ + makeAuthenticatedGithubRemoteInfo("origin", "github.com", "github-token"), + makeAuthenticatedGithubRemoteInfo("fork", "github.com", "github-token"), + makeAuthenticatedGithubRemoteInfo("enterprise", "ghe.example.com", "ghe-token"), + }, result) + // Two remotes share github.com; the lookup runs only once. + assert.Equal(t, map[string]int{ + "github.com": 1, + "ghe.example.com": 1, + "no-token.example.com": 1, + }, callsByHost) +} + func makeGithubRemoteInfoList(names ...string) []githubRemoteInfo { return lo.Map(names, func(name string, _ int) githubRemoteInfo { - return githubRemoteInfo{remote: &models.Remote{Name: name}, repoName: name} + return makeGithubRemoteInfo(name, name) }) } + +func makeGithubRemoteInfo(name string, webDomain string) githubRemoteInfo { + return githubRemoteInfo{ + remote: &models.Remote{Name: name}, + serviceInfo: hosting_service.ServiceInfo{ + RepoName: name, + WebDomain: webDomain, + }, + } +} + +func makeAuthenticatedGithubRemoteInfo(name string, webDomain string, authToken string) githubRemoteInfo { + info := makeGithubRemoteInfo(name, webDomain) + info.authToken = authToken + return info +} From d955ba8fb819b4aec4a4be8e2d8f05f2a7970df5 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 8 May 2026 10:28:54 +0200 Subject: [PATCH 24/28] Remove now unused code Doing this in a separate commit makes the previous commit's diff easier to read. --- pkg/commands/git_commands/github.go | 43 -------------------- pkg/commands/git_commands/hosting_service.go | 4 -- 2 files changed, 47 deletions(-) diff --git a/pkg/commands/git_commands/github.go b/pkg/commands/git_commands/github.go index 02bbebdc2..e05472ef1 100644 --- a/pkg/commands/git_commands/github.go +++ b/pkg/commands/git_commands/github.go @@ -342,46 +342,3 @@ func graphQLEndpoint(host string) string { } return "https://" + host + "/api/graphql" } - -func (self *GitHubCommands) InGithubRepo(remotes []*models.Remote) bool { - if len(remotes) == 0 { - return false - } - - remote := getMainRemote(remotes) - - if len(remote.Urls) == 0 { - return false - } - - url := remote.Urls[0] - return strings.Contains(strings.ToLower(url), "github.com") -} - -func getMainRemote(remotes []*models.Remote) *models.Remote { - for _, remote := range remotes { - if remote.Name == "origin" { - return remote - } - } - - // need to sort remotes by name so that this is deterministic - return lo.MinBy(remotes, func(a, b *models.Remote) bool { - return a.Name < b.Name - }) -} - -func (self *GitHubCommands) GetBaseRepoOwnerAndName(baseRemote *models.Remote) (string, string, error) { - if len(baseRemote.Urls) == 0 { - return "", "", fmt.Errorf("No URLs found for remote") - } - - url := baseRemote.Urls[0] - - repoInfo, err := hosting_service.GetRepoInfoFromURL(url) - if err != nil { - return "", "", err - } - - return repoInfo.Owner, repoInfo.Repository, nil -} diff --git a/pkg/commands/git_commands/hosting_service.go b/pkg/commands/git_commands/hosting_service.go index 5deceea8e..f43b93e90 100644 --- a/pkg/commands/git_commands/hosting_service.go +++ b/pkg/commands/git_commands/hosting_service.go @@ -21,10 +21,6 @@ func (self *HostingService) GetCommitURL(commitSha string) (string, error) { return self.getHostingServiceMgr(self.config.GetRemoteURL()).GetCommitURL(commitSha) } -func (self *HostingService) GetRepoNameFromRemoteURL(remoteURL string) (string, error) { - return self.getHostingServiceMgr(remoteURL).GetRepoName() -} - func (self *HostingService) GetServiceInfo(remoteURL string) (hosting_service.ServiceInfo, error) { return self.getHostingServiceMgr(remoteURL).GetServiceInfo() } From eba1df11a84620af866633b92794e0121b8a6a84 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 8 May 2026 07:00:00 +0200 Subject: [PATCH 25/28] Document that the services entry also enables GHE pull-request icons The services config has been the path for GHE for a while (for the View-PR-URL feature) but it now does double duty: it's also what enables the branches-panel PR icons for non-github.com hosts. Worth calling out explicitly so users don't assume it's still github.com-only. Co-Authored-By: Claude Opus 4.7 (1M context) --- README.md | 2 +- docs-master/Config.md | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 044386655..549366864 100644 --- a/README.md +++ b/README.md @@ -230,7 +230,7 @@ If you press `shift+w` on a commit (or branch/ref) a menu will open that allows ### Show GitHub pull requests -In the branches panel, lazygit can show which of your branches have an associated GitHub pull request by showing a GitHub icon next to the branch name; its color shows the state of the PR (open, merged, etc.). For those that have one, you can press `shift-G` to open the PR in the browser. There is no configuration needed to enable this, but it requires the [`gh`](https://cli.github.com/) tool to be installed, and you need to do `gh auth login` once to allow lazygit to access GitHub. +In the branches panel, lazygit can show which of your branches have an associated GitHub pull request by showing a GitHub icon next to the branch name; its color shows the state of the PR (open, merged, etc.). For those that have one, you can press `shift-G` to open the PR in the browser. There is no configuration needed to enable this for github.com, but it requires the [`gh`](https://cli.github.com/) tool to be installed, and you need to do `gh auth login` once to allow lazygit to access GitHub. For GitHub Enterprise, also run `gh auth login --hostname ` and add a [`services` entry](docs/Config.md#custom-pull-request-urls) for the host with the `github` provider. ## Tutorials diff --git a/docs-master/Config.md b/docs-master/Config.md index 42eff4666..05d3b6d20 100644 --- a/docs-master/Config.md +++ b/docs-master/Config.md @@ -1117,6 +1117,8 @@ Where: - `provider` is one of `github`, `bitbucket`, `bitbucketServer`, `azuredevops`, `gitlab`, `gitea` or `codeberg` - `webDomain` is the URL where your git service exposes a web interface and APIs, e.g. `gitservice.work.com` +For the `github` provider, configuring an entry here also enables the pull-request icons in the branches panel for that host (e.g. a GitHub Enterprise Server instance). Lazygit picks up the auth token via the same mechanisms as the `gh` CLI: the `GH_ENTERPRISE_TOKEN` / `GITHUB_ENTERPRISE_TOKEN` environment variables, or `gh auth login --hostname `. + ## Predefined commit message prefix In situations where certain naming pattern is used for branches and commits, pattern can be used to populate commit message with prefix that is parsed from the branch name. From 692f56a61b6af8be5cad26a7c76975eb0b15d715 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 8 May 2026 13:21:40 +0200 Subject: [PATCH 26/28] Optimize regex compilations Compile them only once at startup. I didn't measure if this makes a difference, but it's easy to do, and now that we potentially need to check them more often, it might be worth it. --- pkg/commands/hosting_service/definitions.go | 38 ++++++++++--------- .../hosting_service/hosting_service.go | 8 ++-- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/pkg/commands/hosting_service/definitions.go b/pkg/commands/hosting_service/definitions.go index 130bf0481..09fa191c8 100644 --- a/pkg/commands/hosting_service/definitions.go +++ b/pkg/commands/hosting_service/definitions.go @@ -1,10 +1,12 @@ package hosting_service +import "regexp" + // if you want to make a custom regex for a given service feel free to test it out // at https://regex101.com using the flavor Golang -var defaultUrlRegexStrings = []string{ - `^(?:https?|ssh)://[^/]+/(?P.*)/(?P.*?)(?:\.git)?$`, - `^(.*?@)?.*:/*(?P.*)/(?P.*?)(?:\.git)?$`, +var defaultUrlRegexps = []*regexp.Regexp{ + regexp.MustCompile(`^(?:https?|ssh)://[^/]+/(?P.*)/(?P.*?)(?:\.git)?$`), + regexp.MustCompile(`^(.*?@)?.*:/*(?P.*)/(?P.*?)(?:\.git)?$`), } var ( @@ -19,7 +21,7 @@ var githubServiceDef = ServiceDefinition{ pullRequestURLIntoDefaultBranch: "/compare/{{.From}}?expand=1", pullRequestURLIntoTargetBranch: "/compare/{{.To}}...{{.From}}?expand=1", commitURL: "/commit/{{.CommitHash}}", - regexStrings: defaultUrlRegexStrings, + urlRegexps: defaultUrlRegexps, repoURLTemplate: defaultRepoURLTemplate, repoNameTemplate: defaultRepoNameTemplate, } @@ -29,9 +31,9 @@ var bitbucketServiceDef = ServiceDefinition{ pullRequestURLIntoDefaultBranch: "/pull-requests/new?source={{.From}}&t=1", pullRequestURLIntoTargetBranch: "/pull-requests/new?source={{.From}}&dest={{.To}}&t=1", commitURL: "/commits/{{.CommitHash}}", - regexStrings: []string{ - `^(?:https?|ssh)://.*/(?P.*)/(?P.*?)(?:\.git)?$`, - `^.*@.*:/*(?P.*)/(?P.*?)(?:\.git)?$`, + urlRegexps: []*regexp.Regexp{ + regexp.MustCompile(`^(?:https?|ssh)://.*/(?P.*)/(?P.*?)(?:\.git)?$`), + regexp.MustCompile(`^.*@.*:/*(?P.*)/(?P.*?)(?:\.git)?$`), }, repoURLTemplate: defaultRepoURLTemplate, repoNameTemplate: defaultRepoNameTemplate, @@ -42,7 +44,7 @@ var gitLabServiceDef = ServiceDefinition{ pullRequestURLIntoDefaultBranch: "/-/merge_requests/new?merge_request%5Bsource_branch%5D={{.From}}", pullRequestURLIntoTargetBranch: "/-/merge_requests/new?merge_request%5Bsource_branch%5D={{.From}}&merge_request%5Btarget_branch%5D={{.To}}", commitURL: "/-/commit/{{.CommitHash}}", - regexStrings: defaultUrlRegexStrings, + urlRegexps: defaultUrlRegexps, repoURLTemplate: defaultRepoURLTemplate, repoNameTemplate: defaultRepoNameTemplate, } @@ -52,11 +54,11 @@ var azdoServiceDef = ServiceDefinition{ pullRequestURLIntoDefaultBranch: "/pullrequestcreate?sourceRef={{.From}}", pullRequestURLIntoTargetBranch: "/pullrequestcreate?sourceRef={{.From}}&targetRef={{.To}}", commitURL: "/commit/{{.CommitHash}}", - regexStrings: []string{ - `^.+@vs-ssh\.visualstudio\.com[:/](?:v3/)?(?P[^/]+)/(?P[^/]+)/(?P[^/]+?)(?:\.git)?$`, - `^git@ssh.dev.azure.com.*/(?P.*)/(?P.*)/(?P.*?)(?:\.git)?$`, - `^https://.*@dev.azure.com/(?P.*?)/(?P.*?)/_git/(?P.*?)(?:\.git)?$`, - `^https://.*/(?P.*?)/(?P.*?)/_git/(?P.*?)(?:\.git)?$`, + urlRegexps: []*regexp.Regexp{ + regexp.MustCompile(`^.+@vs-ssh\.visualstudio\.com[:/](?:v3/)?(?P[^/]+)/(?P[^/]+)/(?P[^/]+?)(?:\.git)?$`), + regexp.MustCompile(`^git@ssh.dev.azure.com.*/(?P.*)/(?P.*)/(?P.*?)(?:\.git)?$`), + regexp.MustCompile(`^https://.*@dev.azure.com/(?P.*?)/(?P.*?)/_git/(?P.*?)(?:\.git)?$`), + regexp.MustCompile(`^https://.*/(?P.*?)/(?P.*?)/_git/(?P.*?)(?:\.git)?$`), }, repoURLTemplate: "https://{{.webDomain}}/{{.org}}/{{.project}}/_git/{{.repo}}", repoNameTemplate: "{{.org}}/{{.project}}/{{.repo}}", @@ -67,9 +69,9 @@ var bitbucketServerServiceDef = ServiceDefinition{ pullRequestURLIntoDefaultBranch: "/pull-requests?create&sourceBranch={{.From}}", pullRequestURLIntoTargetBranch: "/pull-requests?create&targetBranch={{.To}}&sourceBranch={{.From}}", commitURL: "/commits/{{.CommitHash}}", - regexStrings: []string{ - `^ssh://git@.*/(?P.*)/(?P.*?)(?:\.git)?$`, - `^https://.*/scm/(?P.*)/(?P.*?)(?:\.git)?$`, + urlRegexps: []*regexp.Regexp{ + regexp.MustCompile(`^ssh://git@.*/(?P.*)/(?P.*?)(?:\.git)?$`), + regexp.MustCompile(`^https://.*/scm/(?P.*)/(?P.*?)(?:\.git)?$`), }, repoURLTemplate: "https://{{.webDomain}}/projects/{{.project}}/repos/{{.repo}}", repoNameTemplate: "{{.project}}/{{.repo}}", @@ -80,7 +82,7 @@ var giteaServiceDef = ServiceDefinition{ pullRequestURLIntoDefaultBranch: "/compare/{{.From}}", pullRequestURLIntoTargetBranch: "/compare/{{.To}}...{{.From}}", commitURL: "/commit/{{.CommitHash}}", - regexStrings: defaultUrlRegexStrings, + urlRegexps: defaultUrlRegexps, repoURLTemplate: defaultRepoURLTemplate, } @@ -89,7 +91,7 @@ var codebergServiceDef = ServiceDefinition{ pullRequestURLIntoDefaultBranch: "/compare/{{.From}}", pullRequestURLIntoTargetBranch: "/compare/{{.To}}...{{.From}}", commitURL: "/commit/{{.CommitHash}}", - regexStrings: defaultUrlRegexStrings, + urlRegexps: defaultUrlRegexps, repoURLTemplate: defaultRepoURLTemplate, } diff --git a/pkg/commands/hosting_service/hosting_service.go b/pkg/commands/hosting_service/hosting_service.go index ad1f072c5..ff2641441 100644 --- a/pkg/commands/hosting_service/hosting_service.go +++ b/pkg/commands/hosting_service/hosting_service.go @@ -195,7 +195,7 @@ type ServiceDefinition struct { pullRequestURLIntoDefaultBranch string pullRequestURLIntoTargetBranch string commitURL string - regexStrings []string + urlRegexps []*regexp.Regexp // can expect 'webdomain' to be passed in. Otherwise, you get to pick what we match in the regex repoURLTemplate string @@ -222,8 +222,7 @@ func (self ServiceDefinition) getRepoNameFromRemoteURL(url string) (string, erro } func (self ServiceDefinition) parseRemoteUrl(url string) (map[string]string, error) { - for _, regexStr := range self.regexStrings { - re := regexp.MustCompile(regexStr) + for _, re := range self.urlRegexps { matches := utils.FindNamedMatches(re, url) if matches != nil { return matches, nil @@ -242,8 +241,7 @@ type RepoInformation struct { // GetRepoInfoFromURL parses a remote URL (SSH or HTTPS) and extracts the // owner and repository name using the default URL regex patterns. func GetRepoInfoFromURL(url string) (RepoInformation, error) { - for _, regexStr := range defaultUrlRegexStrings { - re := regexp.MustCompile(regexStr) + for _, re := range defaultUrlRegexps { matches := utils.FindNamedMatches(re, url) if matches != nil { return RepoInformation{ From d9aceaf0daa2449516543b35c9f1a11aded7a954 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 10 May 2026 15:40:09 +0200 Subject: [PATCH 27/28] Keep GitHub PR refresh error logging in one place --- pkg/gui/controllers/helpers/refresh_helper.go | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index 6c554deff..3e0d15bcd 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -831,9 +831,7 @@ func (self *RefreshHelper) refreshGithubPullRequests() { return } - if err := self.setGithubPullRequests(baseInfo); err != nil { - self.c.LogAction(fmt.Sprintf("Error fetching pull requests from GitHub: %s", err.Error())) - } + self.setGithubPullRequests(baseInfo) } type githubRemoteInfo struct { @@ -914,9 +912,7 @@ func (self *RefreshHelper) promptForBaseGithubRepo(githubRemotes []githubRemoteI self.c.Log.Error(err) } - if err := self.setGithubPullRequests(&info); err != nil { - self.c.LogAction(fmt.Sprintf("Error fetching pull requests from GitHub: %s", err.Error())) - } + self.setGithubPullRequests(&info) return nil }) }, @@ -944,9 +940,9 @@ func (self *RefreshHelper) rebuildPullRequestsMap() { ) } -func (self *RefreshHelper) setGithubPullRequests(baseInfo *githubRemoteInfo) error { +func (self *RefreshHelper) setGithubPullRequests(baseInfo *githubRemoteInfo) { if len(self.c.Model().Branches) == 0 { - return nil + return } branches := lo.Filter(self.c.Model().Branches, func(branch *models.Branch, _ int) bool { @@ -958,7 +954,8 @@ func (self *RefreshHelper) setGithubPullRequests(baseInfo *githubRemoteInfo) err prs, err := self.c.Git().GitHub.FetchRecentPRs(branchNames, &baseInfo.serviceInfo, baseInfo.authToken) if err != nil { - return err + self.c.LogAction(fmt.Sprintf("Error fetching pull requests from GitHub: %s", err.Error())) + return } self.c.Model().PullRequests = prs @@ -969,8 +966,6 @@ func (self *RefreshHelper) setGithubPullRequests(baseInfo *githubRemoteInfo) err self.c.PostRefreshUpdate(self.c.Contexts().Branches) return nil }) - - return nil } func (self *RefreshHelper) savePullRequestsToCache(prs []*models.GithubPullRequest) { From 58abf24862dd64f42887fa502d40622c0076126f Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 10 May 2026 15:43:40 +0200 Subject: [PATCH 28/28] Log errors from fetching GitHub PRs to the debug log, not to the Command Log In the Command Log we only want to see errors for user-initiated actions, not from background activity. --- pkg/gui/controllers/helpers/refresh_helper.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index 3e0d15bcd..2922b4403 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -1,7 +1,6 @@ package helpers import ( - "fmt" "strings" "sync" "time" @@ -954,7 +953,7 @@ func (self *RefreshHelper) setGithubPullRequests(baseInfo *githubRemoteInfo) { prs, err := self.c.Git().GitHub.FetchRecentPRs(branchNames, &baseInfo.serviceInfo, baseInfo.authToken) if err != nil { - self.c.LogAction(fmt.Sprintf("Error fetching pull requests from GitHub: %s", err.Error())) + self.c.Log.Error("error fetching pull requests from GitHub: " + err.Error()) return }