diff --git a/AGENTS.md b/AGENTS.md index 46ad3e506..947add510 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -48,6 +48,9 @@ while still being meaningful and self-contained. commits that leave the tree broken and rely on a follow-up to fix it. - **Every commit must be `gofumpt`-formatted.** Run `just format` before committing. +- **Every commit must be lint-clean.** Run `just lint` before committing — + don't introduce a lint warning in one commit and rely on a later commit + (or the user) to clean it up. - **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 @@ -157,6 +160,16 @@ genuine forks — the ones where a reasonable person might pick differently, or where you'd be trading away something the plan assumed (scope, UX, performance, reload behavior, …). When in doubt, surface it. +This applies with equal force to unforeseen _discoveries_, not just to +decisions you set out to make. If you find something the plan didn't account +for — a latent bug, a race, a wrong assumption, a case that turns out +unhandled — stop and raise it before designing or writing a fix, even when the +fix seems obvious and even when it's "just correctness." Finding the problem is +itself the fork: whether to fix it here or in a separate change, how generally +to solve it, and whether it reshapes the current work are all calls for me to +make with you. Don't quietly fold a self-directed fix for a newly-found problem +into the branch and let me discover it in the diff. + ## Prefer the cleaner design over the smaller diff When a task could be implemented either by tacking onto existing code or by @@ -243,6 +256,34 @@ Follow this even when the need for the refactor is only discovered in the middle of working on the branch; suggest to the user to rewrite the history to move the refactor to an earlier commit (but don't do it without asking first). +## Don't read model state right after a `Refresh` + +A `Refresh` (or `RefreshFromWorker`) does its git work on a worker and then +*enqueues* the model update onto the UI thread. So when `Refresh` returns, the +model is **not** updated yet — the write is still queued. Reading a field +synchronously right after refreshing its scope reads the stale, pre-refresh +value (and this is true even for SYNC refreshes): + +```go +self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.FILES}}) +files := self.c.Model().Files // BUG: still the pre-refresh value +``` + +Put the read in `RefreshOptions.Then` instead — it's queued after the scope's +model writes, so it sees the fresh value: + +```go +self.c.Refresh(types.RefreshOptions{ + Scope: []types.RefreshableView{types.FILES}, + Then: func() error { + files := self.c.Model().Files // fresh + return nil + }, +}) +``` + +`Then` is a `func() error` and works with any non-`ASYNC` mode. + ## Integration test conventions Don't bind views to local variables. Always chain method calls directly from diff --git a/go.mod b/go.mod index 6820d941d..c10004176 100644 --- a/go.mod +++ b/go.mod @@ -25,6 +25,7 @@ require ( github.com/lucasb-eyer/go-colorful v1.4.0 github.com/mgutz/str v1.2.0 github.com/mitchellh/go-ps v1.0.0 + github.com/petermattis/goid v0.0.0-20250813065127-a731cc31b4fe github.com/rivo/uniseg v0.4.7 github.com/sahilm/fuzzy v0.1.3 github.com/samber/lo v1.53.0 @@ -62,7 +63,6 @@ require ( github.com/mattn/go-isatty v0.0.20 // indirect github.com/onsi/ginkgo v1.10.3 // indirect github.com/onsi/gomega v1.34.1 // indirect - github.com/petermattis/goid v0.0.0-20250813065127-a731cc31b4fe // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/wk8/go-ordered-map/v2 v2.1.8 // indirect golang.org/x/mod v0.35.0 // indirect diff --git a/pkg/gocui/gui.go b/pkg/gocui/gui.go index ee1995911..6002ebf9c 100644 --- a/pkg/gocui/gui.go +++ b/pkg/gocui/gui.go @@ -9,11 +9,13 @@ import ( "runtime" "strings" "sync" + "sync/atomic" "time" "github.com/gdamore/tcell/v3" "github.com/go-errors/errors" "github.com/jesseduffield/generics/set" + "github.com/petermattis/goid" "github.com/rivo/uniseg" "github.com/samber/lo" ) @@ -193,7 +195,18 @@ type Gui struct { taskManager *TaskManager + // The task of the event currently being processed on the main goroutine, if + // any. Only touched from the main goroutine (in processEvent). It's excluded + // from the Busy() check so that an event handler asking "is anything else + // busy?" doesn't count itself. + currentTask Task + lastHoverView *View + + // uiThreadID is the goroutine id of the main event loop, recorded when + // MainLoop starts. IsUIThread compares against it. Written once, read from + // worker goroutines, so it's atomic. + uiThreadID atomic.Int64 } type NewGuiOpts struct { @@ -273,7 +286,22 @@ func NewGui(opts NewGuiOpts) (*Gui, error) { } func (g *Gui) NewTask() *TaskImpl { - return g.taskManager.NewTask() + return g.taskManager.NewTask(false) +} + +// NewBackgroundTask creates a task that is tracked for idle detection but does +// not count towards the program being busy for repo-switch safety. See +// TaskImpl.background. +func (g *Gui) NewBackgroundTask() *TaskImpl { + return g.taskManager.NewTask(true) +} + +// Busy reports whether any foreground work is in flight, ignoring the event +// currently being processed on the main goroutine (see currentTask). Background +// routines (auto-fetch etc.) don't count. It's used to decide whether it's safe +// to switch repos. Must be called on the main goroutine. +func (g *Gui) Busy() bool { + return g.taskManager.hasBusyForegroundTaskExcept(g.currentTask) } // An idle listener listens for when the program is idle. This is useful for @@ -628,7 +656,18 @@ type userEvent struct { // never fire in practice; if it does, that's a signal to investigate, not // to grow the buffer reflexively. func (g *Gui) Update(f func(*Gui) error) { - task := g.NewTask() + g.update(f, false) +} + +// Like Update, but the enqueued work is a background routine (or triggered by +// one), so it doesn't count towards the program being busy for repo-switch +// safety. See TaskImpl.background. +func (g *Gui) UpdateBackground(f func(*Gui) error) { + g.update(f, true) +} + +func (g *Gui) update(f func(*Gui) error, background bool) { + task := g.taskManager.NewTask(background) select { case g.userEvents <- userEvent{f: f, task: task}: @@ -639,10 +678,59 @@ func (g *Gui) Update(f func(*Gui) error) { // Like Update, but signals that the callback only modifies content. func (g *Gui) UpdateContentOnly(f func(*Gui) error) { - task := g.NewTask() + g.updateContentOnly(f, false) +} + +// Like UpdateContentOnly, but for background work (see UpdateBackground). +func (g *Gui) UpdateContentOnlyBackground(f func(*Gui) error) { + g.updateContentOnly(f, true) +} + +func (g *Gui) updateContentOnly(f func(*Gui) error, background bool) { + task := g.taskManager.NewTask(background) g.userEvents <- userEvent{f: f, task: task, contentOnly: true} } +// IsUIThread reports whether the caller is running on the main event-loop +// goroutine (the one running MainLoop). It calls goid.Get, so use it only for +// debug assertions, not to drive production control flow. +func (g *Gui) IsUIThread() bool { + return goid.Get() == g.uiThreadID.Load() +} + +// OnUIThreadAndWait runs f on the main event-loop goroutine and blocks the +// caller until f has run, returning f's error. Use it to read UI-thread-owned +// state (the model, contexts) from a worker without racing the UI thread. +// +// It must be called from a worker goroutine, never from the UI thread itself: +// the UI thread would block waiting for a callback only it can run, which +// deadlocks. Callers arrange this by construction (see the refresh helper's +// RefreshFromWorker); a debug-only assertion there guards against getting it +// wrong. +func (g *Gui) OnUIThreadAndWait(f func() error) error { + return g.onUIThreadAndWait(f, false) +} + +// Like OnUIThreadAndWait, but the enqueued work belongs to a background routine, +// so it doesn't count towards the program being busy (see UpdateBackground). +func (g *Gui) OnUIThreadAndWaitBackground(f func() error) error { + return g.onUIThreadAndWait(f, true) +} + +func (g *Gui) onUIThreadAndWait(f func() error, background bool) error { + enqueue := g.Update + if background { + enqueue = g.UpdateBackground + } + + result := make(chan error, 1) + enqueue(func(*Gui) error { + result <- f() + return nil + }) + return <-result +} + // 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 @@ -650,7 +738,18 @@ func (g *Gui) UpdateContentOnly(f func(*Gui) error) { // background goroutines where you wouldn't want lazygit to be considered busy // (i.e. when you wouldn't want a loader to be shown to the user) func (g *Gui) OnWorker(f func(Task) error) { - task := g.NewTask() + g.onWorker(f, false) +} + +// Like OnWorker, but for a background routine (or work triggered by one), so it +// doesn't count towards the program being busy for repo-switch safety. See +// TaskImpl.background. +func (g *Gui) OnWorkerBackground(f func(Task) error) { + g.onWorker(f, true) +} + +func (g *Gui) onWorker(f func(Task) error, background bool) { + task := g.taskManager.NewTask(background) go func() { g.onWorkerAux(f, task) task.Done() @@ -714,6 +813,8 @@ func (g *Gui) SetManagerFunc(manager func(*Gui) error) { // MainLoop runs the main loop until an error is returned. A successful // finish should return ErrQuit. func (g *Gui) MainLoop() error { + g.uiThreadID.Store(goid.Get()) + go func() { for { select { @@ -758,17 +859,25 @@ func (g *Gui) handleError(err error) error { func (g *Gui) processEvent() error { contentOnly := false + // currentTask is the task of the event we're about to handle; recording it + // lets Busy() ignore it, so a handler asking "is anything else busy?" (the + // repo-switch guard does) doesn't count itself. Handlers of the remaining + // events drained below run with currentTask still set to this primary event; + // that's fine because the only Busy() callers are keybinding handlers, which + // are always the primary event here. select { case ev := <-g.gEvents: task := g.NewTask() - defer func() { task.Done() }() + g.currentTask = task + defer func() { g.currentTask = nil; task.Done() }() if err := g.handleError(g.handleEvent(&ev)); err != nil { return err } case ev := <-g.userEvents: contentOnly = ev.contentOnly - defer func() { ev.task.Done() }() + g.currentTask = ev.task + defer func() { g.currentTask = nil; ev.task.Done() }() if err := g.handleError(ev.f(g)); err != nil { return err diff --git a/pkg/gocui/task.go b/pkg/gocui/task.go index ace72f4a8..08a77463f 100644 --- a/pkg/gocui/task.go +++ b/pkg/gocui/task.go @@ -8,8 +8,9 @@ type Task interface { Done() Pause() Continue() - // not exporting because we don't need to + // not exporting these because we don't need to isBusy() bool + isBackground() bool } type TaskImpl struct { @@ -17,6 +18,17 @@ type TaskImpl struct { busy bool onDone func() withMutex func(func()) + // Background tasks don't count towards the program being "busy" for the + // purpose of deciding whether a repo switch is safe (see + // TaskManager.hasBusyForegroundTaskExcept). Two kinds of work are tagged + // this way: the ongoing background routines (auto-fetch, files refresh, + // external-change detection) and the refreshes they trigger, whose model + // writes are already guarded against a concurrent repo switch by the repo + // generation; and view-buffer content rendering, which only paints a view + // and so is harmless to leave running across a switch. What stays + // foreground is lazygit driving a git operation and applying its results + // to the model — exactly the work a repo switch must not run underneath. + background bool } func (self *TaskImpl) Done() { @@ -39,6 +51,10 @@ func (self *TaskImpl) isBusy() bool { return self.busy } +func (self *TaskImpl) isBackground() bool { + return self.background +} + type TaskStatus int const ( @@ -73,6 +89,10 @@ func (self *FakeTask) isBusy() bool { return self.status == TaskStatusBusy } +func (self *FakeTask) isBackground() bool { + return false +} + func (self *FakeTask) Status() TaskStatus { return self.status } diff --git a/pkg/gocui/task_manager.go b/pkg/gocui/task_manager.go index e3c82b4d4..23ef0f77e 100644 --- a/pkg/gocui/task_manager.go +++ b/pkg/gocui/task_manager.go @@ -22,7 +22,7 @@ func newTaskManager() *TaskManager { } } -func (self *TaskManager) NewTask() *TaskImpl { +func (self *TaskManager) NewTask(background bool) *TaskImpl { self.mutex.Lock() defer self.mutex.Unlock() @@ -30,12 +30,34 @@ func (self *TaskManager) NewTask() *TaskImpl { taskId := self.nextId onDone := func() { self.delete(taskId) } - task := &TaskImpl{id: taskId, busy: true, onDone: onDone, withMutex: self.withMutex} + task := &TaskImpl{id: taskId, busy: true, background: background, onDone: onDone, withMutex: self.withMutex} self.tasks[taskId] = task return task } +// hasBusyForegroundTaskExcept reports whether any task other than `ignore` is +// currently busy and not a background task. It's used to decide whether a repo +// switch is safe: a foreground operation (or the refresh it triggers, or that +// refresh's follow-up callbacks) still in flight means the switch must wait, so +// it doesn't run against a repo that's about to be swapped out. +// +// `ignore` is the event currently being processed on the UI thread — the switch +// attempt itself — which is always busy and so must not count as a reason to +// refuse itself. +func (self *TaskManager) hasBusyForegroundTaskExcept(ignore Task) bool { + self.mutex.Lock() + defer self.mutex.Unlock() + + for _, task := range self.tasks { + if task != ignore && task.isBusy() && !task.isBackground() { + return true + } + } + + return false +} + func (self *TaskManager) addIdleListener(c chan struct{}) { self.idleListeners = append(self.idleListeners, c) } diff --git a/pkg/gocui/task_manager_test.go b/pkg/gocui/task_manager_test.go new file mode 100644 index 000000000..7fe706d7a --- /dev/null +++ b/pkg/gocui/task_manager_test.go @@ -0,0 +1,63 @@ +package gocui + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestTaskManagerHasBusyForegroundTaskExcept(t *testing.T) { + t.Run("no tasks", func(t *testing.T) { + tm := newTaskManager() + assert.False(t, tm.hasBusyForegroundTaskExcept(nil)) + }) + + t.Run("a busy foreground task counts", func(t *testing.T) { + tm := newTaskManager() + tm.NewTask(false) + assert.True(t, tm.hasBusyForegroundTaskExcept(nil)) + }) + + t.Run("a busy background task does not count", func(t *testing.T) { + tm := newTaskManager() + tm.NewTask(true) + assert.False(t, tm.hasBusyForegroundTaskExcept(nil)) + }) + + t.Run("a done foreground task does not count", func(t *testing.T) { + tm := newTaskManager() + task := tm.NewTask(false) + task.Done() + assert.False(t, tm.hasBusyForegroundTaskExcept(nil)) + }) + + t.Run("a paused foreground task does not count", func(t *testing.T) { + tm := newTaskManager() + task := tm.NewTask(false) + task.Pause() + assert.False(t, tm.hasBusyForegroundTaskExcept(nil)) + }) + + t.Run("the ignored task does not count", func(t *testing.T) { + tm := newTaskManager() + task := tm.NewTask(false) + assert.False(t, tm.hasBusyForegroundTaskExcept(task)) + }) + + t.Run("another foreground task counts even when one is ignored", func(t *testing.T) { + tm := newTaskManager() + ignored := tm.NewTask(false) + tm.NewTask(false) + assert.True(t, tm.hasBusyForegroundTaskExcept(ignored)) + }) + + t.Run("only a background task alongside the ignored current event", func(t *testing.T) { + // This is the repo-switch case: the switch is handled as the current + // event (ignored) while a background refresh is in flight; it must not + // be considered busy. + tm := newTaskManager() + current := tm.NewTask(false) + tm.NewTask(true) + assert.False(t, tm.hasBusyForegroundTaskExcept(current)) + }) +} diff --git a/pkg/gui/background.go b/pkg/gui/background.go index 94bf4f678..8633f4624 100644 --- a/pkg/gui/background.go +++ b/pkg/gui/background.go @@ -114,7 +114,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) + }, nil, true) } return self.backgroundFetch() @@ -133,7 +133,7 @@ func (self *BackgroundRoutineMgr) startBackgroundFilesRefresh() { userConfig := self.gui.UserConfig() self.goEvery(userConfig.Refresher.RefreshIntervalDuration(), self.gui.stopChan, func(_ bool) error { - self.gui.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.FILES}, Background: true}) + self.gui.c.RefreshFromWorker(types.RefreshOptions{Scope: []types.RefreshableView{types.FILES}, Background: true}) return nil }) } @@ -184,7 +184,7 @@ func (self *BackgroundRoutineMgr) checkForExternalChanges() { // No need to update the stored snapshot here; Refresh does that. self.gui.c.Log.Info("External ref change detected — refreshing") - self.gui.c.Refresh(types.RefreshOptions{Background: true}) + self.gui.c.RefreshFromWorker(types.RefreshOptions{Background: true}) } // returns a channel that can be used to trigger the callback immediately @@ -198,7 +198,10 @@ func (self *BackgroundRoutineMgr) goEvery(interval time.Duration, stop chan stru if self.backgroundRefreshesPaused() { return } - self.gui.c.OnWorker(func(gocui.Task) error { + // OnWorkerBackground, not OnWorker: these routines and the refreshes + // they trigger must not count towards lazygit being busy, or they'd + // spuriously block a repo switch every time one happens to be running. + self.gui.c.OnWorkerBackground(func(gocui.Task) error { _ = function(retriggered) done <- struct{}{} return nil diff --git a/pkg/gui/context/local_commits_context.go b/pkg/gui/context/local_commits_context.go index 056035cce..d929aca88 100644 --- a/pkg/gui/context/local_commits_context.go +++ b/pkg/gui/context/local_commits_context.go @@ -4,6 +4,7 @@ import ( "fmt" "log" "strings" + "sync/atomic" "time" "github.com/jesseduffield/lazygit/pkg/commands/models" @@ -142,7 +143,9 @@ type LocalCommitsViewModel struct { // If this is true we limit the amount of commits we load, for the sake of keeping things fast. // If the user attempts to scroll past the end of the list, we will load more commits. - limitCommits bool + // Atomic because a checkout or reset sets it from a worker goroutine while the + // commits refresh reads it on the UI thread to decide how many commits to load. + limitCommits atomic.Bool // If this is true we'll use git log --all when fetching the commits. showWholeGitGraph bool @@ -151,9 +154,9 @@ type LocalCommitsViewModel struct { func NewLocalCommitsViewModel(getModel func() []*models.Commit, c *ContextCommon) *LocalCommitsViewModel { self := &LocalCommitsViewModel{ ListViewModel: NewListViewModel(getModel), - limitCommits: true, showWholeGitGraph: c.UserConfig().Git.Log.ShowWholeGraph, } + self.limitCommits.Store(true) return self } @@ -225,11 +228,11 @@ func (self *LocalCommitsContext) ModelSearchResults(searchStr string, caseSensit } func (self *LocalCommitsViewModel) SetLimitCommits(value bool) { - self.limitCommits = value + self.limitCommits.Store(value) } func (self *LocalCommitsViewModel) GetLimitCommits() bool { - return self.limitCommits + return self.limitCommits.Load() } func (self *LocalCommitsViewModel) SetShowWholeGitGraph(value bool) { diff --git a/pkg/gui/controllers/bisect_controller.go b/pkg/gui/controllers/bisect_controller.go index 1066237c1..eb568240b 100644 --- a/pkg/gui/controllers/bisect_controller.go +++ b/pkg/gui/controllers/bisect_controller.go @@ -274,10 +274,11 @@ func (self *BisectController) afterMark(selectCurrent bool, waitToReselect bool) } func (self *BisectController) afterBisectMarkRefresh(selectCurrent bool, waitToReselect bool) error { - selectFn := func() { + selectFn := func() error { if selectCurrent { self.selectCurrentBisectCommit() } + return nil } if waitToReselect { @@ -285,7 +286,9 @@ func (self *BisectController) afterBisectMarkRefresh(selectCurrent bool, waitToR return nil } - selectFn() + if err := selectFn(); err != nil { + return err + } self.c.Helpers().Bisect.PostBisectCommandRefresh() return nil diff --git a/pkg/gui/controllers/branches_controller.go b/pkg/gui/controllers/branches_controller.go index 45f98e9c5..a886a410b 100644 --- a/pkg/gui/controllers/branches_controller.go +++ b/pkg/gui/controllers/branches_controller.go @@ -599,11 +599,11 @@ func (self *BranchesController) createNewBranchWithName(newBranchName string) er return err } - self.c.Helpers().Refs.SelectFirstBranchAndFirstCommit() self.c.Refresh(types.RefreshOptions{ - Mode: types.ASYNC, - KeepBranchSelectionIndex: true, - CommitSelection: types.KeepCommitSelectionIndex, + Mode: types.ASYNC, + BranchSelection: types.SelectCheckedOutBranch, + CommitSelection: types.SelectHeadCommit, + SelectTopReflogCommit: true, }) return nil } @@ -710,9 +710,9 @@ func (self *BranchesController) fastForward(branch *models.Branch) error { } action := self.c.Tr.Actions.FastForwardBranch + worktree, ok := self.worktreeForBranch(branch) return self.c.WithInlineStatus(branch, types.ItemOperationFastForwarding, context.LOCAL_BRANCHES_CONTEXT_KEY, func(task gocui.Task) error { - worktree, ok := self.worktreeForBranch(branch) if ok { self.c.LogAction(action) @@ -734,7 +734,7 @@ func (self *BranchesController) fastForward(branch *models.Branch) error { WorktreePath: worktreePath, }, ) - self.c.Refresh(types.RefreshOptions{Mode: types.SYNC}) + self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.SYNC}) return err } @@ -743,7 +743,7 @@ func (self *BranchesController) fastForward(branch *models.Branch) error { err := self.c.Git().Sync.FastForward( task, branch.Name, branch.UpstreamRemote, branch.UpstreamBranch, ) - self.c.Refresh(types.RefreshOptions{Mode: types.SYNC, Scope: []types.RefreshableView{types.BRANCHES}}) + self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.SYNC, Scope: []types.RefreshableView{types.BRANCHES}}) return err }) } @@ -783,20 +783,25 @@ func (self *BranchesController) rename(branch *models.Branch) error { return err } - // need to find where the branch is now so that we can re-select it. That means we need to refetch the branches synchronously and then find our branch + // need to find where the branch is now so that we can re-select it. That means we need to + // refetch the branches and then find our branch. The branches model update is bounced + // onto the UI thread, so the re-selection (which reads Model.Branches) has to run in + // Then; reading it inline here would see the previous model. self.c.Refresh(types.RefreshOptions{ Mode: types.SYNC, Scope: []types.RefreshableView{types.BRANCHES, types.WORKTREES}, + Then: func() error { + // now that we've got our stuff again we need to find that branch and reselect it. + for i, newBranch := range self.c.Model().Branches { + if newBranch.Name == newBranchName { + self.context().SetSelection(i) + self.context().HandleRender() + } + } + return nil + }, }) - // now that we've got our stuff again we need to find that branch and reselect it. - for i, newBranch := range self.c.Model().Branches { - if newBranch.Name == newBranchName { - self.context().SetSelection(i) - self.context().HandleRender() - } - } - return nil }, }) diff --git a/pkg/gui/controllers/commits_files_controller.go b/pkg/gui/controllers/commits_files_controller.go index f9fda0b93..b90e14b74 100644 --- a/pkg/gui/controllers/commits_files_controller.go +++ b/pkg/gui/controllers/commits_files_controller.go @@ -337,6 +337,8 @@ func (self *CommitFilesController) discard(selectedNodes []*filetree.CommitFileN Title: self.c.Tr.DiscardFileChangesTitle, Prompt: prompt, HandleConfirm: func() error { + commits := self.c.Model().Commits + selectedLineIdx := self.c.Contexts().LocalCommits.GetSelectedLineIdx() return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func(gocui.Task) error { var filePaths []string selectedNodes = normalisedSelectedCommitFileNodes(selectedNodes) @@ -356,14 +358,17 @@ func (self *CommitFilesController) discard(selectedNodes []*filetree.CommitFileN }) } - err := self.c.Git().Rebase.DiscardOldFileChanges(self.c.Model().Commits, self.c.Contexts().LocalCommits.GetSelectedLineIdx(), filePaths) + err := self.c.Git().Rebase.DiscardOldFileChanges(commits, selectedLineIdx, filePaths) if err := self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(err); err != nil { return err } - if self.context().RangeSelectEnabled() { - self.context().GetList().CancelRangeSelect() - } + self.c.OnUIThread(func() error { + if self.context().RangeSelectEnabled() { + self.context().GetList().CancelRangeSelect() + } + return nil + }) return nil }) @@ -442,20 +447,16 @@ func (self *CommitFilesController) toggleForPatch(selectedNodes []*filetree.Comm self.c.UserConfig().Keybinding.Universal.IncreaseContextInDiffView) } + refName := self.context().GetRef().RefName() + toggle := func() error { return self.c.WithWaitingStatus(self.c.Tr.UpdatingPatch, func(gocui.Task) error { - if !self.c.Git().Patch.PatchBuilder.Active() { - if err := self.startPatchBuilder(); err != nil { - return err - } - } - selectedNodes = normalisedSelectedCommitFileNodes(selectedNodes) // Find if any file in the selection is unselected or partially added adding := lo.SomeBy(selectedNodes, func(node *filetree.CommitFileNode) bool { return node.SomeFile(func(file *models.CommitFile) bool { - fileStatus := self.c.Git().Patch.PatchBuilder.GetFileStatus(file.Path, self.context().GetRef().RefName()) + fileStatus := self.c.Git().Patch.PatchBuilder.GetFileStatus(file.Path, refName) return fileStatus == patch.PART || fileStatus == patch.UNSELECTED }) }) @@ -498,6 +499,12 @@ func (self *CommitFilesController) toggleForPatch(selectedNodes []*filetree.Comm self.c.Git().Patch.PatchBuilder.Reset() } + if !self.c.Git().Patch.PatchBuilder.Active() { + if err := self.startPatchBuilder(); err != nil { + return err + } + } + return toggle() }, }) diff --git a/pkg/gui/controllers/custom_patch_options_menu_action.go b/pkg/gui/controllers/custom_patch_options_menu_action.go index cabba4739..3d15ce899 100644 --- a/pkg/gui/controllers/custom_patch_options_menu_action.go +++ b/pkg/gui/controllers/custom_patch_options_menu_action.go @@ -132,10 +132,11 @@ func (self *CustomPatchOptionsMenuAction) returnFocusFromPatchExplorerIfNecessar func (self *CustomPatchOptionsMenuAction) handleDeletePatchFromCommit() error { self.returnFocusFromPatchExplorerIfNecessary() + commits := self.c.Model().Commits + commitIndex := self.getPatchCommitIndex() return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func(gocui.Task) error { - commitIndex := self.getPatchCommitIndex() self.c.LogAction(self.c.Tr.Actions.RemovePatchFromCommit) - err := self.c.Git().Patch.DeletePatchesFromCommit(self.c.Model().Commits, commitIndex) + err := self.c.Git().Patch.DeletePatchesFromCommit(commits, commitIndex) return self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(err) }) } @@ -143,10 +144,12 @@ func (self *CustomPatchOptionsMenuAction) handleDeletePatchFromCommit() error { func (self *CustomPatchOptionsMenuAction) handleMovePatchToSelectedCommit() error { self.returnFocusFromPatchExplorerIfNecessary() + commits := self.c.Model().Commits + commitIndex := self.getPatchCommitIndex() + toCommitIndex := self.c.Contexts().LocalCommits.GetSelectedLineIdx() return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func(gocui.Task) error { - commitIndex := self.getPatchCommitIndex() self.c.LogAction(self.c.Tr.Actions.MovePatchToSelectedCommit) - err := self.c.Git().Patch.MovePatchToSelectedCommit(self.c.Model().Commits, commitIndex, self.c.Contexts().LocalCommits.GetSelectedLineIdx()) + err := self.c.Git().Patch.MovePatchToSelectedCommit(commits, commitIndex, toCommitIndex) return self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(err) }) } @@ -159,10 +162,11 @@ func (self *CustomPatchOptionsMenuAction) handleMovePatchIntoWorkingTree() error Title: self.c.Tr.MustStashTitle, Prompt: self.c.Tr.MustStashWarning, HandleConfirm: func() error { + commits := self.c.Model().Commits + commitIndex := self.getPatchCommitIndex() return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func(gocui.Task) error { - commitIndex := self.getPatchCommitIndex() self.c.LogAction(self.c.Tr.Actions.MovePatchIntoIndex) - err := self.c.Git().Patch.MovePatchIntoIndex(self.c.Model().Commits, commitIndex, mustStash) + err := self.c.Git().Patch.MovePatchIntoIndex(commits, commitIndex, mustStash) return self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(err) }) }, @@ -183,14 +187,18 @@ func (self *CustomPatchOptionsMenuAction) handlePullPatchIntoNewCommit() error { DescriptionTitle: self.c.Tr.CommitDescriptionTitle, PreserveMessage: false, OnConfirm: func(summary string, description string) error { + commits := self.c.Model().Commits + self.c.Helpers().Commits.CloseCommitMessagePanel() return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func(gocui.Task) error { - self.c.Helpers().Commits.CloseCommitMessagePanel() self.c.LogAction(self.c.Tr.Actions.MovePatchIntoNewCommit) - err := self.c.Git().Patch.PullPatchIntoNewCommit(self.c.Model().Commits, commitIndex, summary, description) + err := self.c.Git().Patch.PullPatchIntoNewCommit(commits, commitIndex, summary, description) if err := self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(err); err != nil { return err } - self.c.Context().Push(self.c.Contexts().LocalCommits, types.OnFocusOpts{}) + self.c.OnUIThread(func() error { + self.c.Context().Push(self.c.Contexts().LocalCommits, types.OnFocusOpts{}) + return nil + }) return nil }) }, @@ -214,14 +222,18 @@ func (self *CustomPatchOptionsMenuAction) handlePullPatchIntoNewCommitBefore() e DescriptionTitle: self.c.Tr.CommitDescriptionTitle, PreserveMessage: false, OnConfirm: func(summary string, description string) error { + commits := self.c.Model().Commits + self.c.Helpers().Commits.CloseCommitMessagePanel() return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func(gocui.Task) error { - self.c.Helpers().Commits.CloseCommitMessagePanel() self.c.LogAction(self.c.Tr.Actions.MovePatchIntoNewCommit) - err := self.c.Git().Patch.PullPatchIntoNewCommitBefore(self.c.Model().Commits, commitIndex, summary, description) + err := self.c.Git().Patch.PullPatchIntoNewCommitBefore(commits, commitIndex, summary, description) if err := self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(err); err != nil { return err } - self.c.Context().Push(self.c.Contexts().LocalCommits, types.OnFocusOpts{}) + self.c.OnUIThread(func() error { + self.c.Context().Push(self.c.Contexts().LocalCommits, types.OnFocusOpts{}) + return nil + }) return nil }) }, diff --git a/pkg/gui/controllers/files_controller.go b/pkg/gui/controllers/files_controller.go index a63c6a15a..b70b67ab7 100644 --- a/pkg/gui/controllers/files_controller.go +++ b/pkg/gui/controllers/files_controller.go @@ -44,7 +44,7 @@ func (self *FilesController) GetKeybindings(opts types.KeybindingsOpts) []*types { Keys: opts.GetKeys(opts.Config.Universal.Select), Handler: self.withItems(self.press), - GetDisabledReason: self.require(self.withFileTreeViewModelMutex(self.itemsSelected(self.canStageSelection))), + GetDisabledReason: self.require(self.itemsSelected(self.canStageSelection)), Description: self.c.Tr.Stage, Tooltip: self.c.Tr.StageTooltip, DisplayOnScreen: true, @@ -91,7 +91,7 @@ func (self *FilesController) GetKeybindings(opts types.KeybindingsOpts) []*types { Keys: opts.GetKeys(opts.Config.Universal.Edit), Handler: self.withItems(self.edit), - GetDisabledReason: self.require(self.withFileTreeViewModelMutex(self.itemsSelected(self.canEditFiles))), + GetDisabledReason: self.require(self.itemsSelected(self.canEditFiles)), Description: self.c.Tr.Edit, Tooltip: self.c.Tr.EditFileTooltip, DisplayOnScreen: true, @@ -145,7 +145,7 @@ func (self *FilesController) GetKeybindings(opts types.KeybindingsOpts) []*types { Keys: opts.GetKeys(opts.Config.Universal.Remove), Handler: self.withItems(self.remove), - GetDisabledReason: self.withFileTreeViewModelMutex(self.require(self.itemsSelected(self.canRemove))), + GetDisabledReason: self.require(self.itemsSelected(self.canRemove)), Description: self.c.Tr.Discard, Tooltip: self.c.Tr.DiscardFileChangesTooltip, OpensMenu: true, @@ -182,7 +182,7 @@ func (self *FilesController) GetKeybindings(opts types.KeybindingsOpts) []*types Handler: self.withItems(self.openMergeConflictMenu), Description: self.c.Tr.ViewMergeConflictOptions, Tooltip: self.c.Tr.ViewMergeConflictOptionsTooltip, - GetDisabledReason: self.require(self.withFileTreeViewModelMutex(self.itemsSelected(self.canOpenMergeConflictMenu))), + GetDisabledReason: self.require(self.itemsSelected(self.canOpenMergeConflictMenu)), OpensMenu: true, DisplayOnScreen: true, }, @@ -209,15 +209,6 @@ func (self *FilesController) GetKeybindings(opts types.KeybindingsOpts) []*types } } -func (self *FilesController) withFileTreeViewModelMutex(callback func() *types.DisabledReason) func() *types.DisabledReason { - return func() *types.DisabledReason { - self.c.Contexts().Files.FileTreeViewModel.RWMutex.RLock() - defer self.c.Contexts().Files.FileTreeViewModel.RWMutex.RUnlock() - - return callback() - } -} - func (self *FilesController) GetMouseKeybindings(opts types.KeybindingsOpts) []*gocui.ViewMouseBinding { return []*gocui.ViewMouseBinding{ { @@ -574,11 +565,6 @@ func (self *FilesController) toggleStaged( } func (self *FilesController) pressWithLock(selectedNodes []*filetree.FileNode) error { - // Obtaining this lock because optimistic rendering requires us to mutate - // the files in our model. - self.c.Mutexes().RefreshingFilesMutex.Lock() - defer self.c.Mutexes().RefreshingFilesMutex.Unlock() - // When filtering, expand directory nodes to individual visible file paths // so that only filtered files are staged/unstaged. toPaths := func(nodes []*filetree.FileNode) []string { @@ -942,9 +928,6 @@ func (self *FilesController) toggleStagedAll() error { } func (self *FilesController) toggleStagedAllWithLock() error { - self.c.Mutexes().RefreshingFilesMutex.Lock() - defer self.c.Mutexes().RefreshingFilesMutex.Unlock() - root := self.context().FileTreeViewModel.GetRoot() stage := func(unstagedNodes []*filetree.FileNode) error { @@ -1808,10 +1791,10 @@ func (self *FilesController) remove(selectedNodes []*filetree.FileNode) error { } func (self *FilesController) ResetSubmodule(submodule *models.SubmoduleConfig) error { + file := self.c.Helpers().WorkingTree.FileForSubmodule(submodule) return self.c.WithWaitingStatus(self.c.Tr.ResettingSubmoduleStatus, func(gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.ResetSubmodule) - file := self.c.Helpers().WorkingTree.FileForSubmodule(submodule) if file != nil { if err := self.c.Git().WorkingTree.UnStageFile(file.Names(), file.Tracked); err != nil { return err @@ -1825,7 +1808,7 @@ func (self *FilesController) ResetSubmodule(submodule *models.SubmoduleConfig) e return err } - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.FILES, types.SUBMODULES}}) + self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.FILES, types.SUBMODULES}}) return nil }) } diff --git a/pkg/gui/controllers/filtering_menu_action.go b/pkg/gui/controllers/filtering_menu_action.go index 01a236f7a..7ae26c4ef 100644 --- a/pkg/gui/controllers/filtering_menu_action.go +++ b/pkg/gui/controllers/filtering_menu_action.go @@ -122,9 +122,10 @@ func (self *FilteringMenuAction) setFiltering() error { self.c.Context().Push(self.c.Contexts().LocalCommits, types.OnFocusOpts{}) - self.c.Refresh(types.RefreshOptions{Scope: helpers.ScopesToRefreshWhenFilteringModeChanges(), Then: func() { + self.c.Refresh(types.RefreshOptions{Scope: helpers.ScopesToRefreshWhenFilteringModeChanges(), Then: func() error { self.c.Contexts().LocalCommits.SetSelection(0) self.c.Contexts().LocalCommits.HandleFocus(types.OnFocusOpts{}) + return nil }}) return nil diff --git a/pkg/gui/controllers/helpers/app_status_helper.go b/pkg/gui/controllers/helpers/app_status_helper.go index b691db4a4..90b87b3b8 100644 --- a/pkg/gui/controllers/helpers/app_status_helper.go +++ b/pkg/gui/controllers/helpers/app_status_helper.go @@ -34,7 +34,12 @@ func (self *AppStatusHelper) Toast(message string, kind types.ToastKind) { self.statusMgr().AddToastStatus(message, kind) - self.renderAppStatus() + // 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) } // A custom task for WithWaitingStatus calls; it wraps the original one and @@ -61,11 +66,14 @@ 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) + return self.WithWaitingStatusImpl(message, f, task, false) }) } -func (self *AppStatusHelper) WithWaitingStatusImpl(message string, f func(gocui.Task) error, task gocui.Task) error { +// 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 { // 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 @@ -73,7 +81,7 @@ func (self *AppStatusHelper) WithWaitingStatusImpl(message string, f func(gocui. self.c.PauseBackgroundRefreshes(true) defer self.c.PauseBackgroundRefreshes(false) - return self.statusMgr().WithWaitingStatus(message, self.renderAppStatus, func(waitingStatusHandle *status.WaitingStatusHandle) error { + return self.statusMgr().WithWaitingStatus(message, func() { self.renderAppStatus(background) }, func(waitingStatusHandle *status.WaitingStatusHandle) error { return f(appStatusHelperTask{task, waitingStatusHandle}) }) } @@ -100,21 +108,33 @@ func (self *AppStatusHelper) GetStatusString() string { return appStatus } -func (self *AppStatusHelper) renderAppStatus() { - self.c.OnWorker(func(_ gocui.Task) error { +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 + } + + 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()) - update := self.c.OnUIThreadContentOnly + update := 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 = onUIThread } update(func() error { self.c.Views().AppStatus.FgColor = color diff --git a/pkg/gui/controllers/helpers/branches_helper.go b/pkg/gui/controllers/helpers/branches_helper.go index 4283bd29a..5c72bacfd 100644 --- a/pkg/gui/controllers/helpers/branches_helper.go +++ b/pkg/gui/controllers/helpers/branches_helper.go @@ -45,8 +45,11 @@ func (self *BranchesHelper) ConfirmLocalDelete(branches []*models.Branch) error return err } - self.c.Contexts().Branches.CollapseRangeSelectionToTop() - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.BRANCHES}}) + self.c.OnUIThread(func() error { + self.c.Contexts().Branches.CollapseRangeSelectionToTop() + return nil + }) + self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.BRANCHES}}) return nil }) }) @@ -84,9 +87,12 @@ func (self *BranchesHelper) ConfirmDeleteRemote(remoteBranches []*models.RemoteB if err := self.deleteRemoteBranches(remoteBranches, task); err != nil { return err } - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.BRANCHES, types.REMOTES}}) + self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.BRANCHES, types.REMOTES}}) if resetRemoteBranchesSelection { - self.c.Contexts().RemoteBranches.CollapseRangeSelectionToTop() + self.c.OnUIThread(func() error { + self.c.Contexts().RemoteBranches.CollapseRangeSelectionToTop() + return nil + }) } return nil }) @@ -151,8 +157,11 @@ func (self *BranchesHelper) ConfirmLocalAndRemoteDelete(branches []*models.Branc return err } - self.c.Contexts().Branches.CollapseRangeSelectionToTop() - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.BRANCHES, types.REMOTES}}) + self.c.OnUIThread(func() error { + self.c.Contexts().Branches.CollapseRangeSelectionToTop() + return nil + }) + self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.BRANCHES, types.REMOTES}}) return nil }) }, @@ -311,8 +320,11 @@ func (self *BranchesHelper) deleteLocalBranchesContinuation(branches []*models.B return err } - self.c.Contexts().Branches.CollapseRangeSelectionToTop() - self.c.Refresh(types.RefreshOptions{ + self.c.OnUIThread(func() error { + self.c.Contexts().Branches.CollapseRangeSelectionToTop() + return nil + }) + self.c.RefreshFromWorker(types.RefreshOptions{ Mode: types.ASYNC, Scope: []types.RefreshableView{types.WORKTREES, types.BRANCHES, types.FILES}, }) @@ -329,8 +341,11 @@ func (self *BranchesHelper) deleteLocalAndRemoteBranchesContinuation(branches [] return err } - self.c.Contexts().Branches.CollapseRangeSelectionToTop() - self.c.Refresh(types.RefreshOptions{ + self.c.OnUIThread(func() error { + self.c.Contexts().Branches.CollapseRangeSelectionToTop() + return nil + }) + self.c.RefreshFromWorker(types.RefreshOptions{ Mode: types.ASYNC, Scope: []types.RefreshableView{types.WORKTREES, types.BRANCHES, types.REMOTES, types.FILES}, }) @@ -387,14 +402,31 @@ func (self *BranchesHelper) PostFetchRefresh(fetchErr error, background bool) er if self.c.UserConfig().Git.AutoForwardBranches != "none" { scope = append(scope, types.WORKTREES) } - self.c.Refresh(types.RefreshOptions{Scope: scope, Mode: types.SYNC, Background: background}) - if fetchErr != nil { - return fetchErr - } - return self.AutoForwardBranches() + // AutoForwardBranches reads Model.Branches, which the branches refresh writes + // via a bounce, so it has to run in Then rather than right after Refresh + // returns (where it would still see the previous branches). + self.c.RefreshFromWorker(types.RefreshOptions{ + Scope: scope, + Mode: types.SYNC, + Background: background, + Then: func() error { + if fetchErr != nil { + return nil + } + err := self.AutoForwardBranches(background) + if background && err != nil { + // The background poller discards this return value, so surface + // the error in the log rather than as a popup for background work. + self.c.Log.Error(err) + return nil + } + return err + }, + }) + return fetchErr } -func (self *BranchesHelper) AutoForwardBranches() error { +func (self *BranchesHelper) AutoForwardBranches(background bool) error { if self.c.UserConfig().Git.AutoForwardBranches == "none" { return nil } @@ -426,7 +458,7 @@ func (self *BranchesHelper) AutoForwardBranches() error { self.c.LogCommand(strings.TrimRight(updateCommands, "\n"), false) err := self.c.Git().Branch.UpdateBranchRefs(updateCommands) - self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.BRANCHES}, Mode: types.SYNC}) + self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.BRANCHES}, Mode: types.SYNC, Background: background}) return err } diff --git a/pkg/gui/controllers/helpers/cherry_pick_helper.go b/pkg/gui/controllers/helpers/cherry_pick_helper.go index e2fe46545..673f657f5 100644 --- a/pkg/gui/controllers/helpers/cherry_pick_helper.go +++ b/pkg/gui/controllers/helpers/cherry_pick_helper.go @@ -95,7 +95,7 @@ func (self *CherryPickHelper) Paste() error { cherryPickedCommits := self.getData().CherryPickedCommits result := self.c.Git().Rebase.CherryPickCommits(cherryPickedCommits) - err := self.rebaseHelper.CheckMergeOrRebaseWithRefreshOptions(result, types.RefreshOptions{Mode: types.SYNC}) + err := self.rebaseHelper.CheckMergeOrRebaseWithRefreshOptionsFromUIThread(result, types.RefreshOptions{Mode: types.SYNC}) if err != nil { return result } diff --git a/pkg/gui/controllers/helpers/gpg_helper.go b/pkg/gui/controllers/helpers/gpg_helper.go index fb8fae628..fd74a400b 100644 --- a/pkg/gui/controllers/helpers/gpg_helper.go +++ b/pkg/gui/controllers/helpers/gpg_helper.go @@ -88,7 +88,7 @@ func (self *GpgHelper) runAndStream( ) error { return self.c.WithWaitingStatus(waitingStatus, func(gocui.Task) error { if err := cmdObj.StreamOutput().Run(); err != nil { - self.c.Refresh(failureRefreshOptions) + self.c.RefreshFromWorker(failureRefreshOptions) return fmt.Errorf( self.c.Tr.GitCommandFailed, self.c.UserConfig().Keybinding.Universal.ExtrasMenu, ) @@ -100,7 +100,7 @@ func (self *GpgHelper) runAndStream( } } - self.c.Refresh(successRefreshOptions) + self.c.RefreshFromWorker(successRefreshOptions) return nil }) } diff --git a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go index 847b89893..b0c53b831 100644 --- a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go +++ b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go @@ -79,7 +79,9 @@ func (self *MergeAndRebaseHelper) ContinueRebase() error { } func (self *MergeAndRebaseHelper) genericMergeCommand(command string) error { - return self.genericMergeCommandImpl(command, true) + // The menu/prompt/confirm handlers that reach here run on the UI thread and + // spin up a worker (via the waiting status below) to do the actual work. + return self.genericMergeCommandImpl(command, true, false) } // genericMergeCommandImpl runs a merge/rebase continue/skip/abort and handles @@ -87,10 +89,12 @@ func (self *MergeAndRebaseHelper) genericMergeCommand(command string) error { // non-subprocess path runs on a worker with a waiting status. // // showWaitingStatus is false only for the recursive auto-skip in -// CheckMergeOrRebaseWithRefreshOptions: that call already runs on the caller's -// thread (the worker of the enclosing waiting status, or the UI thread for the -// synchronous callers), so it must not spin up a second one. -func (self *MergeAndRebaseHelper) genericMergeCommandImpl(command string, showWaitingStatus bool) error { +// checkMergeOrRebaseImpl: that call already runs on the caller's thread (the +// worker of the enclosing waiting status, or the UI thread for the synchronous +// callers), so it must not spin up a second one. calledFromWorker says which of +// those two the body runs on, so the post-action refresh picks Refresh vs +// RefreshFromWorker correctly. +func (self *MergeAndRebaseHelper) genericMergeCommandImpl(command string, showWaitingStatus bool, calledFromWorker bool) error { status := self.c.Git().Status.WorkingTreeState() if status.None() { @@ -128,29 +132,30 @@ func (self *MergeAndRebaseHelper) genericMergeCommandImpl(command string, showWa if needsSubprocess { // TODO: see if we should be calling more of the code from self.Git.Rebase.GenericMergeOrRebaseAction success, err := self.c.RunSubprocess(self.c.Git().Rebase.GenericMergeOrRebaseActionCmdObj(commandType, command)) - self.c.Refresh(types.RefreshOptions{ + self.refreshAfterMergeOrRebase(types.RefreshOptions{ Mode: types.ASYNC, CommitSelection: commitSelectionAfterMerge(success && selectHeadCommitOnSuccess), - }) + }, calledFromWorker) self.RecordWhetherMergeOrRebaseStartedInLazygit() return err } - runAction := func() error { + runAction := func(calledFromWorker bool) error { result := self.c.Git().Rebase.GenericMergeOrRebaseAction(commandType, command) - return self.CheckMergeOrRebaseWithRefreshOptions(result, + return self.checkMergeOrRebaseImpl(result, types.RefreshOptions{ Mode: types.ASYNC, CommitSelection: commitSelectionAfterMerge(result == nil && selectHeadCommitOnSuccess), - }) + }, calledFromWorker) } if showWaitingStatus { return self.c.WithWaitingStatus(status.Title(self.c.Tr), func(gocui.Task) error { - return runAction() + // The waiting status ran runAction on a worker. + return runAction(true) }) } - return runAction() + return runAction(calledFromWorker) } // commitSelectionAfterMerge maps whether a merge/rebase/pull created a new @@ -205,17 +210,34 @@ func (self *MergeAndRebaseHelper) RecordWhetherMergeOrRebaseStartedInLazygit() { self.c.Git().Status.WorkingTreeState().Any()) } +// CheckMergeOrRebaseWithRefreshOptions handles the result of a merge/rebase +// step and refreshes. It's for callers running on a worker (the +// WithWaitingStatus / WithInlineStatus handlers), which is the large majority; +// UI-thread callers use CheckMergeOrRebaseWithRefreshOptionsFromUIThread. func (self *MergeAndRebaseHelper) CheckMergeOrRebaseWithRefreshOptions(result error, refreshOptions types.RefreshOptions) error { - self.c.Refresh(refreshOptions) + return self.checkMergeOrRebaseImpl(result, refreshOptions, true) +} + +// CheckMergeOrRebaseWithRefreshOptionsFromUIThread is like +// CheckMergeOrRebaseWithRefreshOptions, but for the callers that run the +// merge/rebase synchronously on the UI thread (the WithWaitingStatusSync +// move/revert/squash-fixups/cherry-pick-paste/patch-discard handlers, kept sync +// so rapid key presses batch) rather than on a worker. +func (self *MergeAndRebaseHelper) CheckMergeOrRebaseWithRefreshOptionsFromUIThread(result error, refreshOptions types.RefreshOptions) error { + return self.checkMergeOrRebaseImpl(result, refreshOptions, false) +} + +func (self *MergeAndRebaseHelper) checkMergeOrRebaseImpl(result error, refreshOptions types.RefreshOptions, calledFromWorker bool) error { + self.refreshAfterMergeOrRebase(refreshOptions, calledFromWorker) self.RecordWhetherMergeOrRebaseStartedInLazygit() if result == nil { return nil } else if strings.Contains(result.Error(), "No changes - did you forget to use") { - return self.genericMergeCommandImpl(REBASE_OPTION_SKIP, false) + return self.genericMergeCommandImpl(REBASE_OPTION_SKIP, false, calledFromWorker) } else if strings.Contains(result.Error(), "The previous cherry-pick is now empty") { - return self.genericMergeCommandImpl(REBASE_OPTION_SKIP, false) + return self.genericMergeCommandImpl(REBASE_OPTION_SKIP, false, calledFromWorker) } else if strings.Contains(result.Error(), "No rebase in progress?") { // assume in this case that we're already done return nil @@ -223,6 +245,18 @@ func (self *MergeAndRebaseHelper) CheckMergeOrRebaseWithRefreshOptions(result er return self.CheckForConflicts(result) } +// refreshAfterMergeOrRebase issues the post-action refresh on the entry point +// that matches the thread the merge/rebase ran on: RefreshFromWorker for the +// worker callers, Refresh for the ones that stayed synchronously on the UI +// thread. +func (self *MergeAndRebaseHelper) refreshAfterMergeOrRebase(refreshOptions types.RefreshOptions, calledFromWorker bool) { + if calledFromWorker { + self.c.RefreshFromWorker(refreshOptions) + } else { + self.c.Refresh(refreshOptions) + } +} + func (self *MergeAndRebaseHelper) CheckMergeOrRebase(result error) error { return self.CheckMergeOrRebaseWithRefreshOptions(result, types.RefreshOptions{Mode: types.ASYNC}) } @@ -307,29 +341,36 @@ func (self *MergeAndRebaseHelper) PromptToContinueRebase() error { // Need to refresh the files to be really sure if this is the case. // We would otherwise be relying on lazygit's auto-refresh on focus, // but this is not supported by all terminals or on all platforms. + // + // The model.Files update is bounced onto the UI thread, so we have + // to read it in Then; reading it inline here would see the previous + // model. self.c.Refresh(types.RefreshOptions{ Mode: types.SYNC, Scope: []types.RefreshableView{types.FILES}, + Then: func() error { + unstagedFiles := GetUnstagedFilesExceptSubmodules(self.c.Model().Files, self.c.Model().Submodules) + if len(unstagedFiles) > 0 { + self.c.Confirm(types.ConfirmOpts{ + Title: self.c.Tr.Continue, + Prompt: self.c.Tr.UnstagedFilesAfterConflictsResolved, + HandleConfirm: func() error { + self.c.LogAction(self.c.Tr.Actions.StageAllFiles) + if err := self.c.Git().WorkingTree.StageFiles(unstagedFiles, []string{}); err != nil { + return err + } + + return self.genericMergeCommand(REBASE_OPTION_CONTINUE) + }, + }) + + return nil + } + + return self.genericMergeCommand(REBASE_OPTION_CONTINUE) + }, }) - unstagedFiles := GetUnstagedFilesExceptSubmodules(self.c.Model().Files, self.c.Model().Submodules) - if len(unstagedFiles) > 0 { - self.c.Confirm(types.ConfirmOpts{ - Title: self.c.Tr.Continue, - Prompt: self.c.Tr.UnstagedFilesAfterConflictsResolved, - HandleConfirm: func() error { - self.c.LogAction(self.c.Tr.Actions.StageAllFiles) - if err := self.c.Git().WorkingTree.StageFiles(unstagedFiles, []string{}); err != nil { - return err - } - - return self.genericMergeCommand(REBASE_OPTION_CONTINUE) - }, - }) - - return nil - } - - return self.genericMergeCommand(REBASE_OPTION_CONTINUE) + return nil }, }) @@ -383,8 +424,8 @@ func (self *MergeAndRebaseHelper) RebaseOntoRef(ref string) error { DisabledReason: disabledReason, OnPress: func() error { self.c.LogAction(self.c.Tr.Actions.RebaseBranch) + baseCommit := self.c.Modes().MarkedBaseCommit.GetHash() return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func(task gocui.Task) error { - baseCommit := self.c.Modes().MarkedBaseCommit.GetHash() var err error if baseCommit != "" { err = self.c.Git().Rebase.RebaseBranchFromBaseCommit(ref, baseCommit) @@ -393,7 +434,9 @@ func (self *MergeAndRebaseHelper) RebaseOntoRef(ref string) error { } err = self.CheckMergeOrRebase(err) if err == nil { - return self.ResetMarkedBaseCommit() + self.c.OnUIThread(func() error { + return self.ResetMarkedBaseCommit() + }) } return err }) @@ -408,8 +451,8 @@ func (self *MergeAndRebaseHelper) RebaseOntoRef(ref string) error { Tooltip: self.c.Tr.InteractiveRebaseTooltip, OnPress: func() error { self.c.LogAction(self.c.Tr.Actions.RebaseBranch) + baseCommit := self.c.Modes().MarkedBaseCommit.GetHash() return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func(task gocui.Task) error { - baseCommit := self.c.Modes().MarkedBaseCommit.GetHash() var err error if baseCommit != "" { err = self.c.Git().Rebase.EditRebaseFromBaseCommit(ref, baseCommit) @@ -419,10 +462,13 @@ func (self *MergeAndRebaseHelper) RebaseOntoRef(ref string) error { if err = self.CheckMergeOrRebase(err); err != nil { return err } - if err = self.ResetMarkedBaseCommit(); err != nil { - return err - } - self.c.Context().Push(self.c.Contexts().LocalCommits, types.OnFocusOpts{}) + self.c.OnUIThread(func() error { + if err := self.ResetMarkedBaseCommit(); err != nil { + return err + } + self.c.Context().Push(self.c.Contexts().LocalCommits, types.OnFocusOpts{}) + return nil + }) return nil }) }, @@ -436,8 +482,8 @@ func (self *MergeAndRebaseHelper) RebaseOntoRef(ref string) error { Tooltip: self.c.Tr.RebaseOntoBaseBranchTooltip, OnPress: func() error { self.c.LogAction(self.c.Tr.Actions.RebaseBranch) + baseCommit := self.c.Modes().MarkedBaseCommit.GetHash() return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func(task gocui.Task) error { - baseCommit := self.c.Modes().MarkedBaseCommit.GetHash() var err error if baseCommit != "" { err = self.c.Git().Rebase.RebaseBranchFromBaseCommit(baseBranch, baseCommit) @@ -446,7 +492,9 @@ func (self *MergeAndRebaseHelper) RebaseOntoRef(ref string) error { } err = self.CheckMergeOrRebase(err) if err == nil { - return self.ResetMarkedBaseCommit() + self.c.OnUIThread(func() error { + return self.ResetMarkedBaseCommit() + }) } return err }) @@ -621,7 +669,7 @@ func (self *MergeAndRebaseHelper) SquashMergeCommitted(refName, checkedOutBranch if err != nil { return err } - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC}) return nil }) } diff --git a/pkg/gui/controllers/helpers/merge_conflicts_helper.go b/pkg/gui/controllers/helpers/merge_conflicts_helper.go index 6e6a01531..175bc3cc0 100644 --- a/pkg/gui/controllers/helpers/merge_conflicts_helper.go +++ b/pkg/gui/controllers/helpers/merge_conflicts_helper.go @@ -51,11 +51,17 @@ func (self *MergeConflictsHelper) resetMergeState() { self.context().GetState().Reset() } -func (self *MergeConflictsHelper) EscapeMerge() error { +func (self *MergeConflictsHelper) EscapeMerge(background bool) error { self.resetMergeState() // doing this in separate UI thread so that we're not still holding the lock by the time refresh the file - self.c.OnUIThread(func() error { + onUIThread := self.c.OnUIThread + if background { + // Reached from a background files refresh; keep it off the busy count + // (see the *Background dispatch methods) so it doesn't block a repo switch. + onUIThread = self.c.OnUIThreadBackground + } + onUIThread(func() error { // There is a race condition here: refreshing the files scope can trigger the // confirmation context to be pushed if all conflicts are resolved (prompting // to continue the merge/rebase. In that case, we don't want to then push the @@ -120,7 +126,7 @@ func (self *MergeConflictsHelper) Render() { }) } -func (self *MergeConflictsHelper) RefreshMergeState() error { +func (self *MergeConflictsHelper) RefreshMergeState(background bool) error { self.c.Contexts().MergeConflicts.GetMutex().Lock() defer self.c.Contexts().MergeConflicts.GetMutex().Unlock() @@ -134,7 +140,7 @@ func (self *MergeConflictsHelper) RefreshMergeState() error { } if !hasConflicts { - return self.EscapeMerge() + return self.EscapeMerge(background) } return nil diff --git a/pkg/gui/controllers/helpers/mode_helper.go b/pkg/gui/controllers/helpers/mode_helper.go index e44d7b01b..4947e42d1 100644 --- a/pkg/gui/controllers/helpers/mode_helper.go +++ b/pkg/gui/controllers/helpers/mode_helper.go @@ -191,7 +191,7 @@ func (self *ModeHelper) ClearFiltering() error { self.c.Refresh(types.RefreshOptions{ Scope: ScopesToRefreshWhenFilteringModeChanges(), - Then: func() { + Then: func() error { // Find the commit that was last selected in filtering mode, and select it again after refreshing if !self.c.Contexts().LocalCommits.SelectCommitByHash(selectedCommitHash) { // If we couldn't find it (either because no commit was selected @@ -202,6 +202,7 @@ func (self *ModeHelper) ClearFiltering() error { } self.c.PostRefreshUpdate(self.c.Contexts().LocalCommits) + return nil }, }) return nil diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index 3dbd19674..3c8524ffe 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -3,6 +3,7 @@ package helpers import ( "strings" "sync" + "sync/atomic" "time" "github.com/jesseduffield/generics/set" @@ -44,6 +45,15 @@ type RefreshHelper struct { // refresh that re-read refs/commits, read by the poller. refsSnapshotMutex deadlock.Mutex refsSnapshot string + + // branchLoadSeq hands out a monotonically increasing sequence number to + // each branch load (via Add, on the worker); appliedBranchLoadSeq is the + // highest sequence whose result has been written to the model (touched only + // on the UI thread, inside the bounce). Together they let a branch load's + // bounce drop its write if a later-started load has already applied, so + // concurrent branch loads don't clobber each other out of order. + branchLoadSeq atomic.Int64 + appliedBranchLoadSeq int64 } func NewRefreshHelper( @@ -69,6 +79,26 @@ func NewRefreshHelper( } func (self *RefreshHelper) Refresh(options types.RefreshOptions) { + self.performRefresh(options, false) +} + +// RefreshFromWorker is Refresh for callers already running on a worker +// goroutine (e.g. inside a WithWaitingStatus handler) rather than the UI +// thread. See IGuiCommon.RefreshFromWorker. +func (self *RefreshHelper) RefreshFromWorker(options types.RefreshOptions) { + self.performRefresh(options, true) +} + +type refreshEnv struct { + // whether this is a background refresh (which selects the dispatch variant that + // doesn't count towards lazygit being busy) + background bool + + // the repo generation captured when the refresh started + generation int +} + +func (self *RefreshHelper) performRefresh(options types.RefreshOptions, calledFromWorker bool) { if options.Mode == types.ASYNC && options.Then != nil { panic("RefreshOptions.Then doesn't work with mode ASYNC") } @@ -91,7 +121,30 @@ func (self *RefreshHelper) Refresh(options types.RefreshOptions) { ) } + // f runs on the UI thread when the refresh was initiated there, and also for + // BLOCK_UI, which dispatches f onto the UI thread regardless of the caller. + // Only a SYNC/ASYNC refresh initiated from a worker runs f on that worker. + // This, not calledFromWorker alone, is what decides whether a scope capture + // runs inline or has to hop (see captureOnUIThread). + fRunsOnUIThread := options.Mode == types.BLOCK_UI || !calledFromWorker + + // Debug-only guard: every refresh must be issued from the entry point that + // matches its goroutine — Refresh on the UI thread, RefreshFromWorker on a + // worker. We check the caller's own goroutine here, before a BLOCK_UI + // refresh dispatches f onto the UI thread, so it holds regardless of the + // mode. goid stays out of production control flow (debug only). + if self.c.GetConfig().GetDebug() && self.c.GocuiGui().IsUIThread() == calledFromWorker { + panic("Refresh called from a worker, or RefreshFromWorker called from the UI thread") + } + f := func() { + // Capture the repo generation once, here at the start, so every scope's + // bounce is guarded against the same baseline. + env := refreshEnv{ + background: options.Background, + generation: self.c.State().GetRepoGeneration(), + } + var scopeSet *set.Set[types.RefreshableView] if len(options.Scope) == 0 { // not refreshing staging/patch-building unless explicitly requested because we only need @@ -121,6 +174,8 @@ func (self *RefreshHelper) Refresh(options types.RefreshOptions) { // can move HEAD), so refresh commits + branches alongside // - submodules are refreshed as part of the files refresh // - merge conflicts are part of what the files refresh produces + // - pull requests are fetched for the tracking branches against the + // remotes, so refresh both alongside to fetch against fresh data if scopeSet.Includes(types.REFLOG) || scopeSet.Includes(types.BISECT_INFO) { scopeSet.Add(types.COMMITS, types.BRANCHES) } @@ -130,6 +185,9 @@ func (self *RefreshHelper) Refresh(options types.RefreshOptions) { if scopeSet.Includes(types.FILES) { scopeSet.Add(types.MERGE_CONFLICTS) } + if scopeSet.Includes(types.PULL_REQUESTS) { + scopeSet.Add(types.BRANCHES, types.REMOTES) + } // Capture the refs snapshot now, before we start reading git's state // below, rather than after. This is important to guard against the race @@ -144,7 +202,7 @@ func (self *RefreshHelper) Refresh(options types.RefreshOptions) { // everything happens fast and it's better to have everything update // in the one frame if !self.c.InDemo() && options.Mode == types.ASYNC { - self.c.OnWorker(func(t gocui.Task) error { + self.onWorker(env.background, func(t gocui.Task) error { f() return nil }) @@ -160,66 +218,119 @@ func (self *RefreshHelper) Refresh(options types.RefreshOptions) { } branchesAndRemotesWg := sync.WaitGroup{} + // The pull-request fetch (below) needs the just-loaded branches and + // remotes. Their model writes are bounced onto the UI thread, so the + // fetch worker can't read them back from the model without racing (and + // would see the pre-refresh values); instead the branches and remotes + // loads stash what they loaded here, and the wait on + // branchesAndRemotesWg gives the fetch the happens-before to read them. + var loadedBranches []*models.Branch + var loadedRemotes []*models.Remote includeWorktreesWithBranches := false if scopeSet.Includes(types.COMMITS) || scopeSet.Includes(types.BRANCHES) { // whenever we change commits, we should update branches because the upstream/downstream // counts can change. Whenever we change branches we should also change commits // e.g. in the case of switching branches. + // Capture the commits, reflog and branches refresh inputs (model, + // contexts, modes) on the UI thread, before the git work is dispatched + // to a worker, so the workers compute from an immutable snapshot + // instead of reading state the UI thread concurrently mutates. + var capturedCommits capturedCommitState + var capturedReflog capturedReflogState + var capturedBranches capturedBranchState + self.captureOnUIThread(fRunsOnUIThread, env.background, func() { + capturedCommits = self.captureCommitsState(options.CommitSelection) + capturedReflog = self.captureReflogState() + capturedBranches = self.captureBranchState() + }) refresh("commits and commit files", func() { - self.refreshCommitsAndCommitFiles(options.CommitSelection) + self.refreshCommitsAndCommitFiles(capturedCommits, options.CommitSelection, env) }) includeWorktreesWithBranches = scopeSet.Includes(types.WORKTREES) if self.c.UserConfig().Git.LocalBranchSortOrder == "recency" { branchesAndRemotesWg.Add(1) refresh("reflog and branches", func() { - self.refreshReflogAndBranches(includeWorktreesWithBranches, options.KeepBranchSelectionIndex) + loadedBranches = self.refreshReflogAndBranches(capturedReflog, capturedBranches, includeWorktreesWithBranches, options.BranchSelection, options.SelectTopReflogCommit, env) branchesAndRemotesWg.Done() }) } else { branchesAndRemotesWg.Add(1) refresh("branches", func() { - self.refreshBranches(includeWorktreesWithBranches, options.KeepBranchSelectionIndex, true) + // Not a recency sort, so branches doesn't depend on the reflog + // being fresh; it runs concurrently with the reflog refresh + // below and uses the reflog we captured up front, as it always has. + loadedBranches = self.refreshBranches(capturedBranches, includeWorktreesWithBranches, options.BranchSelection, true, capturedReflog.reflogCommits, env) branchesAndRemotesWg.Done() }) - refresh("reflog", func() { _ = self.refreshReflogCommits() }) + refresh("reflog", func() { + _, _ = self.refreshReflogCommits(capturedReflog, env, options.SelectTopReflogCommit) + }) } } else if scopeSet.Includes(types.REBASE_COMMITS) { // the above block handles rebase commits so we only need to call this one // if we've asked specifically for rebase commits and not those other things - refresh("rebase commits", func() { _ = self.refreshRebaseCommits() }) + var rebaseHashPool *utils.StringPool + var rebaseCommits []*models.Commit + self.captureOnUIThread(fRunsOnUIThread, env.background, func() { + rebaseHashPool, rebaseCommits = self.captureRebaseCommitState() + }) + refresh("rebase commits", func() { _ = self.refreshRebaseCommits(rebaseHashPool, rebaseCommits, env) }) } if scopeSet.Includes(types.SUB_COMMITS) { - refresh("sub commits", func() { _ = self.refreshSubCommitsWithLimit() }) + var capturedSubCommits capturedSubCommitState + self.captureOnUIThread(fRunsOnUIThread, env.background, func() { + capturedSubCommits = self.captureSubCommitState() + }) + refresh("sub commits", func() { _ = self.refreshSubCommitsWithLimit(capturedSubCommits, env) }) } // reason we're not doing this if the COMMITS type is included is that if the COMMITS type _is_ included we will refresh the commit files context anyway if scopeSet.Includes(types.COMMIT_FILES) && !scopeSet.Includes(types.COMMITS) { - refresh("commit files", func() { _ = self.refreshCommitFilesContext() }) + var capturedCommitFiles capturedCommitFilesState + self.captureOnUIThread(fRunsOnUIThread, env.background, func() { + capturedCommitFiles = self.captureCommitFilesState() + }) + refresh("commit files", func() { _ = self.refreshCommitFilesContext(capturedCommitFiles, env) }) } fileWg := sync.WaitGroup{} if scopeSet.Includes(types.FILES) { + var capturedFiles capturedFilesState + self.captureOnUIThread(fRunsOnUIThread, env.background, func() { + capturedFiles = self.captureFilesState() + }) fileWg.Add(1) refresh("files", func() { - _ = self.refreshFilesAndSubmodules(options.Background) + _ = self.refreshFilesAndSubmodules(capturedFiles, env) fileWg.Done() }) } if scopeSet.Includes(types.STASH) { - refresh("stash", func() { self.refreshStashEntries() }) + var stashFilterPath string + self.captureOnUIThread(fRunsOnUIThread, env.background, func() { + stashFilterPath = self.c.Modes().Filtering.GetPath() + }) + refresh("stash", func() { self.refreshStashEntries(stashFilterPath, env) }) } if scopeSet.Includes(types.TAGS) { - refresh("tags", func() { _ = self.refreshTags() }) + refresh("tags", func() { _ = self.refreshTags(env) }) } if scopeSet.Includes(types.REMOTES) { + // Capture the previously-selected remote on the UI thread; the worker + // needs it to keep the remote-branches selection valid, and reading + // the Remotes context off the UI thread races its render. + var prevSelectedRemote *models.Remote + self.captureOnUIThread(fRunsOnUIThread, env.background, func() { + prevSelectedRemote = self.c.Contexts().Remotes.GetSelected() + }) branchesAndRemotesWg.Add(1) refresh("remotes", func() { - _ = self.refreshRemotes() + loadedRemotes, _ = self.refreshRemotes(prevSelectedRemote, env) branchesAndRemotesWg.Done() }) } @@ -227,18 +338,30 @@ func (self *RefreshHelper) Refresh(options types.RefreshOptions) { if scopeSet.Includes(types.PULL_REQUESTS) { refresh("pull requests", func() { branchesAndRemotesWg.Wait() - self.refreshGithubPullRequests() + // Use the branches and remotes the loads above stashed, not + // Model().Branches/Remotes: those writes are bounced onto the + // UI thread and may not have landed on this worker yet. The + // wait above orders us after both loads have stashed theirs. + self.refreshGithubPullRequests(loadedBranches, loadedRemotes, env) }) } if scopeSet.Includes(types.WORKTREES) && !includeWorktreesWithBranches { - refresh("worktrees", func() { self.refreshWorktrees() }) + refresh("worktrees", func() { self.refreshWorktrees(env) }) } if scopeSet.Includes(types.STAGING) { refresh("staging", func() { fileWg.Wait() - self.stagingHelper.RefreshStagingPanel(types.OnFocusOpts{}) + // Bounce onto the UI thread so this runs after the files + // scope's model-update bounce — RefreshStagingPanel reads + // Model.Files (via Files.GetSelected) and would otherwise + // see the pre-refresh model. Guard on the generation so a + // repo switch mid-refresh drops it, like the model bounces. + self.onUIThreadUnlessRepoChanged(env, func() error { + self.stagingHelper.RefreshStagingPanel(types.OnFocusOpts{}) + return nil + }) }) } @@ -247,15 +370,21 @@ func (self *RefreshHelper) Refresh(options types.RefreshOptions) { } if scopeSet.Includes(types.MERGE_CONFLICTS) { - refresh("merge conflicts", func() { _ = self.mergeConflictsHelper.RefreshMergeState() }) + refresh("merge conflicts", func() { _ = self.mergeConflictsHelper.RefreshMergeState(env.background) }) } - self.refreshStatus() + self.refreshStatus(env) wg.Wait() if options.Then != nil { - options.Then() + // Queue Then via OnUIThread so it runs *after* the refresh-scope + // functions' model-update bounces (which are already queued by + // now), not synchronously here — at this point the workers have + // returned but their bounces haven't been processed yet, so + // invoking Then synchronously would run it on a model that's + // still pre-refresh. + self.onUIThread(env.background, options.Then) } } @@ -361,49 +490,153 @@ func getModeName(mode types.RefreshMode) string { } } -// during startup, the bottleneck is fetching the reflog entries. We need these -// on startup to sort the branches by recency. So we have two phases: INITIAL, and COMPLETE. -// In the initial phase we don't get any reflog commits, but we asynchronously get them -// and refresh the branches after that -func (self *RefreshHelper) refreshReflogCommitsConsideringStartup() { +// During startup, the bottleneck is fetching the reflog entries, which we need +// in order to sort the branches by recency. So we have two phases: INITIAL and +// COMPLETE. In the INITIAL phase we don't have any reflog commits yet, so we +// show the branches right away sorted by whatever we have (typically nothing, +// i.e. not by recency), then load the reflog on a worker and refresh the +// branches again, this time recency-sorted. From then on we're in the COMPLETE +// phase and load the reflog synchronously before refreshing the branches. +// +// The immediate refresh must run before we spawn the async one, not after: that +// order gives the immediate (non-recency) load a lower branch-load sequence +// than the async (recency) load, so the sequence guard in refreshBranches keeps +// the recency-sorted result even if the two loads' bounces land out of order. +// capturedReflogState holds the reflog refresh's model/mode inputs, gathered on +// the UI thread before the git work runs. The existing reflog slices feed the +// incremental fetch (we only load entries newer than the ones we already have). +type capturedReflogState struct { + reflogCommits []*models.Commit + filteredReflogCommits []*models.Commit + hashPool *utils.StringPool + filteringActive bool + filterPath string + filterAuthor string +} + +// captureReflogState reads the reflog refresh's inputs into an immutable +// snapshot. It must run on the UI thread. +func (self *RefreshHelper) captureReflogState() capturedReflogState { + return capturedReflogState{ + reflogCommits: self.c.Model().ReflogCommits, + filteredReflogCommits: self.c.Model().FilteredReflogCommits, + hashPool: self.c.Model().HashPool, + filteringActive: self.c.Modes().Filtering.Active(), + filterPath: self.c.Modes().Filtering.GetPath(), + filterAuthor: self.c.Modes().Filtering.GetAuthor(), + } +} + +// capturedBranchState holds the branches refresh's model inputs, gathered on the +// UI thread before the git work runs. oldBranches is used only to carry over the +// previous BehindBaseBranch values (to reduce flicker) — an atomic each, so a +// pre-refresh snapshot serves both the immediate and recency loads identically. +type capturedBranchState struct { + mainBranches *git_commands.MainBranches + oldBranches []*models.Branch +} + +// captureBranchState reads the branches refresh's model inputs into an immutable +// snapshot. It must run on the UI thread. +func (self *RefreshHelper) captureBranchState() capturedBranchState { + return capturedBranchState{ + mainBranches: self.c.Model().MainBranches, + oldBranches: self.c.Model().Branches, + } +} + +func (self *RefreshHelper) refreshReflogAndBranches(capturedReflog capturedReflogState, capturedBranches capturedBranchState, refreshWorktrees bool, branchSelection types.BranchSelectionBehavior, selectTopReflogCommit bool, env refreshEnv) []*models.Branch { switch self.c.State().GetRepoState().GetStartupStage() { case types.INITIAL: - self.c.OnWorker(func(_ gocui.Task) error { - _ = self.refreshReflogCommits() - self.refreshBranches(false, true, true) + // Return the immediate (non-recency) load's branches; the recency-sorted + // reload below runs on its own worker after we return. Both hold the same + // set of branches, which is all the caller (the PR fetch) needs. + branches := self.refreshBranches(capturedBranches, refreshWorktrees, branchSelection, false, capturedReflog.reflogCommits, env) + + self.onWorker(env.background, func(_ gocui.Task) error { + reflogCommits, _ := self.refreshReflogCommits(capturedReflog, env, false) + self.refreshBranches(capturedBranches, false, types.SelectCheckedOutBranch, true, reflogCommits, env) self.c.State().GetRepoState().SetStartupStage(types.COMPLETE) return nil }) + return branches + case types.COMPLETE: - _ = self.refreshReflogCommits() + reflogCommits, _ := self.refreshReflogCommits(capturedReflog, env, selectTopReflogCommit) + return self.refreshBranches(capturedBranches, refreshWorktrees, branchSelection, true, reflogCommits, env) + } + + return nil +} + +// capturedCommitState holds everything the commits refresh reads from the +// model, contexts, and modes. It is gathered on the UI thread (see +// captureCommitsState) before the git work is dispatched to a worker, so the +// worker computes from an immutable snapshot rather than reading state the UI +// thread concurrently mutates. +type capturedCommitState struct { + selectionRange *localCommitSelectionRange + limitCommits bool + showWholeGitGraph bool + filterPath string + filterAuthor string + mainBranches *git_commands.MainBranches + hashPool *utils.StringPool + parentIsLocalCommits bool +} + +// captureCommitsState reads the commits refresh's model/context/mode inputs +// into an immutable snapshot. It must run on the UI thread. +func (self *RefreshHelper) captureCommitsState(commitSelection types.CommitSelectionBehavior) capturedCommitState { + var selectionRange *localCommitSelectionRange + if commitSelection == types.KeepCommitSelectionByHash { + selectedIdx, rangeStartIdx, rangeSelectMode := self.c.Contexts().LocalCommits.GetSelectionRangeAndMode() + selectionRange = captureLocalCommitSelectionRange(self.c.Model().Commits, selectedIdx, rangeStartIdx, rangeSelectMode) + } + + parentCtx := self.c.Contexts().CommitFiles.GetParentContext() + + return capturedCommitState{ + selectionRange: selectionRange, + limitCommits: self.c.Contexts().LocalCommits.GetLimitCommits(), + showWholeGitGraph: self.c.Contexts().LocalCommits.GetShowWholeGitGraph(), + filterPath: self.c.Modes().Filtering.GetPath(), + filterAuthor: self.c.Modes().Filtering.GetAuthor(), + mainBranches: self.c.Model().MainBranches, + hashPool: self.c.Model().HashPool, + parentIsLocalCommits: parentCtx != nil && parentCtx.GetKey() == context.LOCAL_COMMITS_CONTEXT_KEY, } } -func (self *RefreshHelper) refreshReflogAndBranches(refreshWorktrees bool, keepBranchSelectionIndex bool) { - loadBehindCounts := self.c.State().GetRepoState().GetStartupStage() == types.COMPLETE - - self.refreshReflogCommitsConsideringStartup() - - self.refreshBranches(refreshWorktrees, keepBranchSelectionIndex, loadBehindCounts) -} - -func (self *RefreshHelper) refreshCommitsAndCommitFiles(commitSelection types.CommitSelectionBehavior) { - _ = self.refreshCommitsWithLimit(commitSelection) - ctx := self.c.Contexts().CommitFiles.GetParentContext() - if ctx != nil && ctx.GetKey() == context.LOCAL_COMMITS_CONTEXT_KEY { +func (self *RefreshHelper) refreshCommitsAndCommitFiles(captured capturedCommitState, commitSelection types.CommitSelectionBehavior, env refreshEnv) { + _ = self.refreshCommitsWithLimit(captured, commitSelection, env) + if captured.parentIsLocalCommits { // This makes sense when we've e.g. just amended a commit, meaning we get a new commit hash at the same position. // However if we've just added a brand new commit, it pushes the list down by one and so we would end up // showing the contents of a different commit than the one we initially entered. // Ideally we would know when to refresh the commit files context and when not to, // or perhaps we could just pop that context off the stack whenever cycling windows. // For now the awkwardness remains. - commit := self.c.Contexts().LocalCommits.GetSelected() - if commit != nil && commit.RefName() != "" { - refRange := self.c.Contexts().LocalCommits.GetSelectedRefRangeForDiffFiles() - self.c.Contexts().CommitFiles.ReInit(commit, refRange) - _ = self.refreshCommitFilesContext() - } + // + // The commit selection is restored in refreshCommitsWithLimit's bounce, + // so read it on the UI thread after that bounce; then load the commit + // files back on a worker (refreshCommitFilesContext does git work). + self.onUIThreadUnlessRepoChanged(env, func() error { + commit := self.c.Contexts().LocalCommits.GetSelected() + if commit != nil && commit.RefName() != "" { + refRange := self.c.Contexts().LocalCommits.GetSelectedRefRangeForDiffFiles() + self.c.Contexts().CommitFiles.ReInit(commit, refRange) + // Capture the diff endpoints here, on the UI thread and after + // ReInit has set them, before dispatching the git work. + capturedCommitFiles := self.captureCommitFilesState() + self.onWorker(env.background, func(gocui.Task) error { + _ = self.refreshCommitFilesContext(capturedCommitFiles, env) + return nil + }) + } + return nil + }) } } @@ -433,68 +666,70 @@ func (self *RefreshHelper) determineCheckedOutRef() models.Ref { return nil } -func (self *RefreshHelper) refreshCommitsWithLimit(commitSelection types.CommitSelectionBehavior) error { - self.c.Mutexes().LocalCommitsMutex.Lock() - defer self.c.Mutexes().LocalCommitsMutex.Unlock() - - var selectionRange *localCommitSelectionRange - if commitSelection == types.KeepCommitSelectionByHash { - selectedIdx, rangeStartIdx, rangeSelectMode := self.c.Contexts().LocalCommits.GetSelectionRangeAndMode() - selectionRange = captureLocalCommitSelectionRange(self.c.Model().Commits, selectedIdx, rangeStartIdx, rangeSelectMode) - } - +func (self *RefreshHelper) refreshCommitsWithLimit(captured capturedCommitState, commitSelection types.CommitSelectionBehavior, env refreshEnv) error { checkedOutRef := self.determineCheckedOutRef() + refName, bisectInfo := self.refForLog() commits, err := self.c.Git().Loaders.CommitLoader.GetCommits( git_commands.GetCommitsOptions{ - Limit: self.c.Contexts().LocalCommits.GetLimitCommits(), - FilterPath: self.c.Modes().Filtering.GetPath(), - FilterAuthor: self.c.Modes().Filtering.GetAuthor(), + Limit: captured.limitCommits, + FilterPath: captured.filterPath, + FilterAuthor: captured.filterAuthor, IncludeRebaseCommits: true, - RefName: self.refForLog(), + RefName: refName, RefForPushedStatus: checkedOutRef, - All: self.c.Contexts().LocalCommits.GetShowWholeGitGraph(), - MainBranches: self.c.Model().MainBranches, - HashPool: self.c.Model().HashPool, + All: captured.showWholeGitGraph, + MainBranches: captured.mainBranches, + HashPool: captured.hashPool, }, ) if err != nil { return err } - self.c.Model().Commits = commits - self.RefreshAuthors(commits) - self.c.Model().WorkingTreeStateAtLastCommitRefresh = self.c.Git().Status.WorkingTreeState() - if checkedOutRef != nil { - self.c.Model().CheckedOutBranch = checkedOutRef.RefName() - } else { - self.c.Model().CheckedOutBranch = "" - } + workingTreeState := self.c.Git().Status.WorkingTreeState() - scrollSelectionIntoView := false - switch commitSelection { - case types.SelectHeadCommit: - if headCommitIdx := models.HeadCommitIdx(commits); headCommitIdx >= 0 { - self.c.Contexts().LocalCommits.SetSelection(headCommitIdx) - scrollSelectionIntoView = true + self.onUIThreadUnlessRepoChanged(env, func() error { + self.c.Model().BisectInfo = bisectInfo + self.c.Model().Commits = commits + self.RefreshAuthors(commits) + self.c.Model().WorkingTreeStateAtLastCommitRefresh = workingTreeState + if checkedOutRef != nil { + self.c.Model().CheckedOutBranch = checkedOutRef.RefName() + } else { + self.c.Model().CheckedOutBranch = "" } - case types.KeepCommitSelectionByHash: - if selectionRange != nil { - selectedIdx, rangeStartIdx, didMove, found := findLocalCommitSelectionRange(commits, selectionRange) - if found { - self.c.Contexts().LocalCommits.SetSelectionRangeAndMode(selectedIdx, rangeStartIdx, selectionRange.mode) - scrollSelectionIntoView = didMove + + scrollSelectionIntoView := false + switch commitSelection { + case types.SelectHeadCommit: + if headCommitIdx := models.HeadCommitIdx(commits); headCommitIdx >= 0 { + self.c.Contexts().LocalCommits.SetSelection(headCommitIdx) + scrollSelectionIntoView = true } + case types.KeepCommitSelectionByHash: + if captured.selectionRange != nil { + selectedIdx, rangeStartIdx, didMove, found := findLocalCommitSelectionRange(commits, captured.selectionRange) + if found { + self.c.Contexts().LocalCommits.SetSelectionRangeAndMode(selectedIdx, rangeStartIdx, captured.selectionRange.mode) + scrollSelectionIntoView = didMove + } + } + case types.KeepCommitSelectionIndex: + // The caller set the selection index deliberately; leave it untouched. } - case types.KeepCommitSelectionIndex: - // The caller set the selection index deliberately; leave it untouched. - } - self.refreshView(self.c.Contexts().LocalCommits) - if scrollSelectionIntoView { - self.c.OnUIThread(func() error { - self.c.Contexts().LocalCommits.FocusLine(true) - return nil - }) - } + if scrollSelectionIntoView { + // Enqueued from within this bounce so it runs after refreshView's + // render below (which was enqueued first), matching the previous + // ordering where FocusLine ran after the view was re-rendered. + self.onUIThreadUnlessRepoChanged(env, func() error { + self.c.Contexts().LocalCommits.FocusLine(true) + return nil + }) + } + return nil + }) + + self.refreshView(self.c.Contexts().LocalCommits, env) return nil } @@ -573,41 +808,65 @@ func hasRestorableCommitHash(commits []*models.Commit, idx int) bool { return idx >= 0 && idx < len(commits) && commits[idx].Hash() != "" } -func (self *RefreshHelper) refreshSubCommitsWithLimit() error { - if self.c.Contexts().SubCommits.GetRef() == nil { +// capturedSubCommitState holds the sub-commits refresh's model/context/mode +// inputs, gathered on the UI thread (see captureSubCommitState) before the git +// work is dispatched to a worker. +type capturedSubCommitState struct { + ref models.Ref + limitCommits bool + refToShowDivergenceFrom string + filterPath string + filterAuthor string + mainBranches *git_commands.MainBranches + hashPool *utils.StringPool +} + +// captureSubCommitState reads the sub-commits refresh's inputs into an immutable +// snapshot. It must run on the UI thread. +func (self *RefreshHelper) captureSubCommitState() capturedSubCommitState { + return capturedSubCommitState{ + ref: self.c.Contexts().SubCommits.GetRef(), + limitCommits: self.c.Contexts().SubCommits.GetLimitCommits(), + refToShowDivergenceFrom: self.c.Contexts().SubCommits.GetRefToShowDivergenceFrom(), + filterPath: self.c.Modes().Filtering.GetPath(), + filterAuthor: self.c.Modes().Filtering.GetAuthor(), + mainBranches: self.c.Model().MainBranches, + hashPool: self.c.Model().HashPool, + } +} + +func (self *RefreshHelper) refreshSubCommitsWithLimit(captured capturedSubCommitState, env refreshEnv) error { + if captured.ref == nil { return nil } - self.c.Mutexes().SubCommitsMutex.Lock() - defer self.c.Mutexes().SubCommitsMutex.Unlock() - commits, err := self.c.Git().Loaders.CommitLoader.GetCommits( git_commands.GetCommitsOptions{ - Limit: self.c.Contexts().SubCommits.GetLimitCommits(), - FilterPath: self.c.Modes().Filtering.GetPath(), - FilterAuthor: self.c.Modes().Filtering.GetAuthor(), + Limit: captured.limitCommits, + FilterPath: captured.filterPath, + FilterAuthor: captured.filterAuthor, IncludeRebaseCommits: false, - RefName: self.c.Contexts().SubCommits.GetRef().FullRefName(), - RefToShowDivergenceFrom: self.c.Contexts().SubCommits.GetRefToShowDivergenceFrom(), - RefForPushedStatus: self.c.Contexts().SubCommits.GetRef(), - MainBranches: self.c.Model().MainBranches, - HashPool: self.c.Model().HashPool, + RefName: captured.ref.FullRefName(), + RefToShowDivergenceFrom: captured.refToShowDivergenceFrom, + RefForPushedStatus: captured.ref, + MainBranches: captured.mainBranches, + HashPool: captured.hashPool, }, ) if err != nil { return err } - self.c.Model().SubCommits = commits - self.RefreshAuthors(commits) + self.onUIThreadUnlessRepoChanged(env, func() error { + self.c.Model().SubCommits = commits + self.RefreshAuthors(commits) + return nil + }) - self.refreshView(self.c.Contexts().SubCommits) + self.refreshView(self.c.Contexts().SubCommits, env) return nil } func (self *RefreshHelper) RefreshAuthors(commits []*models.Commit) { - self.c.Mutexes().AuthorsMutex.Lock() - defer self.c.Mutexes().AuthorsMutex.Unlock() - authors := self.c.Model().Authors for _, commit := range commits { if _, ok := authors[commit.AuthorEmail]; !ok { @@ -619,79 +878,97 @@ func (self *RefreshHelper) RefreshAuthors(commits []*models.Commit) { } } -func (self *RefreshHelper) refreshCommitFilesContext() error { +// capturedCommitFilesState holds the commit-files refresh's context/mode inputs +// (the diff endpoints), gathered on the UI thread before the git work runs. +type capturedCommitFilesState struct { + from string + to string + reverse bool +} + +// captureCommitFilesState reads the commit-files refresh's diff endpoints into +// an immutable snapshot. It must run on the UI thread. +func (self *RefreshHelper) captureCommitFilesState() capturedCommitFilesState { from, to := self.c.Contexts().CommitFiles.GetFromAndToForDiff() from, reverse := self.c.Modes().Diffing.GetFromAndReverseArgsForDiff(from) + return capturedCommitFilesState{from: from, to: to, reverse: reverse} +} - files, err := self.c.Git().Loaders.CommitFileLoader.GetFilesInDiff(from, to, reverse) +func (self *RefreshHelper) refreshCommitFilesContext(captured capturedCommitFilesState, env refreshEnv) error { + files, err := self.c.Git().Loaders.CommitFileLoader.GetFilesInDiff(captured.from, captured.to, captured.reverse) if err != nil { return err } - self.c.Model().CommitFiles = files - self.c.Contexts().CommitFiles.CommitFileTreeViewModel.SetTree() - - self.refreshView(self.c.Contexts().CommitFiles) + self.onUIThreadUnlessRepoChanged(env, func() error { + self.c.Model().CommitFiles = files + self.c.Contexts().CommitFiles.CommitFileTreeViewModel.SetTree() + return nil + }) + self.refreshView(self.c.Contexts().CommitFiles, env) return nil } -func (self *RefreshHelper) refreshRebaseCommits() error { - self.c.Mutexes().LocalCommitsMutex.Lock() - defer self.c.Mutexes().LocalCommitsMutex.Unlock() +// captureRebaseCommitState reads the rebase-commits refresh's model inputs into +// an immutable snapshot. It must run on the UI thread. +func (self *RefreshHelper) captureRebaseCommitState() (hashPool *utils.StringPool, commits []*models.Commit) { + return self.c.Model().HashPool, self.c.Model().Commits +} - updatedCommits, err := self.c.Git().Loaders.CommitLoader.MergeRebasingCommits(self.c.Model().HashPool, self.c.Model().Commits) +func (self *RefreshHelper) refreshRebaseCommits(hashPool *utils.StringPool, commits []*models.Commit, env refreshEnv) error { + updatedCommits, err := self.c.Git().Loaders.CommitLoader.MergeRebasingCommits(hashPool, commits) if err != nil { return err } - self.c.Model().Commits = updatedCommits - self.c.Model().WorkingTreeStateAtLastCommitRefresh = self.c.Git().Status.WorkingTreeState() + workingTreeState := self.c.Git().Status.WorkingTreeState() - self.refreshView(self.c.Contexts().LocalCommits) + self.onUIThreadUnlessRepoChanged(env, func() error { + self.c.Model().Commits = updatedCommits + self.c.Model().WorkingTreeStateAtLastCommitRefresh = workingTreeState + return nil + }) + + self.refreshView(self.c.Contexts().LocalCommits, env) return nil } -func (self *RefreshHelper) refreshTags() error { +func (self *RefreshHelper) refreshTags(env refreshEnv) error { tags, err := self.c.Git().Loaders.TagLoader.GetTags() if err != nil { return err } - self.c.Model().Tags = tags + self.onUIThreadUnlessRepoChanged(env, func() error { + self.c.Model().Tags = tags + return nil + }) - self.refreshView(self.c.Contexts().Tags) + self.refreshView(self.c.Contexts().Tags, env) return nil } -func (self *RefreshHelper) refreshStateSubmoduleConfigs() error { - configs, err := self.c.Git().Submodule.GetConfigs(nil) - if err != nil { - return err - } - - self.c.Model().Submodules = configs - - return nil +func (self *RefreshHelper) refreshStateSubmoduleConfigs() ([]*models.SubmoduleConfig, error) { + return self.c.Git().Submodule.GetConfigs(nil) } // self.refreshStatus is called at the end of this because that's when we can // be sure there is a State.Model.Branches array to pick the current branch from -func (self *RefreshHelper) refreshBranches(refreshWorktrees bool, keepBranchSelectionIndex bool, loadBehindCounts bool) { - self.c.Mutexes().RefreshingBranchesMutex.Lock() - defer self.c.Mutexes().RefreshingBranchesMutex.Unlock() +func (self *RefreshHelper) refreshBranches(captured capturedBranchState, refreshWorktrees bool, branchSelection types.BranchSelectionBehavior, loadBehindCounts bool, reflogCommits []*models.Commit, env refreshEnv) []*models.Branch { + loadSeq := self.branchLoadSeq.Add(1) branches, err := self.c.Git().Loaders.BranchLoader.Load( - self.c.Model().ReflogCommits, - self.c.Model().MainBranches, - self.c.Model().Branches, + reflogCommits, + captured.mainBranches, + captured.oldBranches, loadBehindCounts, func(f func() error) { - self.c.OnWorker(func(_ gocui.Task) error { + self.onWorker(env.background, func(_ gocui.Task) error { return f() }) }, func() { - self.c.OnUIThread(func() error { + self.onUIThreadUnlessRepoChanged(env, func() error { self.c.Contexts().Branches.HandleRender() - self.refreshStatus() + self.refreshStatus(env) return nil }) }) @@ -699,66 +976,175 @@ func (self *RefreshHelper) refreshBranches(refreshWorktrees bool, keepBranchSele self.c.Log.Error(err) } - prevSelectedBranch := self.c.Contexts().Branches.GetSelected() - - self.c.Model().Branches = branches - self.rebuildPullRequestsMap() - + var worktrees []*models.Worktree if refreshWorktrees { - self.loadWorktrees() - self.refreshView(self.c.Contexts().Worktrees) + worktrees = self.loadWorktrees() } - if !keepBranchSelectionIndex && prevSelectedBranch != nil { - self.searchHelper.ReApplyFilter(self.c.Contexts().Branches) - - _, idx, found := lo.FindIndexOf(self.c.Contexts().Branches.GetItems(), - func(b *models.Branch) bool { return b.Name == prevSelectedBranch.Name }) - if found { - self.c.Contexts().Branches.SetSelectedLineIdx(idx) + self.onUIThreadUnlessRepoChanged(env, func() error { + // Drop this write if a branch load that started later has already applied + // its result. At the INITIAL startup stage an immediate load (not + // recency-sorted) and an async recency-sorted load run concurrently; this + // makes the later-started (recency-sorted) one win regardless of which + // finishes first, so its result isn't clobbered by the stale immediate one. + if loadSeq < self.appliedBranchLoadSeq { + return nil } - } + self.appliedBranchLoadSeq = loadSeq - self.refreshView(self.c.Contexts().Branches) + // Read the currently-selected branch before overwriting the list, so we + // can restore it by name below. Reading it here in the bounce keeps it on + // the UI thread. + prevSelectedBranch := self.c.Contexts().Branches.GetSelected() - // Need to re-render the commits view because the visualization of local - // branch heads might have changed - self.c.OnUIThread(func() error { - self.c.Mutexes().LocalCommitsMutex.Lock() + self.c.Model().Branches = branches + // Rebuilding here (rather than on the worker) means the map is built from + // the branches we just wrote, on the UI thread. + self.rebuildPullRequestsMap() + + if refreshWorktrees { + self.c.Model().Worktrees = worktrees + self.refreshView(self.c.Contexts().Worktrees, env) + } + + // Setting the selection here, in the same bounce that writes the list, + // keeps it on the UI thread and keeps the list and selection updating in + // the same frame. + switch branchSelection { + case types.KeepBranchSelectionByName: + if prevSelectedBranch != nil { + self.searchHelper.ReApplyFilter(self.c.Contexts().Branches) + + _, idx, found := lo.FindIndexOf(self.c.Contexts().Branches.GetItems(), + func(b *models.Branch) bool { return b.Name == prevSelectedBranch.Name }) + if found { + self.c.Contexts().Branches.SetSelectedLineIdx(idx) + } + } + case types.SelectCheckedOutBranch: + // The checked-out branch is always at the top of the list. Setting + // the selection doesn't scroll the view, so also reset the origin. + self.c.Contexts().Branches.SetSelectedLineIdx(0) + self.c.Contexts().Branches.GetView().SetOriginY(0) + } + + // Need to re-render the commits view because the visualization of local + // branch heads might have changed self.c.Contexts().LocalCommits.HandleRender() - self.c.Mutexes().LocalCommitsMutex.Unlock() return nil }) - self.refreshStatus() + self.refreshView(self.c.Contexts().Branches, env) + + self.refreshStatus(env) + + // Return the freshly-loaded branches so the caller can hand them to the PR + // fetch without reading them back from the (bounce-written) model. + return branches } -func (self *RefreshHelper) refreshFilesAndSubmodules(background bool) error { - self.c.Mutexes().RefreshingFilesMutex.Lock() - self.c.State().SetIsRefreshingFiles(true) - defer func() { - self.c.State().SetIsRefreshingFiles(false) - self.c.Mutexes().RefreshingFilesMutex.Unlock() - }() - - if err := self.refreshStateSubmoduleConfigs(); err != nil { +func (self *RefreshHelper) refreshFilesAndSubmodules(captured capturedFilesState, env refreshEnv) error { + configs, err := self.refreshStateSubmoduleConfigs() + if err != nil { return err } - if err := self.refreshStateFiles(background); err != nil { + if err := self.refreshStateFiles(captured, env, configs); err != nil { return err } - self.c.OnUIThread(func() error { - self.refreshView(self.c.Contexts().Submodules) - self.refreshView(self.c.Contexts().Files) - return nil - }) + self.refreshView(self.c.Contexts().Submodules, env) + self.refreshView(self.c.Contexts().Files, env) return nil } -func (self *RefreshHelper) refreshStateFiles(background bool) error { +// onUIThreadUnlessRepoChanged bounces a refresh's model/view update onto the UI +// thread, but drops it if the repo was switched while the refresh was in flight. +// Refresh workers do their git work off the UI thread and enqueue their model +// writes here; a repo switch (which replaces the whole model and context tree) +// bumps the generation, so a write captured under the old generation must not +// clobber the new repo's state. The generation is captured once at the start of +// the refresh and carried in env (see refreshEnv). +func (self *RefreshHelper) onUIThreadUnlessRepoChanged(env refreshEnv, f func() error) { + self.onUIThread(env.background, func() error { + if self.c.State().GetRepoGeneration() != env.generation { + return nil + } + return f() + }) +} + +// onWorker and onUIThread pick the foreground or background variant of the +// corresponding dispatch method depending on whether we're servicing a +// background refresh. Background refreshes (auto-fetch and friends) must not +// count towards lazygit being busy, or they'd spuriously block a repo switch; +// see the *Background methods on gocui.Gui. +func (self *RefreshHelper) onWorker(background bool, f func(gocui.Task) error) { + if background { + self.c.OnWorkerBackground(f) + } else { + self.c.OnWorker(f) + } +} + +func (self *RefreshHelper) onUIThread(background bool, f func() error) { + if background { + self.c.OnUIThreadBackground(f) + } else { + self.c.OnUIThread(f) + } +} + +// captureOnUIThread runs fn on the UI thread and returns once it has run. fn +// reads the model/context/mode state a refresh scope needs into locals, so the +// worker that follows computes from an immutable snapshot instead of reading +// state the UI thread concurrently mutates. When the enclosing refresh function +// runs on the UI thread (fRunsOnUIThread is true) fn runs inline; when it runs +// on a worker, fn is dispatched to the UI thread and we block for it. +// +// The inline case matters for correctness as much as the hop: a SYNC or +// BLOCK_UI refresh parks the UI thread in a wg.Wait while its scope workers +// run, so a scope worker that tried to hop to the UI thread there would +// deadlock. Capturing before those workers are spawned — inline, on the UI +// thread — avoids that entirely. This is why BLOCK_UI (which always runs on the +// UI thread, even from a worker caller) captures inline rather than hopping. +func (self *RefreshHelper) captureOnUIThread(fRunsOnUIThread bool, background bool, fn func()) { + if fRunsOnUIThread { + fn() + return + } + + wrapped := func() error { + fn() + return nil + } + if background { + _ = self.c.GocuiGui().OnUIThreadAndWaitBackground(wrapped) + } else { + _ = self.c.GocuiGui().OnUIThreadAndWait(wrapped) + } +} + +// capturedFilesState holds the files refresh's context/model inputs, gathered +// on the UI thread before the git work runs: the previous files list (to detect +// resolved conflicts and drive the auto-stage), and whether untracked files are +// force-shown. +type capturedFilesState struct { + prevFiles []*models.File + forceShowUntracked bool +} + +// captureFilesState reads the files refresh's inputs into an immutable snapshot. +// It must run on the UI thread. +func (self *RefreshHelper) captureFilesState() capturedFilesState { + return capturedFilesState{ + prevFiles: self.c.Model().Files, + forceShowUntracked: self.c.Contexts().Files.ForceShowUntracked(), + } +} + +func (self *RefreshHelper) refreshStateFiles(captured capturedFilesState, env refreshEnv, submoduleConfigs []*models.SubmoduleConfig) error { fileTreeViewModel := self.c.Contexts().Files.FileTreeViewModel prevConflictFileCount := 0 @@ -771,7 +1157,7 @@ func (self *RefreshHelper) refreshStateFiles(background bool) error { // Although this also means that at startup we won't be staging anything until // we call git status again. pathsToStage := []string{} - for _, file := range self.c.Model().Files { + for _, file := range captured.prevFiles { if file.HasMergeConflicts { prevConflictFileCount++ } @@ -795,8 +1181,8 @@ func (self *RefreshHelper) refreshStateFiles(background bool) error { files := self.c.Git().Loaders.FileLoader. GetStatusFiles(git_commands.GetStatusFileOptions{ - ForceShowUntracked: self.c.Contexts().Files.ForceShowUntracked(), - Background: background, + ForceShowUntracked: captured.forceShowUntracked, + Background: env.background, }) conflictFileCount := 0 @@ -820,35 +1206,41 @@ func (self *RefreshHelper) refreshStateFiles(background bool) error { // (e.g. in the user's editor). Offer to continue it. We only do this // for operations we started ourselves; prompting for one that was // started outside lazygit (e.g. by a coding agent) would be confusing. - self.c.OnUIThread(func() error { return self.mergeAndRebaseHelper.PromptToContinueRebase() }) + self.onUIThreadUnlessRepoChanged(env, func() error { + return self.mergeAndRebaseHelper.PromptToContinueRebase() + }) } } else { // Either there's no operation in progress any more, or new conflicts have // appeared. Either way, a "continue?" prompt we're showing is now stale // (e.g. the operation was continued or aborted outside lazygit), so // dismiss it rather than leave the user with a prompt that would fail. - self.c.OnUIThread(func() error { + // Guard on the generation like the sibling PromptToContinueRebase + // bounce above: if the repo was switched while this refresh was in + // flight, a prompt showing now belongs to the new repo, so leave it be. + self.onUIThreadUnlessRepoChanged(env, func() error { self.mergeAndRebaseHelper.DismissContinueRebasePromptIfShowing() return nil }) } - fileTreeViewModel.RWMutex.Lock() - - // only taking over the filter if it hasn't already been set by the user. - if conflictFileCount > 0 && prevConflictFileCount == 0 { - if fileTreeViewModel.GetStatusFilter() == filetree.DisplayAll { - fileTreeViewModel.SetStatusFilter(filetree.DisplayConflicted) - self.c.Contexts().Files.GetView().Subtitle = self.c.Tr.FilterLabelConflictingFiles + self.onUIThreadUnlessRepoChanged(env, func() error { + // only taking over the filter if it hasn't already been set by the user. + if conflictFileCount > 0 && prevConflictFileCount == 0 { + if fileTreeViewModel.GetStatusFilter() == filetree.DisplayAll { + fileTreeViewModel.SetStatusFilter(filetree.DisplayConflicted) + self.c.Contexts().Files.GetView().Subtitle = self.c.Tr.FilterLabelConflictingFiles + } + } else if conflictFileCount == 0 && fileTreeViewModel.GetStatusFilter() == filetree.DisplayConflicted { + fileTreeViewModel.SetStatusFilter(filetree.DisplayAll) + self.c.Contexts().Files.GetView().Subtitle = "" } - } else if conflictFileCount == 0 && fileTreeViewModel.GetStatusFilter() == filetree.DisplayConflicted { - fileTreeViewModel.SetStatusFilter(filetree.DisplayAll) - self.c.Contexts().Files.GetView().Subtitle = "" - } - self.c.Model().Files = files - fileTreeViewModel.SetTree() - fileTreeViewModel.RWMutex.Unlock() + self.c.Model().Submodules = submoduleConfigs + self.c.Model().Files = files + fileTreeViewModel.SetTree() + return nil + }) return nil } @@ -860,147 +1252,184 @@ func (self *RefreshHelper) refreshStateFiles(background bool) error { // This method also manages two things: ReflogCommits and FilteredReflogCommits. // FilteredReflogCommits are rendered in the reflogs panel, and ReflogCommits // are used by the branches panel to obtain recency values for sorting. -func (self *RefreshHelper) refreshReflogCommits() error { +// refreshReflogCommits returns the (non-filtered) ReflogCommits it loaded, so +// that a subsequent branches refresh can use them for recency sorting without +// having to read them back out of the model. +func (self *RefreshHelper) refreshReflogCommits(captured capturedReflogState, env refreshEnv, selectTopEntry bool) ([]*models.Commit, error) { // pulling state into its own variable in case it gets swapped out for another state // and we get an out of bounds exception model := self.c.Model() - refresh := func(stateCommits *[]*models.Commit, filterPath string, filterAuthor string) error { + // load does the git work on the worker and returns the new value for a + // reflog slice, reading the existing slice (captured on the UI thread) for + // the incremental fetch. The caller writes the result in the bounce. + load := func(existing []*models.Commit, filterPath string, filterAuthor string) ([]*models.Commit, error) { var lastReflogCommit *models.Commit - if filterPath == "" && filterAuthor == "" && len(*stateCommits) > 0 { - lastReflogCommit = (*stateCommits)[0] + if filterPath == "" && filterAuthor == "" && len(existing) > 0 { + lastReflogCommit = existing[0] } commits, onlyObtainedNewReflogCommits, err := self.c.Git().Loaders.ReflogCommitLoader. - GetReflogCommits(self.c.Model().HashPool, lastReflogCommit, filterPath, filterAuthor) + GetReflogCommits(captured.hashPool, lastReflogCommit, filterPath, filterAuthor) if err != nil { - return err + return nil, err } if onlyObtainedNewReflogCommits { - *stateCommits = append(commits, *stateCommits...) - } else { - *stateCommits = commits + return append(commits, existing...), nil + } + return commits, nil + } + + reflogCommits, err := load(captured.reflogCommits, "", "") + if err != nil { + return nil, err + } + + filteredReflogCommits := reflogCommits + if captured.filteringActive { + filteredReflogCommits, err = load(captured.filteredReflogCommits, captured.filterPath, captured.filterAuthor) + if err != nil { + return nil, err + } + } + + self.onUIThreadUnlessRepoChanged(env, func() error { + model.ReflogCommits = reflogCommits + model.FilteredReflogCommits = filteredReflogCommits + // Setting the selection here, in the same bounce that writes the list, + // keeps it on the UI thread and atomic with the list update. Setting the + // selection doesn't scroll the view, so also reset the origin. + if selectTopEntry { + self.c.Contexts().ReflogCommits.SetSelectedLineIdx(0) + self.c.Contexts().ReflogCommits.GetView().SetOriginY(0) } return nil - } + }) - if err := refresh(&model.ReflogCommits, "", ""); err != nil { - return err - } - - if self.c.Modes().Filtering.Active() { - if err := refresh(&model.FilteredReflogCommits, self.c.Modes().Filtering.GetPath(), self.c.Modes().Filtering.GetAuthor()); err != nil { - return err - } - } else { - model.FilteredReflogCommits = model.ReflogCommits - } - - self.refreshView(self.c.Contexts().ReflogCommits) - return nil + self.refreshView(self.c.Contexts().ReflogCommits, env) + return reflogCommits, nil } -func (self *RefreshHelper) refreshRemotes() error { - prevSelectedRemote := self.c.Contexts().Remotes.GetSelected() - +func (self *RefreshHelper) refreshRemotes(prevSelectedRemote *models.Remote, env refreshEnv) ([]*models.Remote, error) { remotes, err := self.c.Git().Loaders.RemoteLoader.GetRemotes() if err != nil { - return err + return nil, err } - self.c.Model().Remotes = remotes + self.onUIThreadUnlessRepoChanged(env, func() error { + self.c.Model().Remotes = remotes - hadPrs := len(self.c.Model().PullRequestsMap) != 0 - self.rebuildPullRequestsMap() - if !hadPrs && len(self.c.Model().PullRequestsMap) != 0 { - // if we didn't have PRs in the map before but now we do, we need to redraw the branches view - self.refreshView(self.c.Contexts().Branches) - } + hadPrs := len(self.c.Model().PullRequestsMap) != 0 + self.rebuildPullRequestsMap() + if !hadPrs && len(self.c.Model().PullRequestsMap) != 0 { + // if we didn't have PRs in the map before but now we do, we need to redraw the branches view + self.refreshView(self.c.Contexts().Branches, env) + } - // we need to ensure our selected remote branches aren't now outdated - if prevSelectedRemote != nil && self.c.Model().RemoteBranches != nil { - // find remote now - for _, remote := range remotes { - if remote.Name == prevSelectedRemote.Name { - self.c.Model().RemoteBranches = remote.Branches - break + // we need to ensure our selected remote branches aren't now outdated + if prevSelectedRemote != nil && self.c.Model().RemoteBranches != nil { + // find remote now + for _, remote := range remotes { + if remote.Name == prevSelectedRemote.Name { + self.c.Model().RemoteBranches = remote.Branches + break + } } } - } + return nil + }) - self.refreshView(self.c.Contexts().Remotes) - self.refreshView(self.c.Contexts().RemoteBranches) - return nil + self.refreshView(self.c.Contexts().Remotes, env) + self.refreshView(self.c.Contexts().RemoteBranches, env) + return remotes, nil } -func (self *RefreshHelper) loadWorktrees() { +func (self *RefreshHelper) loadWorktrees() []*models.Worktree { worktrees, err := self.c.Git().Loaders.Worktrees.GetWorktrees() if err != nil { self.c.Log.Error(err) - self.c.Model().Worktrees = []*models.Worktree{} - } else { - self.c.Model().Worktrees = worktrees + return []*models.Worktree{} } + return worktrees } -func (self *RefreshHelper) refreshWorktrees() { - self.loadWorktrees() +func (self *RefreshHelper) refreshWorktrees(env refreshEnv) { + worktrees := self.loadWorktrees() + + self.onUIThreadUnlessRepoChanged(env, func() error { + self.c.Model().Worktrees = worktrees + return nil + }) // need to refresh branches because the branches view shows worktrees against // branches - self.refreshView(self.c.Contexts().Branches) - self.refreshView(self.c.Contexts().Worktrees) + self.refreshView(self.c.Contexts().Branches, env) + self.refreshView(self.c.Contexts().Worktrees, env) } -func (self *RefreshHelper) refreshStashEntries() { - self.c.Model().StashEntries = self.c.Git().Loaders.StashLoader. - GetStashEntries(self.c.Modes().Filtering.GetPath()) +func (self *RefreshHelper) refreshStashEntries(filterPath string, env refreshEnv) { + stashEntries := self.c.Git().Loaders.StashLoader. + GetStashEntries(filterPath) - self.refreshView(self.c.Contexts().Stash) + self.onUIThreadUnlessRepoChanged(env, func() error { + self.c.Model().StashEntries = stashEntries + return nil + }) + + self.refreshView(self.c.Contexts().Stash, env) } // never call this on its own, it should only be called from within refreshCommits() -func (self *RefreshHelper) refreshStatus() { - self.c.Mutexes().RefreshingStatusMutex.Lock() - defer self.c.Mutexes().RefreshingStatusMutex.Unlock() - - currentBranch := self.refsHelper.GetCheckedOutRef() - if currentBranch == nil { - // need to wait for branches to refresh - return - } - +func (self *RefreshHelper) refreshStatus(env refreshEnv) { workingTreeState := self.c.Git().Status.WorkingTreeState() - linkedWorktreeName := self.worktreeHelper.GetLinkedWorktreeName() - repoName := self.c.Git().RepoPaths.RepoName() - status := presentation.FormatStatus(repoName, currentBranch, types.ItemOperationNone, linkedWorktreeName, workingTreeState, self.c.Tr, self.c.UserConfig()) + self.onUIThreadUnlessRepoChanged(env, func() error { + // Read the checked-out branch and the linked worktree name here on the UI + // thread: both derive from models (Branches, Worktrees) that their + // refreshes now write via bounces, so reading them on the worker would + // see stale values from before those bounces applied. + currentBranch := self.refsHelper.GetCheckedOutRef() + if currentBranch == nil { + // need to wait for branches to refresh + return nil + } + linkedWorktreeName := self.worktreeHelper.GetLinkedWorktreeName() - self.c.SetViewContent(self.c.Views().Status, status) + status := presentation.FormatStatus(repoName, currentBranch, types.ItemOperationNone, linkedWorktreeName, workingTreeState, self.c.Tr, self.c.UserConfig()) + self.c.SetViewContent(self.c.Views().Status, status) + return nil + }) } -func (self *RefreshHelper) refForLog() string { +// refForLog returns the ref to log commits from, along with the bisect info it +// read to decide that. The caller writes the bisect info to the model (in its +// bounce) rather than refForLog doing it, so the model write stays on the UI +// thread. +func (self *RefreshHelper) refForLog() (string, *git_commands.BisectInfo) { bisectInfo := self.c.Git().Bisect.GetInfo() - self.c.Model().BisectInfo = bisectInfo if !bisectInfo.Started() { - return "HEAD" + return "HEAD", bisectInfo } // need to see if our bisect's current commit is reachable from our 'new' ref. if bisectInfo.Bisecting() && !self.c.Git().Bisect.ReachableFromStart(bisectInfo) { - return bisectInfo.GetNewHash() + return bisectInfo.GetNewHash(), bisectInfo } - return bisectInfo.GetStartHash() + return bisectInfo.GetStartHash(), bisectInfo } -func (self *RefreshHelper) refreshView(context types.Context) { +func (self *RefreshHelper) refreshView(context types.Context, env refreshEnv) { // 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 { + // refreshes, so bounce to the UI thread before mutating view content. Guard + // on the generation like the model-update bounces do: if the repo was + // switched while the refresh was in flight, its model write was already + // dropped, so there's nothing fresh to render — and the captured context + // belongs to the old repo's now-replaced context tree anyway. + self.onUIThreadUnlessRepoChanged(env, 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) @@ -1023,29 +1452,32 @@ func (self *RefreshHelper) refreshView(context types.Context) { }) } -func (self *RefreshHelper) refreshGithubPullRequests() { - self.c.Mutexes().RefreshingPullRequestsMutex.Lock() - defer self.c.Mutexes().RefreshingPullRequestsMutex.Unlock() +func (self *RefreshHelper) refreshGithubPullRequests(branches []*models.Branch, remotes []*models.Remote, env refreshEnv) { + clearPullRequests := func() { + self.onUIThreadUnlessRepoChanged(env, func() error { + self.c.Model().PullRequests = nil + self.c.Model().PullRequestsMap = nil + return nil + }) + } - githubRemotes := getAuthenticatedGithubRemotes(self.getGithubRemotes(), self.c.Git().GitHub.GetAuthToken) + githubRemotes := getAuthenticatedGithubRemotes(self.getGithubRemotes(remotes), self.c.Git().GitHub.GetAuthToken) if len(githubRemotes) == 0 { - self.c.Model().PullRequests = nil - self.c.Model().PullRequestsMap = nil + clearPullRequests() return } baseInfo := getGithubBaseRemote(githubRemotes, self.c.Git().GitHub.ConfiguredBaseRemoteName()) if baseInfo == nil { - self.c.Model().PullRequests = nil - self.c.Model().PullRequestsMap = nil + clearPullRequests() if !self.githubBaseRemotePromptDismissed[self.c.Git().RepoPaths.RepoPath()] { - self.promptForBaseGithubRepo(githubRemotes) + self.promptForBaseGithubRepo(githubRemotes, branches) } return } - self.setGithubPullRequests(baseInfo) + self.setGithubPullRequests(baseInfo, branches, env) } type githubRemoteInfo struct { @@ -1054,8 +1486,8 @@ type githubRemoteInfo struct { authToken string } -func (self *RefreshHelper) getGithubRemotes() []githubRemoteInfo { - return lo.FilterMap(self.c.Model().Remotes, func(remote *models.Remote, _ int) (githubRemoteInfo, bool) { +func (self *RefreshHelper) getGithubRemotes(remotes []*models.Remote) []githubRemoteInfo { + return lo.FilterMap(remotes, func(remote *models.Remote, _ int) (githubRemoteInfo, bool) { if len(remote.Urls) == 0 { return githubRemoteInfo{}, false } @@ -1116,7 +1548,7 @@ func getGithubBaseRemote(githubRemotes []githubRemoteInfo, configuredRemoteName return nil } -func (self *RefreshHelper) promptForBaseGithubRepo(githubRemotes []githubRemoteInfo) { +func (self *RefreshHelper) promptForBaseGithubRepo(githubRemotes []githubRemoteInfo, branches []*models.Branch) { menuItems := lo.Map(githubRemotes, func(info githubRemoteInfo, _ int) *types.MenuItem { return &types.MenuItem{ LabelColumns: []string{info.remote.Name, style.FgCyan.Sprint(info.serviceInfo.RepoName)}, @@ -1126,7 +1558,11 @@ func (self *RefreshHelper) promptForBaseGithubRepo(githubRemotes []githubRemoteI self.c.Log.Error(err) } - self.setGithubPullRequests(&info) + // This fetch runs on its own worker after the user picked a + // base remote, so it's not part of a performRefresh and has no + // ambient env; build a foreground one now, capturing the + // current generation as the guard baseline. + self.setGithubPullRequests(&info, branches, refreshEnv{generation: self.c.State().GetRepoGeneration()}) return nil }) }, @@ -1154,15 +1590,15 @@ func (self *RefreshHelper) rebuildPullRequestsMap() { ) } -func (self *RefreshHelper) setGithubPullRequests(baseInfo *githubRemoteInfo) { - if len(self.c.Model().Branches) == 0 { +func (self *RefreshHelper) setGithubPullRequests(baseInfo *githubRemoteInfo, branches []*models.Branch, env refreshEnv) { + if len(branches) == 0 { return } - branches := lo.Filter(self.c.Model().Branches, func(branch *models.Branch, _ int) bool { + trackingBranches := lo.Filter(branches, func(branch *models.Branch, _ int) bool { return branch.IsTrackingRemote() }) - branchNames := lo.Map(branches, func(branch *models.Branch, _ int) string { + branchNames := lo.Map(trackingBranches, func(branch *models.Branch, _ int) string { return branch.UpstreamBranch }) @@ -1172,11 +1608,14 @@ func (self *RefreshHelper) setGithubPullRequests(baseInfo *githubRemoteInfo) { return } - self.c.Model().PullRequests = prs self.savePullRequestsToCache(prs) - self.rebuildPullRequestsMap() - self.c.OnUIThread(func() error { + self.onUIThreadUnlessRepoChanged(env, func() error { + self.c.Model().PullRequests = prs + // Rebuilding here rather than on the worker means the map is built from + // the branches and remotes as they are on the UI thread, after their + // own refreshes' bounces have applied. + self.rebuildPullRequestsMap() self.c.PostRefreshUpdate(self.c.Contexts().Branches) return nil }) diff --git a/pkg/gui/controllers/helpers/refs_helper.go b/pkg/gui/controllers/helpers/refs_helper.go index b90b9150b..5f07b8ea6 100644 --- a/pkg/gui/controllers/helpers/refs_helper.go +++ b/pkg/gui/controllers/helpers/refs_helper.go @@ -31,15 +31,6 @@ func NewRefsHelper( } } -func (self *RefsHelper) SelectFirstBranchAndFirstCommit() { - self.c.Contexts().Branches.SetSelection(0) - self.c.Contexts().ReflogCommits.SetSelection(0) - self.c.Contexts().LocalCommits.SetSelection(0) - self.c.Contexts().Branches.GetView().SetOriginY(0) - self.c.Contexts().ReflogCommits.GetView().SetOriginY(0) - self.c.Contexts().LocalCommits.GetView().SetOriginY(0) -} - func (self *RefsHelper) CheckoutRef(ref string, options types.CheckoutRefOptions) error { waitingStatus := options.WaitingStatus if waitingStatus == "" { @@ -49,8 +40,6 @@ func (self *RefsHelper) CheckoutRef(ref string, options types.CheckoutRefOptions cmdOptions := git_commands.CheckoutOptions{Force: false, EnvVars: options.EnvVars} refresh := func() { - self.SelectFirstBranchAndFirstCommit() - // loading a heap of commits is slow so we limit them whenever doing a reset self.c.Contexts().LocalCommits.SetLimitCommits(true) @@ -66,11 +55,12 @@ func (self *RefsHelper) CheckoutRef(ref string, options types.CheckoutRefOptions if options.RefreshPullRequests { scope = append(scope, types.PULL_REQUESTS) } - self.c.Refresh(types.RefreshOptions{ - Mode: types.BLOCK_UI, - Scope: scope, - KeepBranchSelectionIndex: true, - CommitSelection: types.KeepCommitSelectionIndex, + self.c.RefreshFromWorker(types.RefreshOptions{ + Mode: types.BLOCK_UI, + Scope: scope, + BranchSelection: types.SelectCheckedOutBranch, + CommitSelection: types.SelectHeadCommit, + SelectTopReflogCommit: true, }) } @@ -209,12 +199,14 @@ func (self *RefsHelper) ResetToRef(ref string, strength string, envVars []string return err } - self.c.Contexts().LocalCommits.SetSelection(0) - self.c.Contexts().ReflogCommits.SetSelection(0) // loading a heap of commits is slow so we limit them whenever doing a reset self.c.Contexts().LocalCommits.SetLimitCommits(true) - self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.FILES, types.BRANCHES, types.REFLOG, types.COMMITS}, CommitSelection: types.KeepCommitSelectionIndex}) + self.c.RefreshFromWorker(types.RefreshOptions{ + Scope: []types.RefreshableView{types.FILES, types.BRANCHES, types.REFLOG, types.COMMITS}, + CommitSelection: types.SelectHeadCommit, + SelectTopReflogCommit: true, + }) return nil } @@ -375,12 +367,11 @@ func (self *RefsHelper) NewBranch(from string, fromFormattedName string, suggest self.c.Context().Push(self.c.Contexts().Branches, types.OnFocusOpts{}) } - self.SelectFirstBranchAndFirstCommit() - self.c.Refresh(types.RefreshOptions{ - Mode: types.BLOCK_UI, - KeepBranchSelectionIndex: true, - CommitSelection: types.KeepCommitSelectionIndex, + Mode: types.BLOCK_UI, + BranchSelection: types.SelectCheckedOutBranch, + CommitSelection: types.SelectHeadCommit, + SelectTopReflogCommit: true, }) } @@ -435,6 +426,8 @@ func (self *RefsHelper) MoveCommitsToNewBranch() error { return err } + mustStash := IsWorkingTreeDirtyExceptSubmodules(self.c.Model().Files, self.c.Model().Submodules) + withNewBranchNamePrompt := func(baseBranchName string, f func(string) error) error { prompt := utils.ResolvePlaceholderString( self.c.Tr.NewBranchNameBranchOff, @@ -473,7 +466,9 @@ func (self *RefsHelper) MoveCommitsToNewBranch() error { Title: self.c.Tr.MoveCommitsToNewBranch, Prompt: prompt, HandleConfirm: func() error { - return withNewBranchNamePrompt(currentBranch.Name, self.moveCommitsToNewBranchStackedOnCurrentBranch) + return withNewBranchNamePrompt(currentBranch.Name, func(newBranchName string) error { + return self.moveCommitsToNewBranchStackedOnCurrentBranch(newBranchName, mustStash) + }) }, }) return nil @@ -493,27 +488,31 @@ func (self *RefsHelper) MoveCommitsToNewBranch() error { { Label: fmt.Sprintf(self.c.Tr.MoveCommitsToNewBranchFromBaseItem, shortBaseBranchName), OnPress: func() error { + commitsToCherryPick := lo.Filter(self.c.Model().Commits, func(commit *models.Commit, _ int) bool { + return commit.Status == models.StatusUnpushed + }) return withNewBranchNamePrompt(shortBaseBranchName, func(newBranchName string) error { - return self.moveCommitsToNewBranchOffOfMainBranch(newBranchName, baseBranchRef) + return self.moveCommitsToNewBranchOffOfMainBranch(newBranchName, baseBranchRef, commitsToCherryPick, mustStash) }) }, }, { Label: fmt.Sprintf(self.c.Tr.MoveCommitsToNewBranchStackedItem, currentBranch.Name), OnPress: func() error { - return withNewBranchNamePrompt(currentBranch.Name, self.moveCommitsToNewBranchStackedOnCurrentBranch) + return withNewBranchNamePrompt(currentBranch.Name, func(newBranchName string) error { + return self.moveCommitsToNewBranchStackedOnCurrentBranch(newBranchName, mustStash) + }) }, }, }, }) } -func (self *RefsHelper) moveCommitsToNewBranchStackedOnCurrentBranch(newBranchName string) error { +func (self *RefsHelper) moveCommitsToNewBranchStackedOnCurrentBranch(newBranchName string, mustStash bool) error { if err := self.c.Git().Branch.NewWithoutCheckout(newBranchName, "HEAD"); err != nil { return err } - mustStash := IsWorkingTreeDirtyExceptSubmodules(self.c.Model().Files, self.c.Model().Submodules) if mustStash { if err := self.c.Git().Stash.Push(fmt.Sprintf(self.c.Tr.AutoStashForNewBranch, newBranchName)); err != nil { return err @@ -534,22 +533,16 @@ func (self *RefsHelper) moveCommitsToNewBranchStackedOnCurrentBranch(newBranchNa } } - self.SelectFirstBranchAndFirstCommit() - - self.c.Refresh(types.RefreshOptions{ - Mode: types.BLOCK_UI, - KeepBranchSelectionIndex: true, - CommitSelection: types.KeepCommitSelectionIndex, + self.c.RefreshFromWorker(types.RefreshOptions{ + Mode: types.BLOCK_UI, + BranchSelection: types.SelectCheckedOutBranch, + CommitSelection: types.SelectHeadCommit, + SelectTopReflogCommit: true, }) return nil } -func (self *RefsHelper) moveCommitsToNewBranchOffOfMainBranch(newBranchName string, baseBranchRef string) error { - commitsToCherryPick := lo.Filter(self.c.Model().Commits, func(commit *models.Commit, _ int) bool { - return commit.Status == models.StatusUnpushed - }) - - mustStash := IsWorkingTreeDirtyExceptSubmodules(self.c.Model().Files, self.c.Model().Submodules) +func (self *RefsHelper) moveCommitsToNewBranchOffOfMainBranch(newBranchName string, baseBranchRef string, commitsToCherryPick []*models.Commit, mustStash bool) error { if mustStash { if err := self.c.Git().Stash.Push(fmt.Sprintf(self.c.Tr.AutoStashForNewBranch, newBranchName)); err != nil { return err @@ -576,12 +569,11 @@ func (self *RefsHelper) moveCommitsToNewBranchOffOfMainBranch(newBranchName stri } } - self.SelectFirstBranchAndFirstCommit() - - self.c.Refresh(types.RefreshOptions{ - Mode: types.BLOCK_UI, - KeepBranchSelectionIndex: true, - CommitSelection: types.KeepCommitSelectionIndex, + self.c.RefreshFromWorker(types.RefreshOptions{ + Mode: types.BLOCK_UI, + BranchSelection: types.SelectCheckedOutBranch, + CommitSelection: types.SelectHeadCommit, + SelectTopReflogCommit: true, }) return nil } diff --git a/pkg/gui/controllers/helpers/repos_helper.go b/pkg/gui/controllers/helpers/repos_helper.go index bde1c47c6..a61ad0013 100644 --- a/pkg/gui/controllers/helpers/repos_helper.go +++ b/pkg/gui/controllers/helpers/repos_helper.go @@ -13,7 +13,6 @@ import ( "github.com/jesseduffield/lazygit/pkg/commands/direnv" "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/env" - "github.com/jesseduffield/lazygit/pkg/gocui" "github.com/jesseduffield/lazygit/pkg/gui/context" "github.com/jesseduffield/lazygit/pkg/gui/presentation/icons" "github.com/jesseduffield/lazygit/pkg/gui/style" @@ -44,13 +43,20 @@ func NewRecentReposHelper( } func (self *ReposHelper) EnterSubmodule(submodule *models.SubmoduleConfig) error { + // Check before pushing onto the repo-path stack, so a refused switch + // doesn't leave a stale entry there (which escape would later switch back + // to, needlessly reloading the current repo). + if self.switchRefusedBecauseBusy() { + return nil + } + wd, err := os.Getwd() if err != nil { return err } self.c.State().GetRepoPathStack().Push(wd) - return self.DispatchSwitchToRepo(submodule.FullPath(), context.NO_CONTEXT) + return self.switchTo(submodule.FullPath(), self.c.Tr.ErrRepositoryMovedOrDeleted, context.NO_CONTEXT) } func (self *ReposHelper) getCurrentBranch(path string) string { @@ -130,10 +136,16 @@ func (self *ReposHelper) CreateRecentReposMenu() error { style.FgMagenta.Sprint(path), }, OnPress: func() error { + // Check before clearing the stack, so a refused switch doesn't + // forget the submodule breadcrumb (which would leave escape + // unable to return to the parent repo). + if self.switchRefusedBecauseBusy() { + return nil + } // if we were in a submodule, we want to forget about that stack of repos // so that hitting escape in the new repo does nothing self.c.State().GetRepoPathStack().Clear() - return self.DispatchSwitchToRepo(path, context.NO_CONTEXT) + return self.switchTo(path, self.c.Tr.ErrRepositoryMovedOrDeleted, context.NO_CONTEXT) }, } }) @@ -141,59 +153,89 @@ func (self *ReposHelper) CreateRecentReposMenu() error { return self.c.Menu(types.CreateMenuOptions{Title: self.c.Tr.RecentRepos, Items: menuItems}) } -func (self *ReposHelper) DispatchSwitchToRepo(path string, contextKey types.ContextKey) error { - return self.DispatchSwitchTo(path, self.c.Tr.ErrRepositoryMovedOrDeleted, contextKey) +// SwitchToParentRepo switches back to the repo the current submodule was +// entered from (the top of the repo-path stack). Like the other callers that do +// work before switching, it checks for an in-flight operation *before* popping +// the stack, so a refused switch leaves the stack intact — otherwise the entry +// would be consumed and escape would no longer return to the parent once the +// operation finished. The caller must only call this when the stack is +// non-empty. +func (self *ReposHelper) SwitchToParentRepo() error { + if self.switchRefusedBecauseBusy() { + return nil + } + return self.switchTo(self.c.State().GetRepoPathStack().Pop(), self.c.Tr.ErrRepositoryMovedOrDeleted, context.NO_CONTEXT) } func (self *ReposHelper) DispatchSwitchTo(path string, errMsg string, contextKey types.ContextKey) error { - return self.c.WithWaitingStatus(self.c.Tr.Switching, func(gocui.Task) error { - env.UnsetGitLocationEnvVars() - originalPath, err := os.Getwd() - if err != nil { - return nil + if self.switchRefusedBecauseBusy() { + return nil + } + return self.switchTo(path, errMsg, contextKey) +} + +// switchRefusedBecauseBusy reports (and shows a toast) whether a repo switch +// must be refused because a foreground git operation is in flight. Switching +// reassigns gui.git and the process cwd, so switching mid-operation would run +// the operation's remaining git commands against the wrong repo. Callers that +// do work before the switch (creating a worktree, recording the repo-path +// stack) check this up front, so they don't do that work only to have the +// switch refused; the switch itself (switchTo) is then unguarded. +func (self *ReposHelper) switchRefusedBecauseBusy() bool { + if self.c.GocuiGui().Busy() { + self.c.ErrorToast(self.c.Tr.CantSwitchWhileOperationInProgress) + return true + } + return false +} + +// switchTo switches lazygit to the repository (or worktree) at the given path. +// It runs synchronously on the UI thread: the switch swaps gui.State (in +// resetState) and reassigns gui.git and the process cwd, all of which the UI +// thread also reads, so doing it here rather than on a worker avoids racing +// those reads. The heavy data loading is still dispatched asynchronously by the +// refresh that onNewRepo kicks off. +func (self *ReposHelper) switchTo(path string, errMsg string, contextKey types.ContextKey) error { + env.UnsetGitLocationEnvVars() + originalPath, err := os.Getwd() + if err != nil { + return nil + } + + msg := utils.ResolvePlaceholderString(self.c.Tr.ChangingDirectoryTo, map[string]string{"path": path}) + self.c.LogCommand(msg, false) + + if err := os.Chdir(path); err != nil { + if os.IsNotExist(err) { + return errors.New(errMsg) } + return err + } - msg := utils.ResolvePlaceholderString(self.c.Tr.ChangingDirectoryTo, map[string]string{"path": path}) - self.c.LogCommand(msg, false) - - if err := os.Chdir(path); err != nil { - if os.IsNotExist(err) { - return errors.New(errMsg) - } + if err := commands.VerifyInGitRepo(self.c.OS()); err != nil { + if err := os.Chdir(originalPath); err != nil { return err } - if err := commands.VerifyInGitRepo(self.c.OS()); err != nil { - if err := os.Chdir(originalPath); err != nil { - return err - } + return err + } - return err - } + direnvResult := self.logDirenvResult(direnv.Load(self.c.OS().Cmd)) - direnvResult := self.logDirenvResult(direnv.Load(self.c.OS().Cmd)) + if err := self.recordDirectoryHelper.RecordCurrentDirectory(); err != nil { + self.c.Log.Errorf("error recording current directory: %v", err) + } - if err := self.recordDirectoryHelper.RecordCurrentDirectory(); err != nil { - self.c.Log.Errorf("error recording current directory: %v", err) - } + if err := self.onNewRepo(appTypes.StartArgs{}, contextKey); err != nil { + return err + } - self.c.Mutexes().RefreshingFilesMutex.Lock() - defer self.c.Mutexes().RefreshingFilesMutex.Unlock() + if direnvResult.Blocked { + self.promptDirenvApproval(direnvResult.EnvrcPath) + return nil + } - if err := self.onNewRepo(appTypes.StartArgs{}, contextKey); err != nil { - return err - } - - if direnvResult.Blocked { - self.c.OnUIThread(func() error { - self.promptDirenvApproval(direnvResult.EnvrcPath) - return nil - }) - return nil - } - - return direnvResult.Err - }) + return direnvResult.Err } // logDirenvResult writes whatever direnv emitted to the command log and the diff --git a/pkg/gui/controllers/helpers/sub_commits_helper.go b/pkg/gui/controllers/helpers/sub_commits_helper.go index fbf100e16..7bd928826 100644 --- a/pkg/gui/controllers/helpers/sub_commits_helper.go +++ b/pkg/gui/controllers/helpers/sub_commits_helper.go @@ -49,7 +49,7 @@ func (self *SubCommitsHelper) ViewSubCommits(opts ViewSubCommitsOpts) error { return err } - self.setSubCommits(commits) + self.c.Model().SubCommits = commits self.refreshHelper.RefreshAuthors(commits) subCommitsContext := self.c.Contexts().SubCommits @@ -71,10 +71,3 @@ func (self *SubCommitsHelper) ViewSubCommits(opts ViewSubCommitsOpts) error { self.c.Context().Push(self.c.Contexts().SubCommits, types.OnFocusOpts{}) return nil } - -func (self *SubCommitsHelper) setSubCommits(commits []*models.Commit) { - self.c.Mutexes().SubCommitsMutex.Lock() - defer self.c.Mutexes().SubCommitsMutex.Unlock() - - self.c.Model().SubCommits = commits -} diff --git a/pkg/gui/controllers/helpers/suggestions_helper.go b/pkg/gui/controllers/helpers/suggestions_helper.go index d26f96f1c..8a5916816 100644 --- a/pkg/gui/controllers/helpers/suggestions_helper.go +++ b/pkg/gui/controllers/helpers/suggestions_helper.go @@ -137,10 +137,12 @@ func (self *SuggestionsHelper) GetFilePathSuggestionsFunc() func(string) []*type trie.Insert(patricia.Prefix(file), file) } - // cache the trie for future use - self.c.Model().FilesTrie = trie - - self.c.Contexts().Suggestions.RefreshSuggestions() + self.c.OnUIThread(func() error { + // cache the trie for future use + self.c.Model().FilesTrie = trie + self.c.Contexts().Suggestions.RefreshSuggestions() + return nil + }) return err }) diff --git a/pkg/gui/controllers/helpers/working_tree_helper.go b/pkg/gui/controllers/helpers/working_tree_helper.go index 7e070ba31..36dfd2032 100644 --- a/pkg/gui/controllers/helpers/working_tree_helper.go +++ b/pkg/gui/controllers/helpers/working_tree_helper.go @@ -149,7 +149,12 @@ func (self *WorkingTreeHelper) handleCommit(summary string, description string, self.c.LogAction(self.c.Tr.Actions.Commit) return self.gpgHelper.WithGpgHandlingAndSelectHeadCommit(cmdObj, git_commands.CommitGpgSign, self.c.Tr.CommittingStatus, func() error { - self.commitsHelper.ClearPreservedCommitMessage() + // This runs on a worker when the commit output is streamed, so + // bounce the preserved-message write to the UI thread. + self.c.OnUIThread(func() error { + self.commitsHelper.ClearPreservedCommitMessage() + return nil + }) return nil }) } @@ -222,15 +227,24 @@ func (self *WorkingTreeHelper) HandleCommitPress() error { } func (self *WorkingTreeHelper) WithEnsureCommittableFiles(handler func() error) error { - if err := self.prepareFilesForCommit(); err != nil { - return err - } - if len(self.c.Model().Files) == 0 { return errors.New(self.c.Tr.NoFilesStagedTitle) } if !self.AnyStagedFiles() { + if self.c.UserConfig().Gui.SkipNoStagedFilesWarning { + self.c.LogAction(self.c.Tr.Actions.StageAllFiles) + if err := self.c.Git().WorkingTree.StageAll(false); err != nil { + return err + } + self.c.Refresh(types.RefreshOptions{ + Mode: types.SYNC, + Scope: []types.RefreshableView{types.FILES}, + Then: handler, + }) + return nil + } + return self.promptToStageAllAndRetry(handler) } @@ -246,7 +260,7 @@ func (self *WorkingTreeHelper) promptToStageAllAndRetry(retry func() error) erro if err := self.c.Git().WorkingTree.StageAll(false); err != nil { return err } - self.syncRefresh() + self.c.Refresh(types.RefreshOptions{Mode: types.SYNC, Scope: []types.RefreshableView{types.FILES}}) return retry() }, @@ -255,26 +269,6 @@ func (self *WorkingTreeHelper) promptToStageAllAndRetry(retry func() error) erro return nil } -// for when you need to refetch files before continuing an action. Runs synchronously. -func (self *WorkingTreeHelper) syncRefresh() { - self.c.Refresh(types.RefreshOptions{Mode: types.SYNC, Scope: []types.RefreshableView{types.FILES}}) -} - -func (self *WorkingTreeHelper) prepareFilesForCommit() error { - noStagedFiles := !self.AnyStagedFiles() - if noStagedFiles && self.c.UserConfig().Gui.SkipNoStagedFilesWarning { - self.c.LogAction(self.c.Tr.Actions.StageAllFiles) - err := self.c.Git().WorkingTree.StageAll(false) - if err != nil { - return err - } - - self.syncRefresh() - } - - return nil -} - func (self *WorkingTreeHelper) commitPrefixConfigsForRepo() []config.CommitPrefixConfig { cfg, ok := self.c.UserConfig().Git.CommitPrefixes[self.c.Git().RepoPaths.RepoName()] if ok { diff --git a/pkg/gui/controllers/helpers/worktree_helper.go b/pkg/gui/controllers/helpers/worktree_helper.go index 7cec9f873..980d810ae 100644 --- a/pkg/gui/controllers/helpers/worktree_helper.go +++ b/pkg/gui/controllers/helpers/worktree_helper.go @@ -163,7 +163,7 @@ func (self *WorktreeHelper) remove(worktree *models.Worktree, force bool, then f return then(task) } - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.WORKTREES, types.BRANCHES, types.FILES}}) + self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.WORKTREES, types.BRANCHES, types.FILES}}) return nil }) } @@ -181,7 +181,7 @@ func (self *WorktreeHelper) Detach(worktree *models.Worktree, then func(gocui.Ta return then(task) } - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.WORKTREES, types.BRANCHES, types.FILES}}) + self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.WORKTREES, types.BRANCHES, types.FILES}}) return nil }) } @@ -426,12 +426,27 @@ func (self *WorktreeHelper) promptForWorktreeLocation(dirName string, prompt str } func (self *WorktreeHelper) createWorktree(opts git_commands.NewWorktreeOpts, contextKey types.ContextKey) error { + // Check now, before we create the worktree, rather than when we come to + // switch to it afterwards: by then this operation's own waiting-status + // spinner would make Busy() true and refuse our own switch. + if self.reposHelper.switchRefusedBecauseBusy() { + return nil + } + return self.c.WithWaitingStatus(self.c.Tr.AddingWorktree, func(gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.AddWorktree) if err := self.c.Git().Worktree.New(opts); err != nil { return err } - return self.reposHelper.DispatchSwitchTo(opts.Path, self.c.Tr.ErrWorktreeMovedOrRemoved, contextKey) + // The switch swaps gui.State and must run on the UI thread, but + // we're on a worker here (creating the worktree is git work), so + // dispatch it. It's unguarded (switchTo, not DispatchSwitchTo) + // because we checked above and creating the worktree is now + // complete, so switching to it is safe. + self.c.OnUIThread(func() error { + return self.reposHelper.switchTo(opts.Path, self.c.Tr.ErrWorktreeMovedOrRemoved, contextKey) + }) + return nil }) } diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 9f3acb1d2..23e94adf7 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -340,9 +340,11 @@ func (self *LocalCommitsController) squashDown(selectedCommits []*models.Commit, Title: self.c.Tr.Squash, Prompt: self.c.Tr.SureSquashThisCommit, HandleConfirm: func() error { + commits := self.c.Model().Commits + self.selectRebaseResultCommit(startIdx) return self.c.WithWaitingStatus(self.c.Tr.SquashingStatus, func(gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.SquashCommitDown) - return self.interactiveRebase(todo.Squash, startIdx, endIdx) + return self.interactiveRebase(commits, todo.Squash, startIdx, endIdx) }) }, }) @@ -362,9 +364,11 @@ func (self *LocalCommitsController) fixup(selectedCommits []*models.Commit, star Label: self.c.Tr.Fixup, Keys: menuKey('f'), OnPress: func() error { + commits := self.c.Model().Commits + self.selectRebaseResultCommit(startIdx) return self.c.WithWaitingStatus(self.c.Tr.FixingStatus, func(gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.FixupCommit) - return self.interactiveRebase(todo.Fixup, startIdx, endIdx) + return self.interactiveRebase(commits, todo.Fixup, startIdx, endIdx) }) }, Tooltip: self.c.Tr.FixupTooltip, @@ -373,9 +377,11 @@ func (self *LocalCommitsController) fixup(selectedCommits []*models.Commit, star Label: self.c.Tr.FixupKeepMessage, Keys: menuKey('c'), OnPress: func() error { + commits := self.c.Model().Commits + self.selectRebaseResultCommit(startIdx) return self.c.WithWaitingStatus(self.c.Tr.FixingStatus, func(gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.FixupCommitKeepMessage) - return self.interactiveRebaseWithFlag(todo.Fixup, startIdx, endIdx, "-C") + return self.interactiveRebaseWithFlag(commits, todo.Fixup, startIdx, endIdx, "-C") }) }, Tooltip: self.c.Tr.FixupKeepMessageTooltip, @@ -475,7 +481,9 @@ func (self *LocalCommitsController) switchFromCommitMessagePanelToEditor(filepat } func (self *LocalCommitsController) handleReword(summary string, description string) error { - if models.IsHeadCommit(self.c.Model().Commits, self.c.Contexts().LocalCommits.GetSelectedLineIdx()) { + commits := self.c.Model().Commits + selectedIdx := self.c.Contexts().LocalCommits.GetSelectedLineIdx() + if models.IsHeadCommit(commits, selectedIdx) { // we've selected the top commit so no rebase is required return self.c.Helpers().GPG.WithGpgHandling(self.c.Git().Commit.RewordLastCommit(summary, description), git_commands.CommitGpgSign, @@ -483,11 +491,11 @@ func (self *LocalCommitsController) handleReword(summary string, description str } return self.c.WithWaitingStatus(self.c.Tr.RewordingStatus, func(gocui.Task) error { - err := self.c.Git().Rebase.RewordCommit(self.c.Model().Commits, self.c.Contexts().LocalCommits.GetSelectedLineIdx(), summary, description) + err := self.c.Git().Rebase.RewordCommit(commits, selectedIdx, summary, description) if err != nil { return err } - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC}) return nil }) } @@ -564,12 +572,16 @@ func (self *LocalCommitsController) drop(selectedCommits []*models.Commit, start Title: self.c.Tr.DropCommitTitle, Prompt: lo.Ternary(isMerge, self.c.Tr.DropMergeCommitPrompt, self.c.Tr.DropCommitPrompt), HandleConfirm: func() error { + commits := self.c.Model().Commits + if !isMerge { + self.selectRebaseResultCommit(startIdx) + } return self.c.WithWaitingStatus(self.c.Tr.DroppingStatus, func(gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.DropCommit) if isMerge { - return self.dropMergeCommit(startIdx) + return self.dropMergeCommit(commits, startIdx) } - return self.interactiveRebase(todo.Drop, startIdx, endIdx) + return self.interactiveRebase(commits, todo.Drop, startIdx, endIdx) }) }, }) @@ -577,8 +589,8 @@ func (self *LocalCommitsController) drop(selectedCommits []*models.Commit, start return nil } -func (self *LocalCommitsController) dropMergeCommit(commitIdx int) error { - err := self.c.Git().Rebase.DropMergeCommit(self.c.Model().Commits, commitIdx) +func (self *LocalCommitsController) dropMergeCommit(commits []*models.Commit, commitIdx int) error { + err := self.c.Git().Rebase.DropMergeCommit(commits, commitIdx) return self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(err) } @@ -616,7 +628,7 @@ func (self *LocalCommitsController) startInteractiveRebaseWithEdit( err := self.c.Git().Rebase.EditRebase(commitsToEdit[len(commitsToEdit)-1].Hash()) return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptions( err, - types.RefreshOptions{Mode: types.BLOCK_UI, Then: func() { + types.RefreshOptions{Mode: types.BLOCK_UI, Then: func() error { todos := make([]*models.Commit, 0, len(commitsToEdit)-1) for _, c := range commitsToEdit[:len(commitsToEdit)-1] { // Merge commits can't be set to "edit", so just skip them @@ -625,11 +637,9 @@ func (self *LocalCommitsController) startInteractiveRebaseWithEdit( } } if len(todos) > 0 { - err := self.updateTodos(todo.Edit, todos) - if err != nil { - self.c.Log.Errorf("error when updating todos: %v", err) - } + return self.updateTodos(todo.Edit, todos) } + return nil }}) }) } @@ -658,22 +668,25 @@ func (self *LocalCommitsController) pick(selectedCommits []*models.Commit) error panic("should be disabled when not rebasing") } -func (self *LocalCommitsController) interactiveRebase(action todo.TodoCommand, startIdx int, endIdx int) error { - return self.interactiveRebaseWithFlag(action, startIdx, endIdx, "") +func (self *LocalCommitsController) interactiveRebase(commits []*models.Commit, action todo.TodoCommand, startIdx int, endIdx int) error { + return self.interactiveRebaseWithFlag(commits, action, startIdx, endIdx, "") } -func (self *LocalCommitsController) interactiveRebaseWithFlag(action todo.TodoCommand, startIdx int, endIdx int, flag string) error { - // When performing an action that will remove the selected commits, we need to select the - // next commit down (which will end up at the start index after the action is performed) - if action == todo.Drop || action == todo.Fixup || action == todo.Squash { - self.context().SetSelection(startIdx) - } - - err := self.c.Git().Rebase.InteractiveRebase(self.c.Model().Commits, startIdx, endIdx, action, flag) +func (self *LocalCommitsController) interactiveRebaseWithFlag(commits []*models.Commit, action todo.TodoCommand, startIdx int, endIdx int, flag string) error { + err := self.c.Git().Rebase.InteractiveRebase(commits, startIdx, endIdx, action, flag) return self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(err) } +// selectRebaseResultCommit selects the commit that a drop/fixup/squash starting +// at startIdx will leave there. It must run on the UI thread before the rebase: +// the commit currently at startIdx is removed, so the refresh's +// keep-selection-by-hash can't restore it and falls back to the index, which by +// then holds the commit that shifted up into its place. +func (self *LocalCommitsController) selectRebaseResultCommit(startIdx int) { + self.context().SetSelection(startIdx) +} + // updateTodos sees if the selected commit is in fact a rebasing // commit meaning you are trying to edit the todo file rather than actually // begin a rebase. It then updates the todo file with that action @@ -743,7 +756,7 @@ func (self *LocalCommitsController) moveDown(selectedCommits []*models.Commit, s self.context().MoveSelection(1) self.context().HandleFocus(types.OnFocusOpts{ScrollSelectionIntoView: true}) } - return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptions( + return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptionsFromUIThread( err, types.RefreshOptions{Mode: types.SYNC, CommitSelection: types.KeepCommitSelectionIndex}) }) } @@ -771,7 +784,7 @@ func (self *LocalCommitsController) moveUp(selectedCommits []*models.Commit, sta self.context().MoveSelection(-1) self.context().HandleFocus(types.OnFocusOpts{ScrollSelectionIntoView: true}) } - return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptions( + return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptionsFromUIThread( err, types.RefreshOptions{Mode: types.SYNC, CommitSelection: types.KeepCommitSelectionIndex}) }) } @@ -790,11 +803,13 @@ func (self *LocalCommitsController) amendTo(commit *models.Commit) error { }) } } else { + commits := self.c.Model().Commits + selectedIdx := self.context().GetView().SelectedLineIdx() handleCommit = func() error { return self.c.Helpers().WorkingTree.WithEnsureCommittableFiles(func() error { return self.c.WithWaitingStatus(self.c.Tr.AmendingStatus, func(gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.AmendCommit) - err := self.c.Git().Rebase.AmendTo(self.c.Model().Commits, self.context().GetView().SelectedLineIdx()) + err := self.c.Git().Rebase.AmendTo(commits, selectedIdx) return self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(err) }) }) @@ -822,26 +837,30 @@ func (self *LocalCommitsController) canAmend(_ *models.Commit) *types.DisabledRe return self.canAmendRange(self.c.Model().Commits, idx, idx) } -func (self *LocalCommitsController) amendAttribute(commits []*models.Commit, start, end int) error { +func (self *LocalCommitsController) amendAttribute(_ []*models.Commit, start, end int) error { + // The author operations index into the full commit list by absolute + // start/end, so capture that here on the UI thread rather than reading + // Model().Commits from the worker the menu items dispatch to. + commits := self.c.Model().Commits opts := self.c.KeybindingsOpts() return self.c.Menu(types.CreateMenuOptions{ Title: "Amend commit attribute", Items: []*types.MenuItem{ { Label: self.c.Tr.ResetAuthor, - OnPress: func() error { return self.resetAuthor(start, end) }, + OnPress: func() error { return self.resetAuthor(commits, start, end) }, Keys: opts.GetKeys(opts.Config.AmendAttribute.ResetAuthor), Tooltip: self.c.Tr.ResetAuthorTooltip, }, { Label: self.c.Tr.SetAuthor, - OnPress: func() error { return self.setAuthor(start, end) }, + OnPress: func() error { return self.setAuthor(commits, start, end) }, Keys: opts.GetKeys(opts.Config.AmendAttribute.SetAuthor), Tooltip: self.c.Tr.SetAuthorTooltip, }, { Label: self.c.Tr.AddCoAuthor, - OnPress: func() error { return self.addCoAuthor(start, end) }, + OnPress: func() error { return self.addCoAuthor(commits, start, end) }, Keys: opts.GetKeys(opts.Config.AmendAttribute.AddCoAuthor), Tooltip: self.c.Tr.AddCoAuthorTooltip, }, @@ -849,30 +868,30 @@ func (self *LocalCommitsController) amendAttribute(commits []*models.Commit, sta }) } -func (self *LocalCommitsController) resetAuthor(start, end int) error { +func (self *LocalCommitsController) resetAuthor(commits []*models.Commit, start, end int) error { return self.c.WithWaitingStatus(self.c.Tr.AmendingStatus, func(gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.ResetCommitAuthor) - if err := self.c.Git().Rebase.ResetCommitAuthor(self.c.Model().Commits, start, end); err != nil { + if err := self.c.Git().Rebase.ResetCommitAuthor(commits, start, end); err != nil { return err } - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC}) return nil }) } -func (self *LocalCommitsController) setAuthor(start, end int) error { +func (self *LocalCommitsController) setAuthor(commits []*models.Commit, start, end int) error { self.c.Prompt(types.PromptOpts{ Title: self.c.Tr.SetAuthorPromptTitle, FindSuggestionsFunc: self.c.Helpers().Suggestions.GetAuthorsSuggestionsFunc(), HandleConfirm: func(value string) error { return self.c.WithWaitingStatus(self.c.Tr.AmendingStatus, func(gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.SetCommitAuthor) - if err := self.c.Git().Rebase.SetCommitAuthor(self.c.Model().Commits, start, end, value); err != nil { + if err := self.c.Git().Rebase.SetCommitAuthor(commits, start, end, value); err != nil { return err } - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC}) return nil }) }, @@ -881,17 +900,17 @@ func (self *LocalCommitsController) setAuthor(start, end int) error { return nil } -func (self *LocalCommitsController) addCoAuthor(start, end int) error { +func (self *LocalCommitsController) addCoAuthor(commits []*models.Commit, start, end int) error { self.c.Prompt(types.PromptOpts{ Title: self.c.Tr.AddCoAuthorPromptTitle, FindSuggestionsFunc: self.c.Helpers().Suggestions.GetAuthorsSuggestionsFunc(), HandleConfirm: func(value string) error { return self.c.WithWaitingStatus(self.c.Tr.AmendingStatus, func(gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.AddCommitCoAuthor) - if err := self.c.Git().Rebase.AddCommitCoAuthor(self.c.Model().Commits, start, end, value); err != nil { + if err := self.c.Git().Rebase.AddCommitCoAuthor(commits, start, end, value); err != nil { return err } - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC}) return nil }) }, @@ -929,7 +948,7 @@ func (self *LocalCommitsController) revert(commits []*models.Commit, start, end } result := self.c.Git().Commit.Revert(hashes, isMerge) - if err := self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptions(result, types.RefreshOptions{Mode: types.SYNC}); err != nil { + if err := self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptionsFromUIThread(result, types.RefreshOptions{Mode: types.SYNC}); err != nil { return err } @@ -1129,7 +1148,7 @@ func (self *LocalCommitsController) squashFixupsImpl(commit *models.Commit, reba self.c.LogAction(self.c.Tr.Actions.SquashAllAboveFixupCommits) err := self.c.Git().Rebase.SquashAllAboveFixupCommits(commit) self.context().MoveSelectedLine(-selectionOffset) - return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptions( + return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptionsFromUIThread( err, types.RefreshOptions{Mode: types.SYNC}) }) } diff --git a/pkg/gui/controllers/patch_building_controller.go b/pkg/gui/controllers/patch_building_controller.go index 5e4a17169..d596c2ead 100644 --- a/pkg/gui/controllers/patch_building_controller.go +++ b/pkg/gui/controllers/patch_building_controller.go @@ -228,7 +228,7 @@ func (self *PatchBuildingController) discardSelectionFromCommit() error { self.c.LogAction(self.c.Tr.Actions.RemovePatchFromCommit) err := self.c.Git().Patch.DeletePatchesFromCommit(self.c.Model().Commits, commitIndex) self.c.Helpers().PatchBuilding.Escape() - return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptions( + return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptionsFromUIThread( err, types.RefreshOptions{Mode: types.SYNC}) }) } diff --git a/pkg/gui/controllers/quit_actions.go b/pkg/gui/controllers/quit_actions.go index 40ad6f7e3..9a7082542 100644 --- a/pkg/gui/controllers/quit_actions.go +++ b/pkg/gui/controllers/quit_actions.go @@ -2,7 +2,6 @@ package controllers import ( "github.com/jesseduffield/lazygit/pkg/gocui" - "github.com/jesseduffield/lazygit/pkg/gui/context" "github.com/jesseduffield/lazygit/pkg/gui/types" ) @@ -81,9 +80,8 @@ func (self *QuitActions) Escape() error { } } - repoPathStack := self.c.State().GetRepoPathStack() - if !repoPathStack.IsEmpty() { - return self.c.Helpers().Repos.DispatchSwitchToRepo(repoPathStack.Pop(), context.NO_CONTEXT) + if !self.c.State().GetRepoPathStack().IsEmpty() { + return self.c.Helpers().Repos.SwitchToParentRepo() } if self.c.UserConfig().QuitOnTopLevelReturn { diff --git a/pkg/gui/controllers/remotes_controller.go b/pkg/gui/controllers/remotes_controller.go index f7b16e228..d4c838f7c 100644 --- a/pkg/gui/controllers/remotes_controller.go +++ b/pkg/gui/controllers/remotes_controller.go @@ -156,24 +156,28 @@ func (self *RemotesController) addAndCheckoutRemote(remoteName string, remoteUrl return err } - // Do a sync refresh of the remotes so that we can select - // the new one. Loading remotes is not expensive, so we can - // afford it. + // Refresh the remotes so that we can select the new one. The remotes model + // update is bounced onto the UI thread, so the selection (which reads + // Model.Remotes) has to run in Then; reading it inline here would see the + // previous model. Loading remotes is not expensive, so a sync refresh is + // affordable. self.c.Refresh(types.RefreshOptions{ Scope: []types.RefreshableView{types.REMOTES}, Mode: types.SYNC, + Then: func() error { + // Select the remote + for idx, remote := range self.c.Model().Remotes { + if remote.Name == remoteName { + self.c.Contexts().Remotes.SetSelection(idx) + break + } + } + + // Fetch the remote + return self.fetchAndCheckout(self.c.Contexts().Remotes.GetSelected(), branchToCheckout) + }, }) - - // Select the remote - for idx, remote := range self.c.Model().Remotes { - if remote.Name == remoteName { - self.c.Contexts().Remotes.SetSelection(idx) - break - } - } - - // Fetch the remote - return self.fetchAndCheckout(self.c.Contexts().Remotes.GetSelected(), branchToCheckout) + return nil } // Ensures the fork remote exists (matching the given URL). @@ -372,13 +376,22 @@ func (self *RemotesController) fetchAndCheckout(remote *models.Remote, branchNam if branchName != "" { err = self.c.Git().Branch.New(branchName, remote.Name+"/"+branchName) if err == nil { - self.c.Context().Push(self.c.Contexts().Branches, types.OnFocusOpts{}) - self.c.Helpers().Refs.SelectFirstBranchAndFirstCommit() - refreshOptions.KeepBranchSelectionIndex = true - refreshOptions.CommitSelection = types.KeepCommitSelectionIndex + // Branch.New checks the new branch out, so HEAD moves: refresh the + // reflog (and, via scope expansion, the commits) as well, and select + // the newly checked-out branch and its head commit. + refreshOptions.Scope = append(refreshOptions.Scope, types.REFLOG) + refreshOptions.BranchSelection = types.SelectCheckedOutBranch + refreshOptions.CommitSelection = types.SelectHeadCommit + refreshOptions.SelectTopReflogCommit = true + // Focus the branches panel on the UI thread once the refresh has + // selected the newly checked-out branch. + refreshOptions.Then = func() error { + self.c.Context().Push(self.c.Contexts().Branches, types.OnFocusOpts{}) + return nil + } } } - self.c.Refresh(refreshOptions) + self.c.RefreshFromWorker(refreshOptions) return err }) } diff --git a/pkg/gui/controllers/submodules_controller.go b/pkg/gui/controllers/submodules_controller.go index 97b7ff3dd..a2dd22ed3 100644 --- a/pkg/gui/controllers/submodules_controller.go +++ b/pkg/gui/controllers/submodules_controller.go @@ -164,7 +164,7 @@ func (self *SubmodulesController) add() error { return err } - self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.SUBMODULES}}) + self.c.RefreshFromWorker(types.RefreshOptions{Scope: []types.RefreshableView{types.SUBMODULES}}) return nil }) }, @@ -193,7 +193,7 @@ func (self *SubmodulesController) editURL(submodule *models.SubmoduleConfig) err return err } - self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.SUBMODULES}}) + self.c.RefreshFromWorker(types.RefreshOptions{Scope: []types.RefreshableView{types.SUBMODULES}}) return nil }) }, @@ -210,7 +210,7 @@ func (self *SubmodulesController) init(submodule *models.SubmoduleConfig) error return err } - self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.SUBMODULES}}) + self.c.RefreshFromWorker(types.RefreshOptions{Scope: []types.RefreshableView{types.SUBMODULES}}) return nil }) } @@ -229,7 +229,7 @@ func (self *SubmodulesController) openBulkActionsMenu() error { return err } - self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.SUBMODULES}}) + self.c.RefreshFromWorker(types.RefreshOptions{Scope: []types.RefreshableView{types.SUBMODULES}}) return nil }) }, @@ -244,7 +244,7 @@ func (self *SubmodulesController) openBulkActionsMenu() error { return err } - self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.SUBMODULES}}) + self.c.RefreshFromWorker(types.RefreshOptions{Scope: []types.RefreshableView{types.SUBMODULES}}) return nil }) }, @@ -259,7 +259,7 @@ func (self *SubmodulesController) openBulkActionsMenu() error { return err } - self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.SUBMODULES}}) + self.c.RefreshFromWorker(types.RefreshOptions{Scope: []types.RefreshableView{types.SUBMODULES}}) return nil }) }, @@ -274,7 +274,7 @@ func (self *SubmodulesController) openBulkActionsMenu() error { return err } - self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.SUBMODULES}}) + self.c.RefreshFromWorker(types.RefreshOptions{Scope: []types.RefreshableView{types.SUBMODULES}}) return nil }) }, @@ -292,7 +292,7 @@ func (self *SubmodulesController) update(submodule *models.SubmoduleConfig) erro return err } - self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.SUBMODULES}}) + self.c.RefreshFromWorker(types.RefreshOptions{Scope: []types.RefreshableView{types.SUBMODULES}}) return nil }) } diff --git a/pkg/gui/controllers/switch_to_diff_files_controller.go b/pkg/gui/controllers/switch_to_diff_files_controller.go index c2ff4d674..afdf92c80 100644 --- a/pkg/gui/controllers/switch_to_diff_files_controller.go +++ b/pkg/gui/controllers/switch_to_diff_files_controller.go @@ -90,18 +90,19 @@ func (self *SwitchToDiffFilesController) enter() error { self.c.Refresh(types.RefreshOptions{ Scope: []types.RefreshableView{types.COMMIT_FILES}, + Then: func() error { + if filterPath := self.c.Modes().Filtering.GetPath(); filterPath != "" { + path, err := filepath.Rel(self.c.Git().RepoPaths.RepoPath(), filterPath) + if err != nil { + path = filterPath + } + commitFilesContext.CommitFileTreeViewModel.SelectPath( + filepath.ToSlash(path), self.c.UserConfig().Gui.ShowRootItemInFileTree) + } + self.c.Context().Push(commitFilesContext, types.OnFocusOpts{}) + return nil + }, }) - - if filterPath := self.c.Modes().Filtering.GetPath(); filterPath != "" { - path, err := filepath.Rel(self.c.Git().RepoPaths.RepoPath(), filterPath) - if err != nil { - path = filterPath - } - commitFilesContext.CommitFileTreeViewModel.SelectPath( - filepath.ToSlash(path), self.c.UserConfig().Gui.ShowRootItemInFileTree) - } - - self.c.Context().Push(commitFilesContext, types.OnFocusOpts{}) return nil } diff --git a/pkg/gui/controllers/sync_controller.go b/pkg/gui/controllers/sync_controller.go index 0f754eb49..fafd4e7dd 100644 --- a/pkg/gui/controllers/sync_controller.go +++ b/pkg/gui/controllers/sync_controller.go @@ -229,7 +229,7 @@ func (self *SyncController) pushAux(currentBranch *models.Branch, opts pushOpts) } return err } - self.c.Refresh(types.RefreshOptions{Mode: types.SYNC}) + self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.SYNC}) return nil }) } diff --git a/pkg/gui/controllers/tags_controller.go b/pkg/gui/controllers/tags_controller.go index fe2c4e80f..2a59af5ec 100644 --- a/pkg/gui/controllers/tags_controller.go +++ b/pkg/gui/controllers/tags_controller.go @@ -168,7 +168,7 @@ func (self *TagsController) localDelete(tag *models.Tag) error { return self.c.WithWaitingStatus(self.c.Tr.DeletingStatus, func(gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.DeleteLocalTag) err := self.c.Git().Tag.LocalDelete(tag.Name) - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.COMMITS, types.TAGS}}) + self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.COMMITS, types.TAGS}}) return err }) } @@ -210,7 +210,7 @@ func (self *TagsController) remoteDelete(tag *models.Tag) error { return err } self.c.Toast(self.c.Tr.RemoteTagDeletedMessage) - self.c.Refresh(types.RefreshOptions{Mode: types.SYNC, Scope: []types.RefreshableView{types.COMMITS, types.TAGS}}) + self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.SYNC, Scope: []types.RefreshableView{types.COMMITS, types.TAGS}}) return nil }) }, @@ -264,7 +264,7 @@ func (self *TagsController) localAndRemoteDelete(tag *models.Tag) error { if err := self.c.Git().Tag.LocalDelete(tag.Name); err != nil { return err } - self.c.Refresh(types.RefreshOptions{Mode: types.SYNC, Scope: []types.RefreshableView{types.COMMITS, types.TAGS}}) + self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.SYNC, Scope: []types.RefreshableView{types.COMMITS, types.TAGS}}) return nil }) }, diff --git a/pkg/gui/controllers/undo_controller.go b/pkg/gui/controllers/undo_controller.go index 775e871a4..0954d66b0 100644 --- a/pkg/gui/controllers/undo_controller.go +++ b/pkg/gui/controllers/undo_controller.go @@ -271,7 +271,7 @@ func (self *UndoController) hardResetWithAutoStash(commitHash string, options ha if err != nil { return err } - self.c.Refresh(types.RefreshOptions{}) + self.c.RefreshFromWorker(types.RefreshOptions{}) return nil }) } diff --git a/pkg/gui/filetree/commit_file_tree_view_model.go b/pkg/gui/filetree/commit_file_tree_view_model.go index c2e7e74e4..a58f7d93e 100644 --- a/pkg/gui/filetree/commit_file_tree_view_model.go +++ b/pkg/gui/filetree/commit_file_tree_view_model.go @@ -2,7 +2,6 @@ package filetree import ( "strings" - "sync" "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/common" @@ -26,7 +25,6 @@ type ICommitFileTreeViewModel interface { } type CommitFileTreeViewModel struct { - sync.RWMutex types.IListCursor ICommitFileTree @@ -144,6 +142,22 @@ func (self *CommitFileTreeViewModel) GetSelectedPath() string { return node.GetPath() } +// SetTree rebuilds the tree and clamps the selection so it stays in range. The +// embedded tree's SetTree only rebuilds the node list and doesn't touch the +// cursor, so after a shrinking rebuild (e.g. moving a patch out into the index) +// the selection index could be left past the end of the tree; GetSelectedItems +// would then return a nil node and crash callers such as canEditFiles when the +// options map is rendered during layout. +// +// Unlike FileTreeViewModel.SetTree we don't re-find the selected node by path +// afterwards: that walk lands on the containing directory when a file is removed +// from a dir that then collapses, whereas keeping the (clamped) index lands on +// the sibling file, which is what we want here. +func (self *CommitFileTreeViewModel) SetTree() { + self.ICommitFileTree.SetTree() + self.ClampSelection() +} + // duplicated from file_tree_view_model.go. Generics will help here func (self *CommitFileTreeViewModel) ToggleShowTree() { selectedNode := self.GetSelected() diff --git a/pkg/gui/filetree/commit_file_tree_view_model_test.go b/pkg/gui/filetree/commit_file_tree_view_model_test.go new file mode 100644 index 000000000..c8862f6f9 --- /dev/null +++ b/pkg/gui/filetree/commit_file_tree_view_model_test.go @@ -0,0 +1,40 @@ +package filetree + +import ( + "testing" + + "github.com/jesseduffield/lazygit/pkg/commands/models" + "github.com/jesseduffield/lazygit/pkg/common" + "github.com/stretchr/testify/assert" +) + +// When the tree shrinks under the selection - e.g. moving a patch out into the +// index removes a file - SetTree must keep the selection in range. Otherwise +// GetSelectedItems returns a nil node, which crashes callers such as +// canEditFiles when the options map is rendered during layout. +func TestCommitFileTreeViewModelSetTreeClampsSelectionOnShrink(t *testing.T) { + files := []*models.CommitFile{ + {Path: "file1"}, + {Path: "file2"}, + {Path: "file3"}, + } + viewModel := NewCommitFileTreeViewModel( + func() []*models.CommitFile { return files }, + common.NewDummyCommon(), + false, // flat list + ) + viewModel.SetTree() + viewModel.SetSelectedLineIdx(viewModel.Len() - 1) + + // The file under the cursor goes away and the tree shrinks. + files = []*models.CommitFile{{Path: "file1"}} + viewModel.SetTree() + + assert.Less(t, viewModel.GetSelectedLineIdx(), viewModel.Len()) + assert.NotNil(t, viewModel.GetSelected()) + items, _, _ := viewModel.GetSelectedItems() + assert.NotEmpty(t, items) + for _, item := range items { + assert.NotNil(t, item) + } +} diff --git a/pkg/gui/filetree/file_tree_view_model.go b/pkg/gui/filetree/file_tree_view_model.go index 741550c19..aabbbce7f 100644 --- a/pkg/gui/filetree/file_tree_view_model.go +++ b/pkg/gui/filetree/file_tree_view_model.go @@ -2,7 +2,6 @@ package filetree import ( "strings" - "sync" "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/common" @@ -22,7 +21,6 @@ type IFileTreeViewModel interface { // which item is selected. It also contains logic for repositioning that cursor // after the files are refreshed type FileTreeViewModel struct { - sync.RWMutex types.IListCursor IFileTree searchHistory *utils.HistoryBuffer[string] diff --git a/pkg/gui/gui.go b/pkg/gui/gui.go index e23afd124..d77673e9c 100644 --- a/pkg/gui/gui.go +++ b/pkg/gui/gui.go @@ -12,6 +12,7 @@ import ( "sort" "strings" "sync" + "sync/atomic" "time" "github.com/jesseduffield/lazycore/pkg/boxlayout" @@ -111,7 +112,10 @@ type Gui struct { PopupHandler types.IPopupHandler - IsRefreshingFiles bool + // Bumped every time we switch to a different repository (in resetState). + // Used to drop refresh results that were computed for a repo we've since + // navigated away from. See RefreshHelper.onUIThreadUnlessRepoChanged. + repoGeneration atomic.Int32 // we use this to decide whether we'll return to the original directory that // lazygit was opened in, or if we'll retain the one we're currently in. @@ -171,18 +175,14 @@ func (self *StateAccessor) GetRepoState() types.IRepoStateAccessor { return self.gui.State } +func (self *StateAccessor) GetRepoGeneration() int { + return int(self.gui.repoGeneration.Load()) +} + func (self *StateAccessor) GetPagerConfig() *config.PagerConfig { return self.gui.pagerConfig } -func (self *StateAccessor) GetIsRefreshingFiles() bool { - return self.gui.IsRefreshingFiles -} - -func (self *StateAccessor) SetIsRefreshingFiles(value bool) { - self.gui.IsRefreshingFiles = value -} - func (self *StateAccessor) GetShowExtrasWindow() bool { return self.gui.ShowExtrasWindow } @@ -234,8 +234,11 @@ type GuiRepoState struct { SplitMainPanel bool - SearchState *types.SearchState - StartupStage types.StartupStage // Allows us to not load everything at once + SearchState *types.SearchState + // Lets us not load everything at once. Written and read from refresh + // workers (the reflog/branches load transitions it INITIAL->COMPLETE), so + // it's atomic. Holds a types.StartupStage. + startupStage atomic.Int32 ContextMgr *ContextMgr Contexts *context.ContextTree @@ -262,7 +265,11 @@ type GuiRepoState struct { // continue such an operation once its conflicts are resolved if we started // it ourselves; for an externally started one, popping up unbidden would be // confusing. Reset whenever we observe that no operation is in progress. - mergeOrRebaseStartedInLazygit bool + // + // Written from both the files refresh worker and the merge/rebase result + // path (which runs on a worker for the async callers), and read from the + // files refresh worker, so it's atomic. + mergeOrRebaseStartedInLazygit atomic.Bool } var _ types.IRepoStateAccessor = new(GuiRepoState) @@ -276,11 +283,11 @@ func (self *GuiRepoState) GetWindowViewNameMap() *utils.ThreadSafeMap[string, st } func (self *GuiRepoState) GetStartupStage() types.StartupStage { - return self.StartupStage + return types.StartupStage(self.startupStage.Load()) } func (self *GuiRepoState) SetStartupStage(value types.StartupStage) { - self.StartupStage = value + self.startupStage.Store(int32(value)) } func (self *GuiRepoState) GetCurrentPopupOpts() *types.CreatePopupPanelOpts { @@ -292,11 +299,11 @@ func (self *GuiRepoState) SetCurrentPopupOpts(value *types.CreatePopupPanelOpts) } func (self *GuiRepoState) GetMergeOrRebaseStartedInLazygit() bool { - return self.mergeOrRebaseStartedInLazygit + return self.mergeOrRebaseStartedInLazygit.Load() } func (self *GuiRepoState) SetMergeOrRebaseStartedInLazygit(value bool) { - self.mergeOrRebaseStartedInLazygit = value + self.mergeOrRebaseStartedInLazygit.Store(value) } func (self *GuiRepoState) GetScreenMode() types.ScreenMode { @@ -585,6 +592,11 @@ func (gui *Gui) checkForChangedConfigsThatDontAutoReload(oldConfig *config.UserC // resetState reuses the repo state from our repo state map, if the repo was // open before; otherwise it creates a new one. func (gui *Gui) resetState(startArgs appTypes.StartArgs) types.Context { + // Bump the repo generation so that any refresh still in flight for the + // previous repo drops its model update instead of applying it here (see + // RefreshHelper.onUIThreadUnlessRepoChanged). + gui.repoGeneration.Add(1) + // Un-highlight the current view if there is one. The reason we do this is // that the repo we are switching to might have a different view focused, // and would then show an inactive highlight for the previous view. @@ -1189,16 +1201,32 @@ func (gui *Gui) onUIThread(f func() error) { }) } +func (gui *Gui) onUIThreadBackground(f func() error) { + gui.g.UpdateBackground(func(*gocui.Gui) error { + return f() + }) +} + func (gui *Gui) onUIThreadContentOnly(f func() error) { gui.g.UpdateContentOnly(func(*gocui.Gui) error { return f() }) } +func (gui *Gui) onUIThreadContentOnlyBackground(f func() error) { + gui.g.UpdateContentOnlyBackground(func(*gocui.Gui) error { + return f() + }) +} + func (gui *Gui) onWorker(f func(gocui.Task) error) { gui.g.OnWorker(f) } +func (gui *Gui) onWorkerBackground(f func(gocui.Task) error) { + gui.g.OnWorkerBackground(f) +} + func (gui *Gui) getWindowDimensions(informationStr string, appStatus string) map[string]boxlayout.Dimensions { return gui.helpers.WindowArrangement.GetWindowDimensions(informationStr, appStatus) } diff --git a/pkg/gui/gui_common.go b/pkg/gui/gui_common.go index c74a99a05..c8de7545d 100644 --- a/pkg/gui/gui_common.go +++ b/pkg/gui/gui_common.go @@ -30,6 +30,10 @@ func (self *guiCommon) Refresh(opts types.RefreshOptions) { self.gui.helpers.Refresh.Refresh(opts) } +func (self *guiCommon) RefreshFromWorker(opts types.RefreshOptions) { + self.gui.helpers.Refresh.RefreshFromWorker(opts) +} + func (self *guiCommon) PostRefreshUpdate(context types.Context) { self.gui.postRefreshUpdate(context) } @@ -124,14 +128,26 @@ func (self *guiCommon) OnUIThread(f func() error) { self.gui.onUIThread(f) } +func (self *guiCommon) OnUIThreadBackground(f func() error) { + self.gui.onUIThreadBackground(f) +} + func (self *guiCommon) OnUIThreadContentOnly(f func() error) { self.gui.onUIThreadContentOnly(f) } +func (self *guiCommon) OnUIThreadContentOnlyBackground(f func() error) { + self.gui.onUIThreadContentOnlyBackground(f) +} + func (self *guiCommon) OnWorker(f func(gocui.Task) error) { self.gui.onWorker(f) } +func (self *guiCommon) OnWorkerBackground(f func(gocui.Task) error) { + self.gui.onWorkerBackground(f) +} + func (self *guiCommon) RenderToMainViews(opts types.RefreshMainOpts) { self.gui.refreshMainViews(opts) } diff --git a/pkg/gui/services/custom_commands/handler_creator.go b/pkg/gui/services/custom_commands/handler_creator.go index 1e321c2de..4eb762019 100644 --- a/pkg/gui/services/custom_commands/handler_creator.go +++ b/pkg/gui/services/custom_commands/handler_creator.go @@ -314,7 +314,7 @@ func (self *HandlerCreator) finalHandler(customCommand config.CustomCommand, ses } output, err := cmdObj.RunWithOutput() - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC}) if err != nil { if customCommand.After != nil && customCommand.After.CheckForConflicts { diff --git a/pkg/gui/tasks_adapter.go b/pkg/gui/tasks_adapter.go index 09edd2d36..dd7999107 100644 --- a/pkg/gui/tasks_adapter.go +++ b/pkg/gui/tasks_adapter.go @@ -136,7 +136,14 @@ func (gui *Gui) getManager(view *gocui.View) *tasks.ViewBufferManager { view.SetOrigin(0, 0) }, func() gocui.Task { - return gui.c.GocuiGui().NewTask() + // A background task: rendering content into a view is display + // work, not lazygit driving a git operation, so it must not + // count towards being busy and block a repo switch. These + // renders fire on nearly every focus/selection change, including + // the context activation that happens right before a menu/prompt + // handler runs (e.g. confirming worktree creation), which would + // otherwise make the switch that handler triggers refuse itself. + return gui.c.GocuiGui().NewBackgroundTask() }, ) gui.viewBufferManagerMap[view.Name()] = manager diff --git a/pkg/gui/types/common.go b/pkg/gui/types/common.go index 7621f8686..4ced8bd79 100644 --- a/pkg/gui/types/common.go +++ b/pkg/gui/types/common.go @@ -30,6 +30,12 @@ type IGuiCommon interface { LogCommand(cmdStr string, isCommandLine bool) // we call this when we want to refetch some models and render the result. Internally calls PostRefreshUpdate Refresh(RefreshOptions) + // Like Refresh, but for callers running on a worker goroutine (e.g. inside + // a WithWaitingStatus handler) rather than the UI thread. The refresh + // captures the model/context state it needs on the UI thread before doing + // its git work; knowing which thread the caller is on lets it capture + // inline (UI thread) or hop across (worker) without racing or deadlocking. + RefreshFromWorker(RefreshOptions) // we call this when we've changed something in the view model but not the actual model, // e.g. expanding or collapsing a folder in a file view. Calling 'Refresh' in this // case would be overkill, although refresh will internally call 'PostRefreshUpdate' @@ -75,13 +81,22 @@ 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 for work triggered by a background routine, so it + // doesn't count towards lazygit being busy (see the *Background methods on + // gocui.Gui and repo-switch safety). + OnUIThreadBackground(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) + // Like OnUIThreadContentOnly, but for background work (see OnUIThreadBackground). + OnUIThreadContentOnlyBackground(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) + // Like OnWorker, but for a background routine (or work it triggers), so it + // doesn't count towards lazygit being busy (see OnUIThreadBackground). + OnWorkerBackground(f func(gocui.Task) error) // Function to call at the end of our 'layout' function which renders views // For example, you may want a view's line to be focused only after that view is // resized, if in accordion mode. @@ -338,16 +353,9 @@ type Model struct { } type Mutexes struct { - RefreshingFilesMutex deadlock.Mutex - RefreshingBranchesMutex deadlock.Mutex - RefreshingStatusMutex deadlock.Mutex - RefreshingPullRequestsMutex deadlock.Mutex - LocalCommitsMutex deadlock.Mutex - SubCommitsMutex deadlock.Mutex - AuthorsMutex deadlock.Mutex - SubprocessMutex deadlock.Mutex - PopupMutex deadlock.Mutex - PtyMutex deadlock.Mutex + SubprocessMutex deadlock.Mutex + PopupMutex deadlock.Mutex + PtyMutex deadlock.Mutex } // A long-running operation associated with an item. For example, we'll show @@ -377,8 +385,6 @@ type IStateAccessor interface { // tells us whether we're currently updating lazygit GetUpdating() bool SetUpdating(bool) - SetIsRefreshingFiles(bool) - GetIsRefreshingFiles() bool GetShowExtrasWindow() bool SetShowExtrasWindow(bool) GetRetainOriginalDir() bool @@ -386,6 +392,13 @@ type IStateAccessor interface { GetItemOperation(item HasUrn) ItemOperation SetItemOperation(item HasUrn, operation ItemOperation) ClearItemOperation(item HasUrn) + + // A counter that is bumped every time we switch to a different repository + // (see Gui.resetState). A refresh captures it when it starts and carries it + // through to onUIThreadUnlessRepoChanged, so that a model update computed for + // one repo can be dropped rather than applied to another if the user switched + // repos while the refresh was in flight. + GetRepoGeneration() int } type IRepoStateAccessor interface { diff --git a/pkg/gui/types/refresh.go b/pkg/gui/types/refresh.go index 8d9704d55..f4041bb2e 100644 --- a/pkg/gui/types/refresh.go +++ b/pkg/gui/types/refresh.go @@ -55,22 +55,41 @@ const ( SelectHeadCommit ) +// BranchSelectionBehavior controls which local branch is selected after the +// branches list is reloaded by a refresh. +type BranchSelectionBehavior int + +const ( + // Keep the same branch selected by name, restoring it at its new position if + // the order changed. This is the right default whenever the list reloads + // underneath a selection the user hasn't deliberately changed. + KeepBranchSelectionByName BranchSelectionBehavior = iota + + // Select the checked-out branch (the one at the top of the list). Used after + // operations that check something out - checkout, creating a branch, moving + // commits to a new branch - so the newly checked-out ref ends up selected. + SelectCheckedOutBranch +) + type RefreshOptions struct { - Then func() + Then func() error Scope []RefreshableView // e.g. []RefreshableView{COMMITS, BRANCHES}. Leave empty to refresh everything Mode RefreshMode // one of SYNC (default), ASYNC, and BLOCK_UI - // Normally a refresh of the branches tries to keep the same branch selected - // (by name); this is usually important in case the order of branches - // changes. Passing true for KeepBranchSelectionIndex suppresses this and - // keeps the selection index the same. Useful after checking out a detached - // head, and selecting index 0. - KeepBranchSelectionIndex bool + // Controls which local branch is selected after the refresh. Defaults to + // KeepBranchSelectionByName. + BranchSelection BranchSelectionBehavior // Controls which local commit is selected after the refresh. Defaults to // KeepCommitSelectionByHash. CommitSelection CommitSelectionBehavior + // When true, select the top (most recent) reflog entry after the refresh. + // Used alongside SelectCheckedOutBranch by operations that check something + // out, since the checkout adds a new reflog entry at the top. Defaults to + // keeping the reflog selection where it is. + SelectTopReflogCommit bool + // When true, this refresh was initiated by a background routine rather than // by a user action. Every git command suppresses optional locks by default // so it can't contend for index.lock (see git_commands.OptionalLocksEnvVar); diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index 0ee0a9e86..69ea7012f 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -769,6 +769,7 @@ type TranslationSet struct { ErrStageDirWithInlineMergeConflicts string ErrRepositoryMovedOrDeleted string ErrWorktreeMovedOrRemoved string + CantSwitchWhileOperationInProgress string CommandLog string ToggleShowCommandLog string FocusCommandLog string @@ -1921,6 +1922,7 @@ func EnglishTranslationSet() *TranslationSet { ErrRepositoryMovedOrDeleted: "Cannot find repo. It might have been moved or deleted ¯\\_(ツ)_/¯", CommandLog: "Command log", ErrWorktreeMovedOrRemoved: "Cannot find worktree. It might have been moved or removed ¯\\_(ツ)_/¯", + CantSwitchWhileOperationInProgress: "Can't switch repositories while an operation is in progress", ToggleShowCommandLog: "Toggle show/hide command log", FocusCommandLog: "Focus command log", CommandLogHeader: "You can hide/focus this panel by pressing '%s'\n",