mirror of
https://github.com/jesseduffield/lazygit.git
synced 2026-08-24 10:13:41 -05:00
Render the app status in a single background render loop
Each status used to start a spinner render loop of its own, running on a worker that inherited the foreground/background flavor of the status's owner, and exiting only once the entire status stack was empty. That shape had a real bug: a foreground operation's loop could be kept alive by someone else's status. Finish a quick operation with a waiting status while a background fetch's "Fetching..." status is still showing, and the operation's render loop — a foreground worker task — keeps ticking until the fetch ends. Busy() stays true for that whole time, so repo switching is refused even though nothing is in flight anymore; with a fetch hanging on a slow network, that means minutes. The shape was also wasteful: overlapping statuses were each drawn by their own loop (plus a duplicate whenever a task was paused and resumed while another status was showing), all redundantly redrawing the same top status. Replace the per-status loops with a single loop owned by the status stack as a whole: whoever shows the first status starts it, and it exits after drawing a final empty frame once the last status is removed. The claim/release methods on StatusManager keep the loop flag's transitions atomic with the stack under the one mutex, so a status added while the loop is about to exit starts a fresh loop instead of going unrendered. The loop always runs as a background task now: rendering issues no git commands, so it never needs to block repo switching, and a foreground operation's busy-ness is already carried by its own worker task. This is what fixes the bug above, and it retires the need to thread a foreground/background flag through the waiting-status helpers altogether. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
7360a8459d
commit
a1561a5e69
@@ -119,7 +119,7 @@ func (self *BackgroundRoutineMgr) startBackgroundFetch() {
|
||||
if self.gui.UserConfig().Gui.ShowBottomLine || firstTimeOrRetriggered {
|
||||
return self.gui.helpers.AppStatus.WithWaitingStatusImpl(self.gui.Tr.FetchingStatus, func(gocui.Task) error {
|
||||
return self.backgroundFetch()
|
||||
}, nil, true)
|
||||
}, nil)
|
||||
}
|
||||
|
||||
return self.backgroundFetch()
|
||||
|
||||
@@ -34,12 +34,7 @@ func (self *AppStatusHelper) Toast(message string, kind types.ToastKind) {
|
||||
|
||||
self.statusMgr().AddToastStatus(message, kind)
|
||||
|
||||
// Render the toast in the background: it's a transient notification, not
|
||||
// lazygit driving an operation, so it must not count towards being busy —
|
||||
// otherwise a toast (e.g. the "can't switch, operation in progress" one)
|
||||
// would itself block a repo switch until it faded. A real operation showing
|
||||
// a toast still keeps its own foreground task busy independently.
|
||||
self.renderAppStatus(true)
|
||||
self.renderAppStatus()
|
||||
}
|
||||
|
||||
// A custom task for WithWaitingStatus calls; it wraps the original one and
|
||||
@@ -66,14 +61,15 @@ func (self appStatusHelperTask) Continue() {
|
||||
// WithWaitingStatus wraps a function and shows a waiting status while the function is still executing
|
||||
func (self *AppStatusHelper) WithWaitingStatus(message string, f func(gocui.Task) error) {
|
||||
self.c.OnWorker(func(task gocui.Task) error {
|
||||
return self.WithWaitingStatusImpl(message, f, task, false)
|
||||
return self.WithWaitingStatusImpl(message, f, task)
|
||||
})
|
||||
}
|
||||
|
||||
// background reports whether this waiting status belongs to a background routine
|
||||
// (the auto-fetch poller); when it does, the spinner it drives must not count
|
||||
// towards lazygit being busy, or it'd block repo switches while a fetch runs.
|
||||
func (self *AppStatusHelper) WithWaitingStatusImpl(message string, f func(gocui.Task) error, task gocui.Task, background bool) error {
|
||||
// WithWaitingStatusImpl is WithWaitingStatus for callers that already run on a
|
||||
// goroutine of their own (e.g. the auto-fetch poller) rather than wanting the
|
||||
// work dispatched to a worker. task is used to hide the status while the task
|
||||
// is paused; it may be nil for callers whose f ignores its task.
|
||||
func (self *AppStatusHelper) WithWaitingStatusImpl(message string, f func(gocui.Task) error, task gocui.Task) error {
|
||||
// A waiting status means lazygit is driving a git operation itself (often
|
||||
// one that internally runs a rebase and continues it). Pause the background
|
||||
// routines for its duration so they don't refresh from an intermediate
|
||||
@@ -81,7 +77,7 @@ func (self *AppStatusHelper) WithWaitingStatusImpl(message string, f func(gocui.
|
||||
self.c.PauseBackgroundRefreshes(true)
|
||||
defer self.c.PauseBackgroundRefreshes(false)
|
||||
|
||||
return self.statusMgr().WithWaitingStatus(message, func() { self.renderAppStatus(background) }, func(waitingStatusHandle *status.WaitingStatusHandle) error {
|
||||
return self.statusMgr().WithWaitingStatus(message, self.renderAppStatus, func(waitingStatusHandle *status.WaitingStatusHandle) error {
|
||||
return f(appStatusHelperTask{task, waitingStatusHandle})
|
||||
})
|
||||
}
|
||||
@@ -112,7 +108,7 @@ func (self *AppStatusHelper) WithWaitingStatusBlockingInput(message string, f fu
|
||||
self.modeHelper.SetSuppressRebasingMode(false)
|
||||
return self.c.GocuiGui().EndBlockingEvents()
|
||||
})
|
||||
return self.WithWaitingStatusImpl(message, f, task, false)
|
||||
return self.WithWaitingStatusImpl(message, f, task)
|
||||
})
|
||||
}
|
||||
|
||||
@@ -125,33 +121,36 @@ func (self *AppStatusHelper) GetStatusString() string {
|
||||
return appStatus
|
||||
}
|
||||
|
||||
func (self *AppStatusHelper) renderAppStatus(background bool) {
|
||||
// A background waiting status (auto-fetch) must not count towards lazygit
|
||||
// being busy, so its spinner worker and per-frame UI updates go through the
|
||||
// background variants.
|
||||
onWorker := self.c.OnWorker
|
||||
onUIThread := self.c.OnUIThread
|
||||
onUIThreadContentOnly := self.c.OnUIThreadContentOnly
|
||||
if background {
|
||||
onWorker = self.c.OnWorkerBackground
|
||||
onUIThread = self.c.OnUIThreadBackground
|
||||
onUIThreadContentOnly = self.c.OnUIThreadContentOnlyBackground
|
||||
// renderAppStatus ensures the render loop that keeps the app-status view up to
|
||||
// date is running. There is one loop for the whole status stack, no matter how
|
||||
// many statuses are showing: it draws whatever the top status currently is,
|
||||
// and exits after drawing a final empty frame once the last status is removed.
|
||||
//
|
||||
// The loop always runs as a background task, regardless of what kind of
|
||||
// operation owns a status: rendering runs no git commands, so it must never
|
||||
// count towards lazygit being busy — otherwise it would block repo switching
|
||||
// for as long as anything is showing (e.g. for the whole duration of a hung
|
||||
// background fetch, or of a toast fading). A foreground operation's busy-ness
|
||||
// is carried by its own worker task, not by the renderer.
|
||||
func (self *AppStatusHelper) renderAppStatus() {
|
||||
if !self.statusMgr().ClaimRenderLoop() {
|
||||
return
|
||||
}
|
||||
|
||||
onWorker(func(_ gocui.Task) error {
|
||||
self.c.OnWorkerBackground(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())
|
||||
|
||||
update := onUIThreadContentOnly
|
||||
update := self.c.OnUIThreadContentOnlyBackground
|
||||
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 = onUIThread
|
||||
update = self.c.OnUIThreadBackground
|
||||
}
|
||||
update(func() error {
|
||||
self.c.Views().AppStatus.FgColor = color
|
||||
@@ -160,7 +159,9 @@ func (self *AppStatusHelper) renderAppStatus(background bool) {
|
||||
})
|
||||
prevAppStatus = appStatus
|
||||
|
||||
if appStatus == "" {
|
||||
// Checked after rendering, so that the frame which clears the view
|
||||
// has already been drawn when we exit.
|
||||
if self.statusMgr().ReleaseRenderLoopIfEmpty() {
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
@@ -17,6 +17,11 @@ type StatusManager struct {
|
||||
statuses []appStatus
|
||||
nextId int
|
||||
mutex deadlock.Mutex
|
||||
|
||||
// Whether a render loop is currently drawing the statuses. Guarded by
|
||||
// mutex, so that claiming and releasing the loop stay atomic with the
|
||||
// changes to statuses; see ClaimRenderLoop and ReleaseRenderLoopIfEmpty.
|
||||
renderLoopRunning bool
|
||||
}
|
||||
|
||||
// Can be used to manipulate a waiting status while it is running (e.g. pause
|
||||
@@ -90,6 +95,39 @@ func (self *StatusManager) HasStatus() bool {
|
||||
return len(self.statuses) > 0
|
||||
}
|
||||
|
||||
// ClaimRenderLoop is called by whoever just added a status; it reports whether
|
||||
// they must start the render loop. When it returns false, a loop is already
|
||||
// running and will pick the new status up on its next tick.
|
||||
func (self *StatusManager) ClaimRenderLoop() bool {
|
||||
self.mutex.Lock()
|
||||
defer self.mutex.Unlock()
|
||||
|
||||
if self.renderLoopRunning {
|
||||
return false
|
||||
}
|
||||
|
||||
self.renderLoopRunning = true
|
||||
return true
|
||||
}
|
||||
|
||||
// ReleaseRenderLoopIfEmpty is called by the render loop after each frame it
|
||||
// draws; a true result releases the loop's claim and tells it to exit, because
|
||||
// there are no statuses left to draw. The emptiness check and the release are
|
||||
// atomic with respect to ClaimRenderLoop, so a status added around this moment
|
||||
// either sees the still-running loop or starts a fresh one — it can't end up
|
||||
// unrendered.
|
||||
func (self *StatusManager) ReleaseRenderLoopIfEmpty() bool {
|
||||
self.mutex.Lock()
|
||||
defer self.mutex.Unlock()
|
||||
|
||||
if len(self.statuses) > 0 {
|
||||
return false
|
||||
}
|
||||
|
||||
self.renderLoopRunning = false
|
||||
return true
|
||||
}
|
||||
|
||||
func (self *StatusManager) addStatus(message string, statusType string, kind types.ToastKind) int {
|
||||
self.mutex.Lock()
|
||||
defer self.mutex.Unlock()
|
||||
|
||||
Reference in New Issue
Block a user