diff --git a/AGENTS.md b/AGENTS.md index 947add510..88e818249 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -411,3 +411,12 @@ Never run `find` (or similar) from `/` or other paths outside the project. All third-party code we use is vendored under `vendor/`, so dependency sources are reachable from inside the working tree — search there instead of the host filesystem. + +## gocui is in-tree, not a dependency + +The `gocui` TUI library is a fork maintained directly in this repo under +`pkg/gocui` — it's an ordinary package, not a Go module dependency. Don't look +for it in `go.mod`/`go.sum` or the module cache (`$GOMODCACHE`); it isn't +there. When you need to read or change gocui internals (the task manager, the +event loop, worker/UI-thread dispatch, view rendering), edit `pkg/gocui` +directly. diff --git a/pkg/gocui/block_events_test.go b/pkg/gocui/block_events_test.go new file mode 100644 index 000000000..277bac89a --- /dev/null +++ b/pkg/gocui/block_events_test.go @@ -0,0 +1,98 @@ +package gocui + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestEventWithheldWhileBlocking(t *testing.T) { + scenarios := []struct { + name string + event GocuiEvent + withheld bool + }{ + {"key", GocuiEvent{Type: eventKey, Key: NewKeyRune('x')}, true}, + {"mouse click", GocuiEvent{Type: eventMouse, Key: NewKeyName(MouseLeft)}, true}, + {"mouse scroll", GocuiEvent{Type: eventMouse, Key: NewKeyName(MouseWheelDown)}, false}, + {"mouse move", GocuiEvent{Type: eventMouseMove}, true}, + {"resize", GocuiEvent{Type: eventResize}, false}, + {"focus", GocuiEvent{Type: eventFocus}, false}, + {"paste", GocuiEvent{Type: eventPaste}, false}, + {"error", GocuiEvent{Type: eventError}, false}, + } + + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + assert.Equal(t, s.withheld, eventWithheldWhileBlocking(&s.event)) + }) + } +} + +// setupKeyRecorder wires a keybinding on a focused view that records each time +// it fires, and returns the key event that triggers it plus the record slice. +func setupKeyRecorder(t *testing.T, g *Gui) (GocuiEvent, *[]int) { + t.Helper() + + _, _ = g.SetView("main", 0, 0, 80, 22, 0) + _, err := g.SetCurrentView("main") + assert.NoError(t, err) + + fired := []int{} + callCount := 0 + key := NewKeyRune('x') + g.SetKeybinding("main", key, func(*Gui, *View) error { + callCount++ + fired = append(fired, callCount) + return nil + }) + + return GocuiEvent{Type: eventKey, Key: key}, &fired +} + +func TestBlockingEvents_KeysBufferedAndReplayed(t *testing.T) { + g := newTestGui(t) + keyEvent, fired := setupKeyRecorder(t, g) + + // Not blocking: the key dispatches immediately. + assert.NoError(t, g.handleEvent(&keyEvent)) + assert.Len(t, *fired, 1) + + // While blocking: the key is buffered, not dispatched. + g.BeginBlockingEvents() + assert.NoError(t, g.handleEvent(&keyEvent)) + assert.NoError(t, g.handleEvent(&keyEvent)) + assert.Len(t, *fired, 1, "buffered keys must not dispatch while blocking") + + // Unblocking replays the buffered keys. + assert.NoError(t, g.EndBlockingEvents()) + assert.Len(t, *fired, 3, "both buffered keys should replay on unblock") + assert.Empty(t, g.bufferedKeyEvents) +} + +func TestBlockingEvents_NestsWithCounter(t *testing.T) { + g := newTestGui(t) + keyEvent, fired := setupKeyRecorder(t, g) + + g.BeginBlockingEvents() + g.BeginBlockingEvents() + assert.NoError(t, g.handleEvent(&keyEvent)) + + // The inner block ending still leaves us blocked: no replay yet. + assert.NoError(t, g.EndBlockingEvents()) + assert.Empty(t, *fired) + + // Only the outermost block ending replays. + assert.NoError(t, g.EndBlockingEvents()) + assert.Len(t, *fired, 1) +} + +func TestBlockingEvents_MouseClicksDroppedNotBuffered(t *testing.T) { + g := newTestGui(t) + + g.BeginBlockingEvents() + click := GocuiEvent{Type: eventMouse, Key: NewKeyName(MouseLeft)} + assert.NoError(t, g.handleEvent(&click)) + assert.Empty(t, g.bufferedKeyEvents, "mouse clicks must be dropped, not buffered") + assert.NoError(t, g.EndBlockingEvents()) +} diff --git a/pkg/gocui/gui.go b/pkg/gocui/gui.go index 9da5225c0..700c2b54c 100644 --- a/pkg/gocui/gui.go +++ b/pkg/gocui/gui.go @@ -210,6 +210,14 @@ type Gui struct { // MainLoop starts. IsUIThread compares against it. Written once, read from // worker goroutines, so it's atomic. uiThreadID atomic.Int64 + + // blockInputCount, when greater than zero, withholds keyboard input from + // the handlers: key events are buffered into bufferedKeyEvents and replayed + // once the count drops back to zero, while mouse clicks and hover are + // dropped outright. It's a counter so blocking can nest. Both fields are + // only touched on the UI thread. See BeginBlockingEvents. + blockInputCount int + bufferedKeyEvents []GocuiEvent } type NewGuiOpts struct { @@ -806,6 +814,42 @@ func (g *Gui) IsUIThread() bool { return goid.Get() == g.uiThreadID.Load() } +// BeginBlockingEvents starts withholding keyboard input from the handlers, so a +// long-running operation can't be disrupted by keys the user presses while it +// runs. Keys are buffered and replayed once EndBlockingEvents balances this +// call; mouse clicks and hover are dropped for the duration. Scrolling, +// resizing, focus changes and all rendering keep working throughout. It's a +// counter, so blocking nests; every call must be paired with EndBlockingEvents. +// +// Must be called on the UI thread. Callers arrange this by beginning the block +// synchronously from the keybinding handler, before dispatching the operation +// to a worker — beginning it from the worker would race the next queued +// keypress, which is exactly the input we mean to withhold. +func (g *Gui) BeginBlockingEvents() { + g.blockInputCount++ +} + +// EndBlockingEvents balances a BeginBlockingEvents call. When the last nested +// block ends, the keys buffered while blocked are replayed in order through the +// normal dispatch path, so they act on the now-current context (a key whose +// binding no longer exists is simply ignored, just as if it had been pressed +// now). Must be called on the UI thread. +func (g *Gui) EndBlockingEvents() error { + g.blockInputCount-- + if g.blockInputCount > 0 { + return nil + } + + buffered := g.bufferedKeyEvents + g.bufferedKeyEvents = nil + for i := range buffered { + if err := g.handleEvent(&buffered[i]); err != nil { + return err + } + } + return nil +} + // 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. @@ -1052,6 +1096,17 @@ func (g *Gui) processRemainingEvents() (bool, error) { // handleEvent handles an event, based on its type (key-press, error, // etc.) func (g *Gui) handleEvent(ev *GocuiEvent) error { + if g.blockInputCount > 0 && eventWithheldWhileBlocking(ev) { + if ev.Type == eventKey { + // Buffer keys so they replay against fresh state on unblock. + g.bufferedKeyEvents = append(g.bufferedKeyEvents, *ev) + } + // Mouse clicks and hover fall through to here without being buffered: + // replaying them once the operation has changed the layout underneath + // them would target the wrong thing, so we drop them outright. + return nil + } + switch ev.Type { case eventKey, eventMouse, eventMouseMove: return g.onKey(ev) @@ -1070,6 +1125,24 @@ func (g *Gui) handleEvent(ev *GocuiEvent) error { } } +// eventWithheldWhileBlocking reports whether an event must not reach the +// handlers while input is blocked (see BeginBlockingEvents). Key events are +// withheld (buffered for replay); mouse clicks and hover are withheld (dropped). +// Everything else — mouse scrolling, resize, focus, paste, errors — flows +// through as usual. +func eventWithheldWhileBlocking(ev *GocuiEvent) bool { + switch ev.Type { + case eventKey: + return true + case eventMouse: + return !IsMouseScrollKey(ev.Key.KeyName()) + case eventMouseMove: + return true + default: + return false + } +} + func (g *Gui) onResize() { // not sure if we actually need this // g.screen.Sync() diff --git a/pkg/gui/controllers/bisect_controller.go b/pkg/gui/controllers/bisect_controller.go index eb568240b..685f932b1 100644 --- a/pkg/gui/controllers/bisect_controller.go +++ b/pkg/gui/controllers/bisect_controller.go @@ -282,7 +282,7 @@ func (self *BisectController) afterBisectMarkRefresh(selectCurrent bool, waitToR } if waitToReselect { - self.c.Refresh(types.RefreshOptions{Mode: types.SYNC, Scope: []types.RefreshableView{}, Then: selectFn}) + self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{}, Then: selectFn}) return nil } diff --git a/pkg/gui/controllers/branches_controller.go b/pkg/gui/controllers/branches_controller.go index a886a410b..a73ee3bc2 100644 --- a/pkg/gui/controllers/branches_controller.go +++ b/pkg/gui/controllers/branches_controller.go @@ -331,7 +331,6 @@ func (self *BranchesController) viewUpstreamOptions(selectedBranch *models.Branc return err } self.c.Refresh(types.RefreshOptions{ - Mode: types.SYNC, Scope: []types.RefreshableView{ types.BRANCHES, types.COMMITS, @@ -355,7 +354,6 @@ func (self *BranchesController) viewUpstreamOptions(selectedBranch *models.Branc return err } self.c.Refresh(types.RefreshOptions{ - Mode: types.SYNC, Scope: []types.RefreshableView{ types.BRANCHES, types.COMMITS, @@ -546,7 +544,7 @@ func (self *BranchesController) forceCheckout() error { if err := self.c.Git().Branch.Checkout(branch.Name, git_commands.CheckoutOptions{Force: true}); err != nil { return err } - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + self.c.Refresh(types.RefreshOptions{}) return nil }, }) @@ -600,7 +598,6 @@ func (self *BranchesController) createNewBranchWithName(newBranchName string) er } self.c.Refresh(types.RefreshOptions{ - Mode: types.ASYNC, BranchSelection: types.SelectCheckedOutBranch, CommitSelection: types.SelectHeadCommit, SelectTopReflogCommit: true, @@ -734,7 +731,7 @@ func (self *BranchesController) fastForward(branch *models.Branch) error { WorktreePath: worktreePath, }, ) - self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.SYNC}) + self.c.RefreshFromWorker(types.RefreshOptions{}) return err } @@ -743,7 +740,7 @@ func (self *BranchesController) fastForward(branch *models.Branch) error { err := self.c.Git().Sync.FastForward( task, branch.Name, branch.UpstreamRemote, branch.UpstreamBranch, ) - self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.SYNC, Scope: []types.RefreshableView{types.BRANCHES}}) + self.c.RefreshFromWorker(types.RefreshOptions{Scope: []types.RefreshableView{types.BRANCHES}}) return err }) } @@ -760,7 +757,7 @@ func (self *BranchesController) createSortMenu() error { if self.c.UserConfig().Git.LocalBranchSortOrder != sortOrder { self.c.UserConfig().Git.LocalBranchSortOrder = sortOrder self.c.Contexts().Branches.SetSelection(0) - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.BRANCHES}}) + self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.BRANCHES}}) return nil } return nil @@ -788,7 +785,6 @@ func (self *BranchesController) rename(branch *models.Branch) error { // 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. diff --git a/pkg/gui/controllers/commits_files_controller.go b/pkg/gui/controllers/commits_files_controller.go index b90e14b74..d129b3f90 100644 --- a/pkg/gui/controllers/commits_files_controller.go +++ b/pkg/gui/controllers/commits_files_controller.go @@ -324,7 +324,7 @@ func (self *CommitFilesController) checkout(node *filetree.CommitFileNode) error return err } - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + self.c.Refresh(types.RefreshOptions{}) return nil } @@ -339,7 +339,7 @@ func (self *CommitFilesController) discard(selectedNodes []*filetree.CommitFileN 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 { + return self.c.WithWaitingStatusBlockingInput(self.c.Tr.RebasingStatus, func(gocui.Task) error { var filePaths []string selectedNodes = normalisedSelectedCommitFileNodes(selectedNodes) diff --git a/pkg/gui/controllers/custom_patch_options_menu_action.go b/pkg/gui/controllers/custom_patch_options_menu_action.go index 3d15ce899..2882ab808 100644 --- a/pkg/gui/controllers/custom_patch_options_menu_action.go +++ b/pkg/gui/controllers/custom_patch_options_menu_action.go @@ -269,7 +269,7 @@ func (self *CustomPatchOptionsMenuAction) handleApplyPatch(reverse bool) error { return err } - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + self.c.Refresh(types.RefreshOptions{}) return nil }, }) diff --git a/pkg/gui/controllers/diffing_menu_action.go b/pkg/gui/controllers/diffing_menu_action.go index 3ae5903d9..8372d7919 100644 --- a/pkg/gui/controllers/diffing_menu_action.go +++ b/pkg/gui/controllers/diffing_menu_action.go @@ -22,7 +22,7 @@ func (self *DiffingMenuAction) Call() error { OnPress: func() error { self.c.Modes().Diffing.Ref = name // can scope this down based on current view but too lazy right now - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + self.c.Refresh(types.RefreshOptions{}) return nil }, }, @@ -38,7 +38,7 @@ func (self *DiffingMenuAction) Call() error { FindSuggestionsFunc: self.c.Helpers().Suggestions.GetRefsSuggestionsFunc(), HandleConfirm: func(response string) error { self.c.Modes().Diffing.Ref = response - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + self.c.Refresh(types.RefreshOptions{}) return nil }, }) @@ -54,7 +54,7 @@ func (self *DiffingMenuAction) Call() error { Label: self.c.Tr.SwapDiff, OnPress: func() error { self.c.Modes().Diffing.Reverse = !self.c.Modes().Diffing.Reverse - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + self.c.Refresh(types.RefreshOptions{}) return nil }, }, @@ -62,7 +62,7 @@ func (self *DiffingMenuAction) Call() error { Label: self.c.Tr.ExitDiffMode, OnPress: func() error { self.c.Modes().Diffing = diffing.New() - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + self.c.Refresh(types.RefreshOptions{}) return nil }, }, diff --git a/pkg/gui/controllers/files_controller.go b/pkg/gui/controllers/files_controller.go index b70b67ab7..bf73d4c8b 100644 --- a/pkg/gui/controllers/files_controller.go +++ b/pkg/gui/controllers/files_controller.go @@ -636,7 +636,7 @@ func (self *FilesController) press(nodes []*filetree.FileNode) error { return err } - self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.FILES}, Mode: types.ASYNC}) + self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.FILES}}) self.context().HandleFocus(types.OnFocusOpts{}) return nil @@ -921,7 +921,7 @@ func (self *FilesController) toggleStagedAll() error { return err } - self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.FILES}, Mode: types.ASYNC}) + self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.FILES}}) self.context().HandleFocus(types.OnFocusOpts{}) return nil @@ -1204,7 +1204,7 @@ func (self *FilesController) setStatusFiltering(filter filetree.FileTreeDisplayF // Whenever we switch between untracked and other filters, we need to refresh the files view // because the untracked files filter applies when running `git status`. if previousFilter != filter && (previousFilter == filetree.DisplayUntracked || filter == filetree.DisplayUntracked) { - self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.FILES}, Mode: types.ASYNC}) + self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.FILES}}) } else { self.c.PostRefreshUpdate(self.context()) } @@ -1740,7 +1740,7 @@ func (self *FilesController) remove(selectedNodes []*filetree.FileNode) error { return err } - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.FILES, types.WORKTREES}}) + self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.FILES, types.WORKTREES}}) return nil }, Keys: self.c.KeybindingsOpts().GetKeys(self.c.UserConfig().Keybinding.Files.ConfirmDiscard), @@ -1766,7 +1766,7 @@ func (self *FilesController) remove(selectedNodes []*filetree.FileNode) error { return err } - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.FILES, types.WORKTREES}}) + self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.FILES, types.WORKTREES}}) return nil }, Keys: menuKey('u'), @@ -1808,7 +1808,7 @@ func (self *FilesController) ResetSubmodule(submodule *models.SubmoduleConfig) e return err } - self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.FILES, types.SUBMODULES}}) + self.c.RefreshFromWorker(types.RefreshOptions{Scope: []types.RefreshableView{types.FILES, types.SUBMODULES}}) return nil }) } diff --git a/pkg/gui/controllers/global_controller.go b/pkg/gui/controllers/global_controller.go index 8b9871294..77ef29070 100644 --- a/pkg/gui/controllers/global_controller.go +++ b/pkg/gui/controllers/global_controller.go @@ -158,7 +158,7 @@ func (self *GlobalController) createCustomPatchOptionsMenu() error { } func (self *GlobalController) refresh() error { - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + self.c.Refresh(types.RefreshOptions{}) return nil } diff --git a/pkg/gui/controllers/helpers/app_status_helper.go b/pkg/gui/controllers/helpers/app_status_helper.go index 90b87b3b8..69daa7d7a 100644 --- a/pkg/gui/controllers/helpers/app_status_helper.go +++ b/pkg/gui/controllers/helpers/app_status_helper.go @@ -86,16 +86,33 @@ func (self *AppStatusHelper) WithWaitingStatusImpl(message string, f func(gocui. }) } -func (self *AppStatusHelper) WithWaitingStatusSync(message string, f func() error) error { - self.c.PauseBackgroundRefreshes(true) - defer self.c.PauseBackgroundRefreshes(false) - - return self.statusMgr().WithWaitingStatus(message, func() {}, func(*status.WaitingStatusHandle) error { - stop := make(chan struct{}) - defer func() { close(stop) }() - self.renderAppStatusSync(stop) - - return f() +// WithWaitingStatusBlockingInput is like WithWaitingStatus, but it also blocks +// keyboard input for the whole duration of the operation: keys the user presses +// while it runs are buffered and replayed against the post-operation state (see +// gocui.BeginBlockingEvents). Use it for operations that manipulate an +// in-progress rebase or otherwise rewrite commits, where a racing keypress +// would target the wrong commit or todo. +// +// Must be called on the UI thread: the block is begun synchronously here, before +// the operation is dispatched to a worker, so no keypress can slip through in +// between. +func (self *AppStatusHelper) WithWaitingStatusBlockingInput(message string, f func(gocui.Task) error) { + self.c.GocuiGui().BeginBlockingEvents() + // Hide the rebasing-mode indicator (and its reset button) while we drive the + // rebase ourselves; it reflects the transient on-disk state and would + // otherwise flash on for the duration of the operation. + self.modeHelper.SetSuppressRebasingMode(true) + self.c.OnWorker(func(task gocui.Task) error { + // End the block and restore the mode indicator once the operation and its + // refresh have applied their UI updates: OnUIThread queues this after the + // refresh's model bounces and Then (which RefreshFromWorker has already + // enqueued by the time f returns), so the replayed keys act on the + // refreshed state and any resulting rebase state shows correctly. + defer self.c.OnUIThread(func() error { + self.modeHelper.SetSuppressRebasingMode(false) + return self.c.GocuiGui().EndBlockingEvents() + }) + return self.WithWaitingStatusImpl(message, f, task, false) }) } @@ -150,57 +167,3 @@ func (self *AppStatusHelper) renderAppStatus(background bool) { return nil }) } - -func (self *AppStatusHelper) renderAppStatusSync(stop chan struct{}) { - go func() { - ticker := time.NewTicker(time.Millisecond * time.Duration(self.c.UserConfig().Gui.Spinner.Rate)) - defer ticker.Stop() - - // Write the status into the view before the first layout below, so that - // layout (which sizes the bottom line based on the actual content of the - // AppStatus view) leaves room for it and it shows right away. The ticker - // only updates the spinner frame using ForceFlushViewsContentOnly, so this - // doesn't re-layout. - self.setAppStatusContent() - - // Forcing a re-layout and redraw after we added the waiting status; - // this is needed in case the gui.showBottomLine config is set to false, - // to make sure the bottom line appears. It's also useful for redrawing - // once after each of several consecutive keypresses, e.g. pressing - // ctrl-j to move a commit down several steps. - _ = self.c.GocuiGui().ForceLayoutAndRedraw() - - self.modeHelper.SetSuppressRebasingMode(true) - defer func() { self.modeHelper.SetSuppressRebasingMode(false) }() - - outer: - for { - select { - case <-ticker.C: - self.setAppStatusContent() - // Redraw all views of the bottom line: - bottomLineViews := []*gocui.View{ - self.c.Views().AppStatus, self.c.Views().Options, self.c.Views().Information, - self.c.Views().StatusSpacer1, self.c.Views().StatusSpacer2, - } - _ = self.c.GocuiGui().ForceFlushViewsContentOnly(bottomLineViews) - case <-stop: - // Clear the status from the view and re-layout, otherwise the - // stale content would keep layout reserving room for it forever. - // The UI thread is free again at this point, so we go through - // OnUIThread like the async renderAppStatus does. - self.c.OnUIThread(func() error { - self.c.SetViewContent(self.c.Views().AppStatus, "") - return nil - }) - break outer - } - } - }() -} - -func (self *AppStatusHelper) setAppStatusContent() { - appStatus, color := self.statusMgr().GetStatusString(self.c.UserConfig()) - self.c.Views().AppStatus.FgColor = color - self.c.SetViewContent(self.c.Views().AppStatus, appStatus) -} diff --git a/pkg/gui/controllers/helpers/bisect_helper.go b/pkg/gui/controllers/helpers/bisect_helper.go index 6ce517dac..bc9548c4c 100644 --- a/pkg/gui/controllers/helpers/bisect_helper.go +++ b/pkg/gui/controllers/helpers/bisect_helper.go @@ -31,5 +31,5 @@ func (self *BisectHelper) Reset() error { } func (self *BisectHelper) PostBisectCommandRefresh() { - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{}}) + self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{}}) } diff --git a/pkg/gui/controllers/helpers/branches_helper.go b/pkg/gui/controllers/helpers/branches_helper.go index 5c72bacfd..83735d87d 100644 --- a/pkg/gui/controllers/helpers/branches_helper.go +++ b/pkg/gui/controllers/helpers/branches_helper.go @@ -49,7 +49,7 @@ func (self *BranchesHelper) ConfirmLocalDelete(branches []*models.Branch) error self.c.Contexts().Branches.CollapseRangeSelectionToTop() return nil }) - self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.BRANCHES}}) + self.c.RefreshFromWorker(types.RefreshOptions{Scope: []types.RefreshableView{types.BRANCHES}}) return nil }) }) @@ -87,7 +87,7 @@ func (self *BranchesHelper) ConfirmDeleteRemote(remoteBranches []*models.RemoteB if err := self.deleteRemoteBranches(remoteBranches, task); err != nil { return err } - self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.BRANCHES, types.REMOTES}}) + self.c.RefreshFromWorker(types.RefreshOptions{Scope: []types.RefreshableView{types.BRANCHES, types.REMOTES}}) if resetRemoteBranchesSelection { self.c.OnUIThread(func() error { self.c.Contexts().RemoteBranches.CollapseRangeSelectionToTop() @@ -161,7 +161,7 @@ func (self *BranchesHelper) ConfirmLocalAndRemoteDelete(branches []*models.Branc self.c.Contexts().Branches.CollapseRangeSelectionToTop() return nil }) - self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.BRANCHES, types.REMOTES}}) + self.c.RefreshFromWorker(types.RefreshOptions{Scope: []types.RefreshableView{types.BRANCHES, types.REMOTES}}) return nil }) }, @@ -325,7 +325,6 @@ func (self *BranchesHelper) deleteLocalBranchesContinuation(branches []*models.B return nil }) self.c.RefreshFromWorker(types.RefreshOptions{ - Mode: types.ASYNC, Scope: []types.RefreshableView{types.WORKTREES, types.BRANCHES, types.FILES}, }) return nil @@ -346,7 +345,6 @@ func (self *BranchesHelper) deleteLocalAndRemoteBranchesContinuation(branches [] return nil }) self.c.RefreshFromWorker(types.RefreshOptions{ - Mode: types.ASYNC, Scope: []types.RefreshableView{types.WORKTREES, types.BRANCHES, types.REMOTES, types.FILES}, }) return nil @@ -407,7 +405,6 @@ func (self *BranchesHelper) PostFetchRefresh(fetchErr error, background bool) er // 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 { @@ -458,7 +455,7 @@ func (self *BranchesHelper) AutoForwardBranches(background bool) 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, Background: background}) + self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.BRANCHES}, 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 673f657f5..fc96b9d1b 100644 --- a/pkg/gui/controllers/helpers/cherry_pick_helper.go +++ b/pkg/gui/controllers/helpers/cherry_pick_helper.go @@ -4,6 +4,7 @@ import ( "strconv" "github.com/jesseduffield/lazygit/pkg/commands/models" + "github.com/jesseduffield/lazygit/pkg/gocui" "github.com/jesseduffield/lazygit/pkg/gui/modes/cherrypicking" "github.com/jesseduffield/lazygit/pkg/gui/types" "github.com/jesseduffield/lazygit/pkg/utils" @@ -82,9 +83,9 @@ func (self *CherryPickHelper) Paste() error { "numCommits": strconv.Itoa(len(self.getData().CherryPickedCommits)), }), HandleConfirm: func() error { - return self.c.WithWaitingStatusSync(self.c.Tr.CherryPickingStatus, func() error { - mustStash := IsWorkingTreeDirtyExceptSubmodules(self.c.Model().Files, self.c.Model().Submodules) - + mustStash := IsWorkingTreeDirtyExceptSubmodules(self.c.Model().Files, self.c.Model().Submodules) + cherryPickedCommits := self.getData().CherryPickedCommits + return self.c.WithWaitingStatusBlockingInput(self.c.Tr.CherryPickingStatus, func(gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.CherryPick) if mustStash { @@ -93,9 +94,9 @@ func (self *CherryPickHelper) Paste() error { } } - cherryPickedCommits := self.getData().CherryPickedCommits result := self.c.Git().Rebase.CherryPickCommits(cherryPickedCommits) - err := self.rebaseHelper.CheckMergeOrRebaseWithRefreshOptionsFromUIThread(result, types.RefreshOptions{Mode: types.SYNC}) + err := self.rebaseHelper.CheckMergeOrRebaseWithRefreshOptions(result, + types.RefreshOptions{BatchUIUpdates: true}) if err != nil { return result } @@ -109,14 +110,19 @@ func (self *CherryPickHelper) Paste() error { return result } if !isInCherryPick { - self.getData().DidPaste = true - self.rerender() + // DidPaste and the re-render touch mode state and contexts, + // so run them on the UI thread. + self.c.OnUIThread(func() error { + self.getData().DidPaste = true + self.rerender() + return nil + }) if mustStash { if err := self.c.Git().Stash.Pop(0); err != nil { return err } - self.c.Refresh(types.RefreshOptions{ + self.c.RefreshFromWorker(types.RefreshOptions{ Scope: []types.RefreshableView{types.STASH, types.FILES}, }) } diff --git a/pkg/gui/controllers/helpers/credentials_helper.go b/pkg/gui/controllers/helpers/credentials_helper.go index 9b2198ccb..7c765020e 100644 --- a/pkg/gui/controllers/helpers/credentials_helper.go +++ b/pkg/gui/controllers/helpers/credentials_helper.go @@ -33,7 +33,7 @@ func (self *CredentialsHelper) PromptUserForCredential(passOrUname oscommands.Cr HandleConfirm: func(input string) error { ch <- input + "\n" - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + self.c.Refresh(types.RefreshOptions{}) return nil }, HandleClose: func() error { diff --git a/pkg/gui/controllers/helpers/diff_helper.go b/pkg/gui/controllers/helpers/diff_helper.go index 668ee916a..6af3b2b5c 100644 --- a/pkg/gui/controllers/helpers/diff_helper.go +++ b/pkg/gui/controllers/helpers/diff_helper.go @@ -94,7 +94,7 @@ func (self *DiffHelper) FilterPathsForCommit(commit *models.Commit) []string { func (self *DiffHelper) ExitDiffMode() error { self.c.Modes().Diffing = diffing.New() - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + self.c.Refresh(types.RefreshOptions{}) return nil } diff --git a/pkg/gui/controllers/helpers/fixup_helper.go b/pkg/gui/controllers/helpers/fixup_helper.go index dfde8365b..a958e58a8 100644 --- a/pkg/gui/controllers/helpers/fixup_helper.go +++ b/pkg/gui/controllers/helpers/fixup_helper.go @@ -137,7 +137,7 @@ func (self *FixupHelper) HandleFindBaseCommitForFixupPress() error { if err := self.c.Git().WorkingTree.StageAll(true); err != nil { return err } - self.c.Refresh(types.RefreshOptions{Mode: types.SYNC, Scope: []types.RefreshableView{types.FILES}}) + self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.FILES}}) } self.c.Contexts().LocalCommits.SetSelection(index) diff --git a/pkg/gui/controllers/helpers/gpg_helper.go b/pkg/gui/controllers/helpers/gpg_helper.go index fd74a400b..9c7667a6d 100644 --- a/pkg/gui/controllers/helpers/gpg_helper.go +++ b/pkg/gui/controllers/helpers/gpg_helper.go @@ -26,7 +26,7 @@ func (self *GpgHelper) WithGpgHandling( onSuccess func() error, refreshScope []types.RefreshableView, ) error { - refreshOptions := types.RefreshOptions{Mode: types.ASYNC, Scope: refreshScope} + refreshOptions := types.RefreshOptions{Scope: refreshScope} return self.withGpgHandling( cmdObj, configKey, waitingStatus, onSuccess, refreshOptions, refreshOptions) } @@ -40,8 +40,8 @@ func (self *GpgHelper) WithGpgHandlingAndSelectHeadCommit( waitingStatus string, onSuccess func() error, ) error { - failureRefreshOptions := types.RefreshOptions{Mode: types.ASYNC} - successRefreshOptions := types.RefreshOptions{Mode: types.ASYNC, CommitSelection: types.SelectHeadCommit} + failureRefreshOptions := types.RefreshOptions{} + successRefreshOptions := types.RefreshOptions{CommitSelection: types.SelectHeadCommit} return self.withGpgHandling( cmdObj, configKey, waitingStatus, onSuccess, failureRefreshOptions, successRefreshOptions) } diff --git a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go index b0c53b831..a8696a21c 100644 --- a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go +++ b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go @@ -89,11 +89,11 @@ 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 -// 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. +// CheckMergeOrRebaseWithRefreshOptions, which already runs on a worker, so it +// must not spin up a second waiting status. calledFromWorker is used only by the +// subprocess path below: it's true for that recursive worker skip and false for +// genericMergeCommand's UI-thread invocation, so the post-action refresh picks +// RefreshFromWorker vs Refresh correctly. func (self *MergeAndRebaseHelper) genericMergeCommandImpl(command string, showWaitingStatus bool, calledFromWorker bool) error { status := self.c.Git().Status.WorkingTreeState() @@ -133,29 +133,29 @@ func (self *MergeAndRebaseHelper) genericMergeCommandImpl(command string, showWa // 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.refreshAfterMergeOrRebase(types.RefreshOptions{ - Mode: types.ASYNC, CommitSelection: commitSelectionAfterMerge(success && selectHeadCommitOnSuccess), }, calledFromWorker) self.RecordWhetherMergeOrRebaseStartedInLazygit() return err } - runAction := func(calledFromWorker bool) error { + // runAction always ends up on a worker: either the waiting status below spins + // one up, or we're the recursive auto-skip reached from + // CheckMergeOrRebaseWithRefreshOptions, which already runs on one. + runAction := func() error { result := self.c.Git().Rebase.GenericMergeOrRebaseAction(commandType, command) - return self.checkMergeOrRebaseImpl(result, + return self.CheckMergeOrRebaseWithRefreshOptions(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 { - // The waiting status ran runAction on a worker. - return runAction(true) + return runAction() }) } - return runAction(calledFromWorker) + return runAction() } // commitSelectionAfterMerge maps whether a merge/rebase/pull created a new @@ -211,33 +211,19 @@ func (self *MergeAndRebaseHelper) RecordWhetherMergeOrRebaseStartedInLazygit() { } // 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. +// step and refreshes. It always runs on a worker (the WithWaitingStatus / +// WithWaitingStatusBlockingInput / WithInlineStatus handlers). func (self *MergeAndRebaseHelper) CheckMergeOrRebaseWithRefreshOptions(result error, refreshOptions types.RefreshOptions) error { - 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.refreshAfterMergeOrRebase(refreshOptions, true) 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, calledFromWorker) + return self.genericMergeCommandImpl(REBASE_OPTION_SKIP, false, true) } else if strings.Contains(result.Error(), "The previous cherry-pick is now empty") { - return self.genericMergeCommandImpl(REBASE_OPTION_SKIP, false, calledFromWorker) + return self.genericMergeCommandImpl(REBASE_OPTION_SKIP, false, true) } else if strings.Contains(result.Error(), "No rebase in progress?") { // assume in this case that we're already done return nil @@ -247,8 +233,8 @@ func (self *MergeAndRebaseHelper) checkMergeOrRebaseImpl(result error, refreshOp // 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. +// worker callers, Refresh for the merge/rebase-continue subprocess path that +// stays on the UI thread. func (self *MergeAndRebaseHelper) refreshAfterMergeOrRebase(refreshOptions types.RefreshOptions, calledFromWorker bool) { if calledFromWorker { self.c.RefreshFromWorker(refreshOptions) @@ -258,7 +244,7 @@ func (self *MergeAndRebaseHelper) refreshAfterMergeOrRebase(refreshOptions types } func (self *MergeAndRebaseHelper) CheckMergeOrRebase(result error) error { - return self.CheckMergeOrRebaseWithRefreshOptions(result, types.RefreshOptions{Mode: types.ASYNC}) + return self.CheckMergeOrRebaseWithRefreshOptions(result, types.RefreshOptions{}) } // Like CheckMergeOrRebase, but for operations that create a new commit at HEAD @@ -267,7 +253,7 @@ func (self *MergeAndRebaseHelper) CheckMergeOrRebase(result error) error { // before the refresh. func (self *MergeAndRebaseHelper) CheckMergeOrRebaseAndSelectHeadCommit(result error) error { return self.CheckMergeOrRebaseWithRefreshOptions(result, - types.RefreshOptions{Mode: types.SYNC, CommitSelection: commitSelectionAfterMerge(result == nil)}) + types.RefreshOptions{CommitSelection: commitSelectionAfterMerge(result == nil)}) } func (self *MergeAndRebaseHelper) CheckForConflicts(result error) error { @@ -321,7 +307,7 @@ func (self *MergeAndRebaseHelper) AbortMergeOrRebaseWithConfirm() error { } // PromptToContinueRebase asks the user if they want to continue the rebase/merge that's in progress -func (self *MergeAndRebaseHelper) PromptToContinueRebase() error { +func (self *MergeAndRebaseHelper) PromptToContinueRebase() { self.continueRebasePromptShowing = true self.c.Confirm(types.ConfirmOpts{ Title: self.c.Tr.Continue, @@ -346,7 +332,7 @@ func (self *MergeAndRebaseHelper) PromptToContinueRebase() error { // 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}, + Scope: []types.RefreshableView{types.FILES}, Then: func() error { unstagedFiles := GetUnstagedFilesExceptSubmodules(self.c.Model().Files, self.c.Model().Submodules) if len(unstagedFiles) > 0 { @@ -373,8 +359,6 @@ func (self *MergeAndRebaseHelper) PromptToContinueRebase() error { return nil }, }) - - return nil } // DismissContinueRebasePromptIfShowing closes the "continue the rebase/merge?" @@ -669,7 +653,7 @@ func (self *MergeAndRebaseHelper) SquashMergeCommitted(refName, checkedOutBranch if err != nil { return err } - self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC}) + self.c.RefreshFromWorker(types.RefreshOptions{}) return nil }) } diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index 21fdc8a7a..768e7a618 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -96,287 +96,334 @@ type refreshEnv struct { // the repo generation captured when the refresh started generation int + + // When non-nil, each scope's UI-thread bounce is collected here instead of + // being dispatched as it's produced, so they can all be applied in a single + // frame once the whole refresh is done (see RefreshOptions.BatchUIUpdates). + // Held by pointer so the copies of env that flow through the scope functions + // all share the one batch. + batch *refreshBounceBatch +} + +// refreshBounceBatch collects the UI-thread bounces of a batched refresh so they +// can be applied together in one frame rather than one scope at a time. The +// scopes run on separate worker goroutines and add concurrently, hence the +// mutex. Once the refresh starts flushing it closes the batch, so that any +// bounces enqueued afterwards — the nested ones a flushed bounce produces in +// turn, e.g. scrolling the selection into view — are dispatched immediately as +// ordinary follow-ups instead of being collected into a batch that nothing +// will drain. +type refreshBounceBatch struct { + mutex deadlock.Mutex + funcs []func() + closed bool +} + +// add collects f and returns true. Once the batch is closed it collects nothing +// and returns false, telling the caller to dispatch f immediately instead. +func (self *refreshBounceBatch) add(f func()) bool { + self.mutex.Lock() + defer self.mutex.Unlock() + + if self.closed { + return false + } + self.funcs = append(self.funcs, f) + return true +} + +// close marks the batch flushed and returns everything collected so far. +func (self *refreshBounceBatch) close() []func() { + self.mutex.Lock() + defer self.mutex.Unlock() + + self.closed = true + return self.funcs } 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") + startTime := time.Now() + + // A refresh from a worker blocks that worker until it's done; one from the + // UI thread returns immediately and finishes in the background. + syncOrAsync := "async" + if calledFromWorker { + syncOrAsync = "sync" } - - t := time.Now() - defer func() { - self.c.Log.Infof("Refresh took %s", time.Since(t)) - }() - if options.Scope == nil { - self.c.Log.Infof( - "refreshing all scopes in %s mode", - getModeName(options.Mode), - ) + self.c.Log.Infof("refreshing all scopes (%s)", syncOrAsync) } else { self.c.Log.Infof( - "refreshing the following scopes in %s mode: %s", - getModeName(options.Mode), + "refreshing the following scopes (%s): %s", + syncOrAsync, strings.Join(getScopeNames(options.Scope), ","), ) } - // 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). + // worker. 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(), - } + // 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(), + } + if options.BatchUIUpdates { + env.batch = &refreshBounceBatch{} + } - var scopeSet *set.Set[types.RefreshableView] - if len(options.Scope) == 0 { - // not refreshing staging/patch-building unless explicitly requested because we only need - // to refresh those while focused. - scopeSet = set.NewFromSlice([]types.RefreshableView{ - types.COMMITS, - types.BRANCHES, - types.FILES, - types.STASH, - types.REFLOG, - types.TAGS, - types.REMOTES, - types.WORKTREES, - types.STATUS, - types.BISECT_INFO, - types.STAGING, - types.PULL_REQUESTS, - }) - } else { - scopeSet = set.NewFromSlice(options.Scope) - } + var scopeSet *set.Set[types.RefreshableView] + if len(options.Scope) == 0 { + // not refreshing staging/patch-building unless explicitly requested because we only need + // to refresh those while focused. + scopeSet = set.NewFromSlice([]types.RefreshableView{ + types.COMMITS, + types.BRANCHES, + types.FILES, + types.STASH, + types.REFLOG, + types.TAGS, + types.REMOTES, + types.WORKTREES, + types.STATUS, + types.BISECT_INFO, + types.STAGING, + types.PULL_REQUESTS, + }) + } else { + scopeSet = set.NewFromSlice(options.Scope) + } - // Expand co-refreshing scopes up front so downstream conditions can be - // simple single-scope checks. The relationships are: - // - whenever the reflog or bisect info changes, commits and branches - // can change too (e.g. switching branches updates the reflog and - // 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) - } - if scopeSet.Includes(types.SUBMODULES) { - scopeSet.Add(types.FILES) - } - if scopeSet.Includes(types.FILES) { - scopeSet.Add(types.MERGE_CONFLICTS) - } - if scopeSet.Includes(types.PULL_REQUESTS) { - scopeSet.Add(types.BRANCHES, types.REMOTES) - } + // Expand co-refreshing scopes up front so downstream conditions can be + // simple single-scope checks. The relationships are: + // - whenever the reflog or bisect info changes, commits and branches + // can change too (e.g. switching branches updates the reflog and + // 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) + } + if scopeSet.Includes(types.SUBMODULES) { + scopeSet.Add(types.FILES) + } + 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 - // of git's state changing externally while (or right after) we are - // refreshing; the risk is one potential extra refresh, but capturing the - // snapshot at the end would risk missing one, which is worse. - self.updateRefsSnapshotIfRelevant(scopeSet) + // Capture the refs snapshot now, before we start reading git's state + // below, rather than after. This is important to guard against the race + // of git's state changing externally while (or right after) we are + // refreshing; the risk is one potential extra refresh, but capturing the + // snapshot at the end would risk missing one, which is worse. + self.updateRefsSnapshotIfRelevant(scopeSet) - wg := sync.WaitGroup{} - refresh := func(name string, f func()) { - // if we're in a demo we don't want any async refreshes because - // everything happens fast and it's better to have everything update - // in the one frame - if !self.c.InDemo() && options.Mode == types.ASYNC { - self.onWorker(env.background, func(t gocui.Task) error { - f() - return nil - }) - } else { - wg.Add(1) - go utils.Safe(func() { - t := time.Now() - defer wg.Done() - f() - self.c.Log.Infof("refreshed %s in %s", name, time.Since(t)) - }) - } - } + wg := sync.WaitGroup{} + refresh := func(name string, f func()) { + wg.Add(1) + // Each scope runs on its own goroutine, joined by the wg.Wait in + // waitAndFinalize. They don't need to be registered as gocui tasks for + // repo-switch safety: performRefresh always runs under a task that stays + // busy until that wg.Wait returns — the calling worker's task when + // called from a worker, or the waitAndFinalize worker task when called + // from the UI thread (created before the triggering event's task ends, + // so there's no gap) — and that task already covers the whole refresh. + go utils.Safe(func() { + t := time.Now() + defer wg.Done() + f() + self.c.Log.Infof("refreshed %s in %s", name, time.Since(t)) + }) + } - 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(capturedCommits, options.CommitSelection, env) - }) + 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(calledFromWorker, env.background, func() { + capturedCommits = self.captureCommitsState(options.CommitSelection) + capturedReflog = self.captureReflogState() + capturedBranches = self.captureBranchState() + }) + refresh("commits and commit files", func() { + 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() { - loadedBranches = self.refreshReflogAndBranches(capturedReflog, capturedBranches, includeWorktreesWithBranches, options.BranchSelection, options.SelectTopReflogCommit, env) - branchesAndRemotesWg.Done() - }) - } else { - branchesAndRemotesWg.Add(1) - refresh("branches", func() { - // 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(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 - 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) { - 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) { - 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(capturedFiles, env) - fileWg.Done() - }) - } - - if scopeSet.Includes(types.STASH) { - 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(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() - }) + includeWorktreesWithBranches = scopeSet.Includes(types.WORKTREES) + if self.c.UserConfig().Git.LocalBranchSortOrder == "recency" { branchesAndRemotesWg.Add(1) - refresh("remotes", func() { - loadedRemotes, _ = self.refreshRemotes(prevSelectedRemote, env) + refresh("reflog and branches", func() { + loadedBranches = self.refreshReflogAndBranches(capturedReflog, capturedBranches, includeWorktreesWithBranches, options.BranchSelection, options.SelectTopReflogCommit, env) branchesAndRemotesWg.Done() }) - } - - if scopeSet.Includes(types.PULL_REQUESTS) { - refresh("pull requests", func() { - branchesAndRemotesWg.Wait() - // 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) + } else { + branchesAndRemotesWg.Add(1) + refresh("branches", func() { + // 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(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 + var rebaseHashPool *utils.StringPool + var rebaseCommits []*models.Commit + self.captureOnUIThread(calledFromWorker, env.background, func() { + rebaseHashPool, rebaseCommits = self.captureRebaseCommitState() + }) + refresh("rebase commits", func() { _ = self.refreshRebaseCommits(rebaseHashPool, rebaseCommits, env) }) + } - if scopeSet.Includes(types.WORKTREES) && !includeWorktreesWithBranches { - refresh("worktrees", func() { self.refreshWorktrees(env) }) - } + if scopeSet.Includes(types.SUB_COMMITS) { + var capturedSubCommits capturedSubCommitState + self.captureOnUIThread(calledFromWorker, env.background, func() { + capturedSubCommits = self.captureSubCommitState() + }) + refresh("sub commits", func() { _ = self.refreshSubCommitsWithLimit(capturedSubCommits, env) }) + } - if scopeSet.Includes(types.STAGING) { - refresh("staging", func() { - fileWg.Wait() - // 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 - }) + // 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) { + var capturedCommitFiles capturedCommitFilesState + self.captureOnUIThread(calledFromWorker, 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(calledFromWorker, env.background, func() { + capturedFiles = self.captureFilesState() + }) + fileWg.Add(1) + refresh("files", func() { + _ = self.refreshFilesAndSubmodules(capturedFiles, env) + fileWg.Done() + }) + } + + if scopeSet.Includes(types.STASH) { + var stashFilterPath string + self.captureOnUIThread(calledFromWorker, 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(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(calledFromWorker, env.background, func() { + prevSelectedRemote = self.c.Contexts().Remotes.GetSelected() + }) + branchesAndRemotesWg.Add(1) + refresh("remotes", func() { + loadedRemotes, _ = self.refreshRemotes(prevSelectedRemote, env) + branchesAndRemotesWg.Done() + }) + } + + if scopeSet.Includes(types.PULL_REQUESTS) { + self.onWorker(env.background, func(gocui.Task) error { + branchesAndRemotesWg.Wait() + + t := time.Now() + // 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) + self.c.Log.Infof("refreshed pull requests in %s", time.Since(t)) + return nil + }) + } + + if scopeSet.Includes(types.WORKTREES) && !includeWorktreesWithBranches { + refresh("worktrees", func() { self.refreshWorktrees(env) }) + } + + if scopeSet.Includes(types.STAGING) { + refresh("staging", func() { + fileWg.Wait() + // 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() { + self.stagingHelper.RefreshStagingPanel(types.OnFocusOpts{}) }) - } + }) + } - if scopeSet.Includes(types.PATCH_BUILDING) { - refresh("patch building", func() { self.patchBuildingHelper.RefreshPatchBuildingPanel(types.OnFocusOpts{}) }) - } + if scopeSet.Includes(types.PATCH_BUILDING) { + refresh("patch building", func() { self.patchBuildingHelper.RefreshPatchBuildingPanel(types.OnFocusOpts{}) }) + } - if scopeSet.Includes(types.MERGE_CONFLICTS) { - refresh("merge conflicts", func() { _ = self.mergeConflictsHelper.RefreshMergeState(env.background) }) - } + if scopeSet.Includes(types.MERGE_CONFLICTS) { + refresh("merge conflicts", func() { _ = self.mergeConflictsHelper.RefreshMergeState(env.background) }) + } - self.refreshStatus(env) + self.refreshStatus(env) + waitAndFinalize := func() { wg.Wait() + if env.batch != nil { + // Apply all the scopes' collected bounces in a single UI-thread task, + // so they land in one frame: gocui drains every queued event before it + // redraws, so one task means one repaint. Bounces enqueued from within + // these (see refreshBounceBatch) run as ordinary follow-ups. + bounces := env.batch.close() + self.onUIThread(env.background, func() error { + for _, bounce := range bounces { + bounce() + } + return nil + }) + } + if options.Then != nil { // Queue Then via OnUIThread so it runs *after* the refresh-scope // functions' model-update bounces (which are already queued by @@ -386,17 +433,21 @@ func (self *RefreshHelper) performRefresh(options types.RefreshOptions, calledFr // still pre-refresh. self.onUIThread(env.background, options.Then) } + + self.c.Log.Infof("Refresh took %s", time.Since(startTime)) } - if options.Mode == types.BLOCK_UI { - self.c.OnUIThread(func() error { - f() + // waitAndFinalize blocks until every scope is done. Run it inline when we're + // already on a worker (or in a demo, for a deterministic single frame); when + // we're on the UI thread, dispatch it to a worker so it doesn't block the UI. + if calledFromWorker || self.c.InDemo() { + waitAndFinalize() + } else { + self.onWorker(env.background, func(t gocui.Task) error { + waitAndFinalize() return nil }) - return } - - f() } // SetRefsSnapshot stores the given snapshot as the last observed refs state. @@ -477,19 +528,6 @@ func getScopeNames(scopes []types.RefreshableView) []string { }) } -func getModeName(mode types.RefreshMode) string { - switch mode { - case types.SYNC: - return "sync" - case types.ASYNC: - return "async" - case types.BLOCK_UI: - return "block-ui" - default: - return "unknown mode" - } -} - // 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 @@ -622,7 +660,7 @@ func (self *RefreshHelper) refreshCommitsAndCommitFiles(captured capturedCommitS // 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 { + self.onUIThreadUnlessRepoChanged(env, func() { commit := self.c.Contexts().LocalCommits.GetSelected() if commit != nil && commit.RefName() != "" { refRange := self.c.Contexts().LocalCommits.GetSelectedRefRangeForDiffFiles() @@ -635,7 +673,6 @@ func (self *RefreshHelper) refreshCommitsAndCommitFiles(captured capturedCommitS return nil }) } - return nil }) } } @@ -687,7 +724,7 @@ func (self *RefreshHelper) refreshCommitsWithLimit(captured capturedCommitState, } workingTreeState := self.c.Git().Status.WorkingTreeState() - self.onUIThreadUnlessRepoChanged(env, func() error { + self.onUIThreadUnlessRepoChanged(env, func() { self.c.Model().BisectInfo = bisectInfo self.c.Model().Commits = commits self.RefreshAuthors(commits) @@ -721,12 +758,10 @@ func (self *RefreshHelper) refreshCommitsWithLimit(captured capturedCommitState, // 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.onUIThreadUnlessRepoChanged(env, func() { self.c.Contexts().LocalCommits.FocusLine(true) - return nil }) } - return nil }) self.refreshView(self.c.Contexts().LocalCommits, env) @@ -856,10 +891,9 @@ func (self *RefreshHelper) refreshSubCommitsWithLimit(captured capturedSubCommit if err != nil { return err } - self.onUIThreadUnlessRepoChanged(env, func() error { + self.onUIThreadUnlessRepoChanged(env, func() { self.c.Model().SubCommits = commits self.RefreshAuthors(commits) - return nil }) self.refreshView(self.c.Contexts().SubCommits, env) @@ -899,10 +933,9 @@ func (self *RefreshHelper) refreshCommitFilesContext(captured capturedCommitFile if err != nil { return err } - self.onUIThreadUnlessRepoChanged(env, func() error { + self.onUIThreadUnlessRepoChanged(env, func() { self.c.Model().CommitFiles = files self.c.Contexts().CommitFiles.CommitFileTreeViewModel.SetTree() - return nil }) self.refreshView(self.c.Contexts().CommitFiles, env) return nil @@ -921,10 +954,9 @@ func (self *RefreshHelper) refreshRebaseCommits(hashPool *utils.StringPool, comm } workingTreeState := self.c.Git().Status.WorkingTreeState() - self.onUIThreadUnlessRepoChanged(env, func() error { + self.onUIThreadUnlessRepoChanged(env, func() { self.c.Model().Commits = updatedCommits self.c.Model().WorkingTreeStateAtLastCommitRefresh = workingTreeState - return nil }) self.refreshView(self.c.Contexts().LocalCommits, env) @@ -937,9 +969,8 @@ func (self *RefreshHelper) refreshTags(env refreshEnv) error { return err } - self.onUIThreadUnlessRepoChanged(env, func() error { + self.onUIThreadUnlessRepoChanged(env, func() { self.c.Model().Tags = tags - return nil }) self.refreshView(self.c.Contexts().Tags, env) @@ -966,10 +997,9 @@ func (self *RefreshHelper) refreshBranches(captured capturedBranchState, refresh }) }, func() { - self.onUIThreadUnlessRepoChanged(env, func() error { + self.onUIThreadUnlessRepoChanged(env, func() { self.c.Contexts().Branches.HandleRender() self.refreshStatus(env) - return nil }) }) if err != nil { @@ -981,14 +1011,14 @@ func (self *RefreshHelper) refreshBranches(captured capturedBranchState, refresh worktrees = self.loadWorktrees() } - self.onUIThreadUnlessRepoChanged(env, func() error { + self.onUIThreadUnlessRepoChanged(env, func() { // 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 + return } self.appliedBranchLoadSeq = loadSeq @@ -1031,7 +1061,6 @@ func (self *RefreshHelper) refreshBranches(captured capturedBranchState, refresh // Need to re-render the commits view because the visualization of local // branch heads might have changed self.c.Contexts().LocalCommits.HandleRender() - return nil }) self.refreshView(self.c.Contexts().Branches, env) @@ -1066,13 +1095,22 @@ func (self *RefreshHelper) refreshFilesAndSubmodules(captured capturedFilesState // 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 { +func (self *RefreshHelper) onUIThreadUnlessRepoChanged(env refreshEnv, f func()) { + wrapper := func() { if self.c.State().GetRepoGeneration() != env.generation { - return nil + return } - return f() - }) + f() + } + + // A batched refresh collects its bounces and fires them together at the end + // (see refreshBounceBatch); add reports false once the batch is flushing, so + // bounces enqueued from within a flushed bounce dispatch immediately. + if env.batch != nil && env.batch.add(wrapper) { + return + } + + self.onUIThread(env.background, func() error { wrapper(); return nil }) } // onWorker and onUIThread pick the foreground or background variant of the @@ -1100,17 +1138,16 @@ func (self *RefreshHelper) onUIThread(background bool, f func() error) { // 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 +// runs on the UI thread (calledFromWorker is false) 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 +// The inline case matters for correctness as much as the hop: a SYNC refresh +// initiated on the UI thread parks that 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 { +// thread — avoids that entirely. +func (self *RefreshHelper) captureOnUIThread(calledFromWorker bool, background bool, fn func()) { + if !calledFromWorker { fn() return } @@ -1206,7 +1243,7 @@ func (self *RefreshHelper) refreshStateFiles(captured capturedFilesState, env re // (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.onUIThreadUnlessRepoChanged(env, func() error { + self.onUIThreadUnlessRepoChanged(env, func() { // The merge-conflicts scope of this refresh also notices that // the conflicts are gone and escapes from the merge conflicts // view to the files context (see RefreshMergeState), but it @@ -1220,7 +1257,7 @@ func (self *RefreshHelper) refreshStateFiles(captured capturedFilesState, env re self.mergeConflictsHelper.ResetMergeState() self.c.Context().Push(self.c.Contexts().Files, types.OnFocusOpts{}) } - return self.mergeAndRebaseHelper.PromptToContinueRebase() + self.mergeAndRebaseHelper.PromptToContinueRebase() }) } } else { @@ -1231,13 +1268,12 @@ func (self *RefreshHelper) refreshStateFiles(captured capturedFilesState, env re // 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.onUIThreadUnlessRepoChanged(env, func() { self.mergeAndRebaseHelper.DismissContinueRebasePromptIfShowing() - return nil }) } - self.onUIThreadUnlessRepoChanged(env, func() error { + self.onUIThreadUnlessRepoChanged(env, func() { // 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 { @@ -1252,7 +1288,6 @@ func (self *RefreshHelper) refreshStateFiles(captured capturedFilesState, env re self.c.Model().Submodules = submoduleConfigs self.c.Model().Files = files fileTreeViewModel.SetTree() - return nil }) return nil @@ -1307,7 +1342,7 @@ func (self *RefreshHelper) refreshReflogCommits(captured capturedReflogState, en } } - self.onUIThreadUnlessRepoChanged(env, func() error { + self.onUIThreadUnlessRepoChanged(env, func() { model.ReflogCommits = reflogCommits model.FilteredReflogCommits = filteredReflogCommits // Setting the selection here, in the same bounce that writes the list, @@ -1317,7 +1352,6 @@ func (self *RefreshHelper) refreshReflogCommits(captured capturedReflogState, en self.c.Contexts().ReflogCommits.SetSelectedLineIdx(0) self.c.Contexts().ReflogCommits.GetView().SetOriginY(0) } - return nil }) self.refreshView(self.c.Contexts().ReflogCommits, env) @@ -1330,7 +1364,7 @@ func (self *RefreshHelper) refreshRemotes(prevSelectedRemote *models.Remote, env return nil, err } - self.onUIThreadUnlessRepoChanged(env, func() error { + self.onUIThreadUnlessRepoChanged(env, func() { self.c.Model().Remotes = remotes hadPrs := len(self.c.Model().PullRequestsMap) != 0 @@ -1350,7 +1384,6 @@ func (self *RefreshHelper) refreshRemotes(prevSelectedRemote *models.Remote, env } } } - return nil }) self.refreshView(self.c.Contexts().Remotes, env) @@ -1370,9 +1403,8 @@ func (self *RefreshHelper) loadWorktrees() []*models.Worktree { func (self *RefreshHelper) refreshWorktrees(env refreshEnv) { worktrees := self.loadWorktrees() - self.onUIThreadUnlessRepoChanged(env, func() error { + self.onUIThreadUnlessRepoChanged(env, func() { self.c.Model().Worktrees = worktrees - return nil }) // need to refresh branches because the branches view shows worktrees against @@ -1385,9 +1417,8 @@ func (self *RefreshHelper) refreshStashEntries(filterPath string, env refreshEnv stashEntries := self.c.Git().Loaders.StashLoader. GetStashEntries(filterPath) - self.onUIThreadUnlessRepoChanged(env, func() error { + self.onUIThreadUnlessRepoChanged(env, func() { self.c.Model().StashEntries = stashEntries - return nil }) self.refreshView(self.c.Contexts().Stash, env) @@ -1398,7 +1429,7 @@ func (self *RefreshHelper) refreshStatus(env refreshEnv) { workingTreeState := self.c.Git().Status.WorkingTreeState() repoName := self.c.Git().RepoPaths.RepoName() - self.onUIThreadUnlessRepoChanged(env, func() error { + self.onUIThreadUnlessRepoChanged(env, func() { // 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 @@ -1406,13 +1437,12 @@ func (self *RefreshHelper) refreshStatus(env refreshEnv) { currentBranch := self.refsHelper.GetCheckedOutRef() if currentBranch == nil { // need to wait for branches to refresh - return nil + return } linkedWorktreeName := self.worktreeHelper.GetLinkedWorktreeName() 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 }) } @@ -1442,7 +1472,7 @@ func (self *RefreshHelper) refreshView(context types.Context, env refreshEnv) { // 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 { + self.onUIThreadUnlessRepoChanged(env, func() { // 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) @@ -1461,16 +1491,14 @@ func (self *RefreshHelper) refreshView(context types.Context, env refreshEnv) { self.searchHelper.ReApplySearch(context) return nil }) - return nil }) } func (self *RefreshHelper) refreshGithubPullRequests(branches []*models.Branch, remotes []*models.Remote, env refreshEnv) { clearPullRequests := func() { - self.onUIThreadUnlessRepoChanged(env, func() error { + self.onUIThreadUnlessRepoChanged(env, func() { self.c.Model().PullRequests = nil self.c.Model().PullRequestsMap = nil - return nil }) } @@ -1623,14 +1651,13 @@ func (self *RefreshHelper) setGithubPullRequests(baseInfo *githubRemoteInfo, bra self.savePullRequestsToCache(prs) - self.onUIThreadUnlessRepoChanged(env, func() error { + self.onUIThreadUnlessRepoChanged(env, func() { 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 5f07b8ea6..675c332a0 100644 --- a/pkg/gui/controllers/helpers/refs_helper.go +++ b/pkg/gui/controllers/helpers/refs_helper.go @@ -56,7 +56,7 @@ func (self *RefsHelper) CheckoutRef(ref string, options types.CheckoutRefOptions scope = append(scope, types.PULL_REQUESTS) } self.c.RefreshFromWorker(types.RefreshOptions{ - Mode: types.BLOCK_UI, + BatchUIUpdates: true, Scope: scope, BranchSelection: types.SelectCheckedOutBranch, CommitSelection: types.SelectHeadCommit, @@ -160,7 +160,6 @@ func (self *RefsHelper) CheckoutRemoteBranch(fullBranchName string, localBranchN // Do a sync refresh to make sure the new branch is visible, // so that we see an inline status when checking it out self.c.Refresh(types.RefreshOptions{ - Mode: types.SYNC, Scope: []types.RefreshableView{types.BRANCHES}, }) return checkout(localBranchName, true) @@ -363,15 +362,21 @@ func (self *RefsHelper) NewBranch(from string, fromFormattedName string, suggest } refresh := func() { - if self.c.Context().Current() != self.c.Contexts().Branches { - self.c.Context().Push(self.c.Contexts().Branches, types.OnFocusOpts{}) - } - - self.c.Refresh(types.RefreshOptions{ - Mode: types.BLOCK_UI, + self.c.RefreshFromWorker(types.RefreshOptions{ + BatchUIUpdates: true, BranchSelection: types.SelectCheckedOutBranch, CommitSelection: types.SelectHeadCommit, SelectTopReflogCommit: true, + Then: func() error { + // Switch to the branches panel only now, in the same batched + // frame that applies the refreshed data, so the panel switch + // and the new branch appear together rather than flashing the + // old branch list while the checkout is still in progress. + if self.c.Context().Current() != self.c.Contexts().Branches { + self.c.Context().Push(self.c.Contexts().Branches, types.OnFocusOpts{}) + } + return nil + }, }) } @@ -385,34 +390,44 @@ func (self *RefsHelper) NewBranch(from string, fromFormattedName string, suggest if newBranchName != suggestedBranchName { newBranchFunc = self.c.Git().Branch.NewWithoutTracking } - if err := newBranchFunc(newBranchName, from); err != nil { - if IsSwitchBranchUncommittedChangesError(err) { - // offer to autostash changes - self.c.Confirm(types.ConfirmOpts{ - Title: self.c.Tr.AutoStashTitle, - Prompt: self.c.Tr.AutoStashPrompt, - HandleConfirm: func() error { - if err := self.c.Git().Stash.Push(fmt.Sprintf(self.c.Tr.AutoStashForNewBranch, newBranchName)); err != nil { - return err - } - if err := newBranchFunc(newBranchName, from); err != nil { - return err - } - err := self.c.Git().Stash.Pop(0) - // Branch switch successful so re-render the UI even if the pop operation failed (e.g. conflict). - refresh() - return err - }, - }) - return nil + // Creating the branch checks it out, which can take a while when + // the ref we're branching off is distant, so do it on a worker. + return self.c.WithWaitingStatus(self.c.Tr.CreatingBranchStatus, func(gocui.Task) error { + if err := newBranchFunc(newBranchName, from); err != nil { + if IsSwitchBranchUncommittedChangesError(err) { + // offer to autostash changes + self.c.OnUIThread(func() error { + self.c.Confirm(types.ConfirmOpts{ + Title: self.c.Tr.AutoStashTitle, + Prompt: self.c.Tr.AutoStashPrompt, + HandleConfirm: func() error { + return self.c.WithWaitingStatus(self.c.Tr.CreatingBranchStatus, func(gocui.Task) error { + if err := self.c.Git().Stash.Push(fmt.Sprintf(self.c.Tr.AutoStashForNewBranch, newBranchName)); err != nil { + return err + } + if err := newBranchFunc(newBranchName, from); err != nil { + return err + } + err := self.c.Git().Stash.Pop(0) + // Branch switch successful so re-render the UI even if the pop operation failed (e.g. conflict). + refresh() + return err + }) + }, + }) + return nil + }) + + return nil + } + + return err } - return err - } - - refresh() - return nil + refresh() + return nil + }) }, }) @@ -534,7 +549,7 @@ func (self *RefsHelper) moveCommitsToNewBranchStackedOnCurrentBranch(newBranchNa } self.c.RefreshFromWorker(types.RefreshOptions{ - Mode: types.BLOCK_UI, + BatchUIUpdates: true, BranchSelection: types.SelectCheckedOutBranch, CommitSelection: types.SelectHeadCommit, SelectTopReflogCommit: true, @@ -558,7 +573,7 @@ func (self *RefsHelper) moveCommitsToNewBranchOffOfMainBranch(newBranchName stri } err := self.c.Git().Rebase.CherryPickCommits(commitsToCherryPick) - err = self.rebaseHelper.CheckMergeOrRebaseWithRefreshOptions(err, types.RefreshOptions{Mode: types.SYNC}) + err = self.rebaseHelper.CheckMergeOrRebaseWithRefreshOptions(err, types.RefreshOptions{}) if err != nil { return err } @@ -570,7 +585,7 @@ func (self *RefsHelper) moveCommitsToNewBranchOffOfMainBranch(newBranchName stri } self.c.RefreshFromWorker(types.RefreshOptions{ - Mode: types.BLOCK_UI, + BatchUIUpdates: true, BranchSelection: types.SelectCheckedOutBranch, CommitSelection: types.SelectHeadCommit, SelectTopReflogCommit: true, diff --git a/pkg/gui/controllers/helpers/working_tree_helper.go b/pkg/gui/controllers/helpers/working_tree_helper.go index 36dfd2032..5f68cb822 100644 --- a/pkg/gui/controllers/helpers/working_tree_helper.go +++ b/pkg/gui/controllers/helpers/working_tree_helper.go @@ -238,7 +238,6 @@ func (self *WorkingTreeHelper) WithEnsureCommittableFiles(handler func() error) return err } self.c.Refresh(types.RefreshOptions{ - Mode: types.SYNC, Scope: []types.RefreshableView{types.FILES}, Then: handler, }) @@ -260,7 +259,7 @@ func (self *WorkingTreeHelper) promptToStageAllAndRetry(retry func() error) erro 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}}) + self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.FILES}}) return retry() }, @@ -360,7 +359,7 @@ func (self *WorkingTreeHelper) CreateMergeConflictMenu(selectedFilepaths []strin } err := self.c.Git().WorkingTree.StageFiles(selectedFilepaths, nil) - self.c.Refresh(types.RefreshOptions{Mode: types.SYNC, Scope: []types.RefreshableView{types.FILES}}) + self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.FILES}}) return err } diff --git a/pkg/gui/controllers/helpers/worktree_helper.go b/pkg/gui/controllers/helpers/worktree_helper.go index 980d810ae..35d515d6e 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.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.WORKTREES, types.BRANCHES, types.FILES}}) + self.c.RefreshFromWorker(types.RefreshOptions{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.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.WORKTREES, types.BRANCHES, types.FILES}}) + self.c.RefreshFromWorker(types.RefreshOptions{Scope: []types.RefreshableView{types.WORKTREES, types.BRANCHES, types.FILES}}) return nil }) } diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 23e94adf7..2da502b79 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -342,7 +342,7 @@ func (self *LocalCommitsController) squashDown(selectedCommits []*models.Commit, HandleConfirm: func() error { commits := self.c.Model().Commits self.selectRebaseResultCommit(startIdx) - return self.c.WithWaitingStatus(self.c.Tr.SquashingStatus, func(gocui.Task) error { + return self.c.WithWaitingStatusBlockingInput(self.c.Tr.SquashingStatus, func(gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.SquashCommitDown) return self.interactiveRebase(commits, todo.Squash, startIdx, endIdx) }) @@ -366,7 +366,7 @@ func (self *LocalCommitsController) fixup(selectedCommits []*models.Commit, star OnPress: func() error { commits := self.c.Model().Commits self.selectRebaseResultCommit(startIdx) - return self.c.WithWaitingStatus(self.c.Tr.FixingStatus, func(gocui.Task) error { + return self.c.WithWaitingStatusBlockingInput(self.c.Tr.FixingStatus, func(gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.FixupCommit) return self.interactiveRebase(commits, todo.Fixup, startIdx, endIdx) }) @@ -379,7 +379,7 @@ func (self *LocalCommitsController) fixup(selectedCommits []*models.Commit, star OnPress: func() error { commits := self.c.Model().Commits self.selectRebaseResultCommit(startIdx) - return self.c.WithWaitingStatus(self.c.Tr.FixingStatus, func(gocui.Task) error { + return self.c.WithWaitingStatusBlockingInput(self.c.Tr.FixingStatus, func(gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.FixupCommitKeepMessage) return self.interactiveRebaseWithFlag(commits, todo.Fixup, startIdx, endIdx, "-C") }) @@ -476,7 +476,7 @@ func (self *LocalCommitsController) switchFromCommitMessagePanelToEditor(filepat return err } - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + self.c.Refresh(types.RefreshOptions{}) return nil } @@ -490,12 +490,12 @@ func (self *LocalCommitsController) handleReword(summary string, description str self.c.Tr.RewordingStatus, nil, nil) } - return self.c.WithWaitingStatus(self.c.Tr.RewordingStatus, func(gocui.Task) error { + return self.c.WithWaitingStatusBlockingInput(self.c.Tr.RewordingStatus, func(gocui.Task) error { err := self.c.Git().Rebase.RewordCommit(commits, selectedIdx, summary, description) if err != nil { return err } - self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC}) + self.c.RefreshFromWorker(types.RefreshOptions{}) return nil }) } @@ -576,7 +576,7 @@ func (self *LocalCommitsController) drop(selectedCommits []*models.Commit, start if !isMerge { self.selectRebaseResultCommit(startIdx) } - return self.c.WithWaitingStatus(self.c.Tr.DroppingStatus, func(gocui.Task) error { + return self.c.WithWaitingStatusBlockingInput(self.c.Tr.DroppingStatus, func(gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.DropCommit) if isMerge { return self.dropMergeCommit(commits, startIdx) @@ -601,10 +601,10 @@ func (self *LocalCommitsController) edit(selectedCommits []*models.Commit, start commits := self.c.Model().Commits if !commits[endIdx].IsMerge() { - return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func(gocui.Task) error { + return self.c.WithWaitingStatusBlockingInput(self.c.Tr.RebasingStatus, func(gocui.Task) error { err := self.c.Git().Rebase.InteractiveRebase(commits, startIdx, endIdx, todo.Edit, "") return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptions( - err, types.RefreshOptions{Mode: types.BLOCK_UI}) + err, types.RefreshOptions{BatchUIUpdates: true}) }) } @@ -623,12 +623,12 @@ func (self *LocalCommitsController) quickStartInteractiveRebase() error { func (self *LocalCommitsController) startInteractiveRebaseWithEdit( commitsToEdit []*models.Commit, ) error { - return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func(gocui.Task) error { + return self.c.WithWaitingStatusBlockingInput(self.c.Tr.RebasingStatus, func(gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.EditCommit) 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() error { + types.RefreshOptions{BatchUIUpdates: true, 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 @@ -700,7 +700,7 @@ func (self *LocalCommitsController) updateTodosWithFlag(action todo.TodoCommand, } self.c.Refresh(types.RefreshOptions{ - Mode: types.SYNC, Scope: []types.RefreshableView{types.REBASE_COMMITS}, + Scope: []types.RefreshableView{types.REBASE_COMMITS}, }) return nil @@ -742,22 +742,30 @@ func (self *LocalCommitsController) moveDown(selectedCommits []*models.Commit, s self.context().HandleFocus(types.OnFocusOpts{ScrollSelectionIntoView: true}) self.c.Refresh(types.RefreshOptions{ - Mode: types.SYNC, Scope: []types.RefreshableView{types.REBASE_COMMITS}, CommitSelection: types.KeepCommitSelectionIndex, }) return nil } - return self.c.WithWaitingStatusSync(self.c.Tr.MovingStatus, func() error { + commits := self.c.Model().Commits + return self.c.WithWaitingStatusBlockingInput(self.c.Tr.MovingStatus, func(gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.MoveCommitDown) - err := self.c.Git().Rebase.MoveCommitsDown(self.c.Model().Commits, startIdx, endIdx) - if err == nil { - self.context().MoveSelection(1) - self.context().HandleFocus(types.OnFocusOpts{ScrollSelectionIntoView: true}) - } - return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptionsFromUIThread( - err, types.RefreshOptions{Mode: types.SYNC, CommitSelection: types.KeepCommitSelectionIndex}) + err := self.c.Git().Rebase.MoveCommitsDown(commits, startIdx, endIdx) + return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptions( + err, types.RefreshOptions{ + BatchUIUpdates: true, + CommitSelection: types.KeepCommitSelectionIndex, + // Move the selection to follow the moved commit, in Then so it + // lands in the same frame as the refreshed commit list. + Then: func() error { + if err == nil { + self.context().MoveSelection(1) + self.context().HandleFocus(types.OnFocusOpts{ScrollSelectionIntoView: true}) + } + return nil + }, + }) }) } @@ -770,22 +778,30 @@ func (self *LocalCommitsController) moveUp(selectedCommits []*models.Commit, sta self.context().HandleFocus(types.OnFocusOpts{ScrollSelectionIntoView: true}) self.c.Refresh(types.RefreshOptions{ - Mode: types.SYNC, Scope: []types.RefreshableView{types.REBASE_COMMITS}, CommitSelection: types.KeepCommitSelectionIndex, }) return nil } - return self.c.WithWaitingStatusSync(self.c.Tr.MovingStatus, func() error { + commits := self.c.Model().Commits + return self.c.WithWaitingStatusBlockingInput(self.c.Tr.MovingStatus, func(gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.MoveCommitUp) - err := self.c.Git().Rebase.MoveCommitsUp(self.c.Model().Commits, startIdx, endIdx) - if err == nil { - self.context().MoveSelection(-1) - self.context().HandleFocus(types.OnFocusOpts{ScrollSelectionIntoView: true}) - } - return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptionsFromUIThread( - err, types.RefreshOptions{Mode: types.SYNC, CommitSelection: types.KeepCommitSelectionIndex}) + err := self.c.Git().Rebase.MoveCommitsUp(commits, startIdx, endIdx) + return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptions( + err, types.RefreshOptions{ + BatchUIUpdates: true, + CommitSelection: types.KeepCommitSelectionIndex, + // Move the selection to follow the moved commit, in Then so it + // lands in the same frame as the refreshed commit list. + Then: func() error { + if err == nil { + self.context().MoveSelection(-1) + self.context().HandleFocus(types.OnFocusOpts{ScrollSelectionIntoView: true}) + } + return nil + }, + }) }) } @@ -798,7 +814,7 @@ func (self *LocalCommitsController) amendTo(commit *models.Commit) error { if err := self.c.Helpers().AmendHelper.AmendHead(); err != nil { return err } - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + self.c.Refresh(types.RefreshOptions{}) return nil }) } @@ -807,7 +823,7 @@ func (self *LocalCommitsController) amendTo(commit *models.Commit) error { 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 { + return self.c.WithWaitingStatusBlockingInput(self.c.Tr.AmendingStatus, func(gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.AmendCommit) err := self.c.Git().Rebase.AmendTo(commits, selectedIdx) return self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(err) @@ -869,13 +885,13 @@ func (self *LocalCommitsController) amendAttribute(_ []*models.Commit, start, en } func (self *LocalCommitsController) resetAuthor(commits []*models.Commit, start, end int) error { - return self.c.WithWaitingStatus(self.c.Tr.AmendingStatus, func(gocui.Task) error { + return self.c.WithWaitingStatusBlockingInput(self.c.Tr.AmendingStatus, func(gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.ResetCommitAuthor) if err := self.c.Git().Rebase.ResetCommitAuthor(commits, start, end); err != nil { return err } - self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC}) + self.c.RefreshFromWorker(types.RefreshOptions{}) return nil }) } @@ -885,13 +901,13 @@ func (self *LocalCommitsController) setAuthor(commits []*models.Commit, start, e 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 { + return self.c.WithWaitingStatusBlockingInput(self.c.Tr.AmendingStatus, func(gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.SetCommitAuthor) if err := self.c.Git().Rebase.SetCommitAuthor(commits, start, end, value); err != nil { return err } - self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC}) + self.c.RefreshFromWorker(types.RefreshOptions{}) return nil }) }, @@ -905,12 +921,12 @@ func (self *LocalCommitsController) addCoAuthor(commits []*models.Commit, start, 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 { + return self.c.WithWaitingStatusBlockingInput(self.c.Tr.AmendingStatus, func(gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.AddCommitCoAuthor) if err := self.c.Git().Rebase.AddCommitCoAuthor(commits, start, end, value); err != nil { return err } - self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC}) + self.c.RefreshFromWorker(types.RefreshOptions{}) return nil }) }, @@ -938,9 +954,8 @@ func (self *LocalCommitsController) revert(commits []*models.Commit, start, end Prompt: promptText, HandleConfirm: func() error { self.c.LogAction(self.c.Tr.Actions.RevertCommit) - return self.c.WithWaitingStatusSync(self.c.Tr.RevertingStatus, func() error { - mustStash := helpers.IsWorkingTreeDirtyExceptSubmodules(self.c.Model().Files, self.c.Model().Submodules) - + mustStash := helpers.IsWorkingTreeDirtyExceptSubmodules(self.c.Model().Files, self.c.Model().Submodules) + return self.c.WithWaitingStatusBlockingInput(self.c.Tr.RevertingStatus, func(gocui.Task) error { if mustStash { if err := self.c.Git().Stash.Push(self.c.Tr.AutoStashForReverting); err != nil { return err @@ -948,7 +963,8 @@ func (self *LocalCommitsController) revert(commits []*models.Commit, start, end } result := self.c.Git().Commit.Revert(hashes, isMerge) - if err := self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptionsFromUIThread(result, types.RefreshOptions{Mode: types.SYNC}); err != nil { + if err := self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptions(result, + types.RefreshOptions{BatchUIUpdates: true}); err != nil { return err } @@ -956,7 +972,7 @@ func (self *LocalCommitsController) revert(commits []*models.Commit, start, end if err := self.c.Git().Stash.Pop(0); err != nil { return err } - self.c.Refresh(types.RefreshOptions{ + self.c.RefreshFromWorker(types.RefreshOptions{ Scope: []types.RefreshableView{types.STASH, types.FILES}, }) } @@ -987,16 +1003,19 @@ func (self *LocalCommitsController) createFixupCommit(commit *models.Commit) err OnPress: func() error { return self.c.Helpers().WorkingTree.WithEnsureCommittableFiles(func() error { self.c.LogAction(self.c.Tr.Actions.CreateFixupCommit) - return self.c.WithWaitingStatusSync(self.c.Tr.CreatingFixupCommitStatus, func() error { + selectedIdx := self.context().GetSelectedLineIdx() + commits := self.c.Model().Commits + branches := self.c.Model().Branches + return self.c.WithWaitingStatusBlockingInput(self.c.Tr.CreatingFixupCommitStatus, func(gocui.Task) error { if err := self.c.Git().Commit.CreateFixupCommit(commit.Hash()); err != nil { return err } - if err := self.moveFixupCommitToOwnerStackedBranch(commit); err != nil { + if err := self.moveFixupCommitToOwnerStackedBranch(commit, selectedIdx, commits, branches); err != nil { return err } - self.c.Refresh(types.RefreshOptions{Mode: types.SYNC}) + self.c.RefreshFromWorker(types.RefreshOptions{BatchUIUpdates: true}) return nil }) }) @@ -1025,7 +1044,12 @@ func (self *LocalCommitsController) createFixupCommit(commit *models.Commit) err }) } -func (self *LocalCommitsController) moveFixupCommitToOwnerStackedBranch(targetCommit *models.Commit) error { +// moveFixupCommitToOwnerStackedBranch takes state captured on the UI thread +// (the selected index and the commits and branches models) so that it can run +// its rebase on a worker without reading the model there. +func (self *LocalCommitsController) moveFixupCommitToOwnerStackedBranch( + targetCommit *models.Commit, selectedIdx int, commits []*models.Commit, branches []*models.Branch, +) error { if self.c.Git().Version.IsOlderThan(2, 38, 0) { // Git 2.38.0 introduced the `rebase.updateRefs` config option. Don't // move the commit down with older versions, as it would break the stack. @@ -1053,9 +1077,9 @@ func (self *LocalCommitsController) moveFixupCommitToOwnerStackedBranch(targetCo } headOfOwnerBranchIdx := -1 - for i := self.context().GetSelectedLineIdx(); i > 0; i-- { - if lo.SomeBy(self.c.Model().Branches, func(b *models.Branch) bool { - return b.CommitHash == self.c.Model().Commits[i].Hash() + for i := selectedIdx; i > 0; i-- { + if lo.SomeBy(branches, func(b *models.Branch) bool { + return b.CommitHash == commits[i].Hash() }) { headOfOwnerBranchIdx = i break @@ -1066,7 +1090,7 @@ func (self *LocalCommitsController) moveFixupCommitToOwnerStackedBranch(targetCo return nil } - return self.c.Git().Rebase.MoveFixupCommitDown(self.c.Model().Commits, headOfOwnerBranchIdx) + return self.c.Git().Rebase.MoveFixupCommitDown(commits, headOfOwnerBranchIdx) } func (self *LocalCommitsController) createAmendCommit(commit *models.Commit, includeFileChanges bool) error { @@ -1087,16 +1111,19 @@ func (self *LocalCommitsController) createAmendCommit(commit *models.Commit, inc PreserveMessage: false, OnConfirm: func(summary string, description string) error { self.c.LogAction(self.c.Tr.Actions.CreateFixupCommit) - return self.c.WithWaitingStatusSync(self.c.Tr.CreatingFixupCommitStatus, func() error { + selectedIdx := self.context().GetSelectedLineIdx() + commits := self.c.Model().Commits + branches := self.c.Model().Branches + return self.c.WithWaitingStatusBlockingInput(self.c.Tr.CreatingFixupCommitStatus, func(gocui.Task) error { if err := self.c.Git().Commit.CreateAmendCommit(originalSubject, summary, description, includeFileChanges); err != nil { return err } - if err := self.moveFixupCommitToOwnerStackedBranch(commit); err != nil { + if err := self.moveFixupCommitToOwnerStackedBranch(commit, selectedIdx, commits, branches); err != nil { return err } - self.c.Refresh(types.RefreshOptions{Mode: types.SYNC}) + self.c.RefreshFromWorker(types.RefreshOptions{BatchUIUpdates: true}) return nil }) }, @@ -1144,12 +1171,28 @@ func (self *LocalCommitsController) squashAllFixupsInCurrentBranch() error { func (self *LocalCommitsController) squashFixupsImpl(commit *models.Commit, rebaseStartIdx int) error { selectionOffset := countSquashableCommitsAbove(self.c.Model().Commits, self.context().GetSelectedLineIdx(), rebaseStartIdx) - return self.c.WithWaitingStatusSync(self.c.Tr.SquashingStatus, func() error { + // The squashed fixups above the selection are removed, so the selection moves + // up by that many rows to stay on the same commit. Compute the target as an + // absolute index now, on the current list. + targetIdx := self.context().GetSelectedLineIdx() - selectionOffset + return self.c.WithWaitingStatusBlockingInput(self.c.Tr.SquashingStatus, func(gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.SquashAllAboveFixupCommits) err := self.c.Git().Rebase.SquashAllAboveFixupCommits(commit) - self.context().MoveSelectedLine(-selectionOffset) - return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptionsFromUIThread( - err, types.RefreshOptions{Mode: types.SYNC}) + return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptions( + err, types.RefreshOptions{ + BatchUIUpdates: true, + // Set the selection in Then so it lands in the same frame as the + // refreshed commit list. It has to be an absolute index: the new + // list is shorter, so a relative move from the (clamped) old index + // could overshoot. PostRefreshUpdate repaints the moved selection. + Then: func() error { + if err == nil { + self.context().SetSelectedLineIdx(targetIdx) + self.c.PostRefreshUpdate(self.context()) + } + return nil + }, + }) }) } @@ -1196,7 +1239,7 @@ func (self *LocalCommitsController) openSearch() error { // we usually lazyload these commits but now that we're searching we need to load them now if self.context().GetLimitCommits() { self.context().SetLimitCommits(false) - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.COMMITS}}) + self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.COMMITS}}) } return self.c.Helpers().Search.OpenSearchPrompt(self.context()) @@ -1217,7 +1260,7 @@ func (self *LocalCommitsController) handleOpenLogMenu() error { return self.c.WithWaitingStatus(self.c.Tr.LoadingCommits, func(gocui.Task) error { self.c.Refresh( - types.RefreshOptions{Mode: types.SYNC, Scope: []types.RefreshableView{types.COMMITS}}, + types.RefreshOptions{Scope: []types.RefreshableView{types.COMMITS}}, ) return nil }) @@ -1271,7 +1314,6 @@ func (self *LocalCommitsController) handleOpenLogMenu() error { return self.c.WithWaitingStatus(self.c.Tr.LoadingCommits, func(gocui.Task) error { self.c.Refresh( types.RefreshOptions{ - Mode: types.SYNC, Scope: []types.RefreshableView{types.COMMITS}, }, ) @@ -1316,7 +1358,7 @@ func (self *LocalCommitsController) GetOnFocus() func(types.OnFocusOpts) { context := self.context() if context.GetSelectedLineIdx() > COMMIT_THRESHOLD && context.GetLimitCommits() { context.SetLimitCommits(false) - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.COMMITS}}) + self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.COMMITS}}) } } } diff --git a/pkg/gui/controllers/merge_conflicts_controller.go b/pkg/gui/controllers/merge_conflicts_controller.go index 898eb356b..1af53fded 100644 --- a/pkg/gui/controllers/merge_conflicts_controller.go +++ b/pkg/gui/controllers/merge_conflicts_controller.go @@ -302,7 +302,7 @@ func (self *MergeConflictsController) resolveConflict(selection mergeconflicts.S func (self *MergeConflictsController) onLastConflictResolved() { // as part of refreshing files, we handle the situation where a file has had // its merge conflicts resolved. - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.FILES}}) + self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.FILES}}) } func (self *MergeConflictsController) openMergeConflictMenu() error { diff --git a/pkg/gui/controllers/patch_building_controller.go b/pkg/gui/controllers/patch_building_controller.go index d596c2ead..f3e26e303 100644 --- a/pkg/gui/controllers/patch_building_controller.go +++ b/pkg/gui/controllers/patch_building_controller.go @@ -223,13 +223,19 @@ func (self *PatchBuildingController) discardSelectionFromCommit() error { return nil } - return self.c.WithWaitingStatusSync(self.c.Tr.RebasingStatus, func() error { - commitIndex := self.getPatchCommitIndex() + commits := self.c.Model().Commits + commitIndex := self.getPatchCommitIndex() + return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func(gocui.Task) 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.CheckMergeOrRebaseWithRefreshOptionsFromUIThread( - err, types.RefreshOptions{Mode: types.SYNC}) + err := self.c.Git().Patch.DeletePatchesFromCommit(commits, commitIndex) + // Escape pops the patch-building context, so run it on the UI thread + // before the refresh below. + _ = self.c.GocuiGui().OnUIThreadAndWait(func() error { + self.c.Helpers().PatchBuilding.Escape() + return nil + }) + return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptions( + err, types.RefreshOptions{}) }) } diff --git a/pkg/gui/controllers/remote_branches_controller.go b/pkg/gui/controllers/remote_branches_controller.go index f70145d7b..0d50068f3 100644 --- a/pkg/gui/controllers/remote_branches_controller.go +++ b/pkg/gui/controllers/remote_branches_controller.go @@ -158,7 +158,7 @@ func (self *RemoteBranchesController) createSortMenu() error { if self.c.UserConfig().Git.RemoteBranchSortOrder != sortOrder { self.c.UserConfig().Git.RemoteBranchSortOrder = sortOrder self.c.Contexts().RemoteBranches.SetSelection(0) - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.REMOTES}}) + self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.REMOTES}}) } return nil }, diff --git a/pkg/gui/controllers/remotes_controller.go b/pkg/gui/controllers/remotes_controller.go index d4c838f7c..76bd16bb1 100644 --- a/pkg/gui/controllers/remotes_controller.go +++ b/pkg/gui/controllers/remotes_controller.go @@ -163,7 +163,6 @@ func (self *RemotesController) addAndCheckoutRemote(remoteName string, remoteUrl // 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 { @@ -371,7 +370,6 @@ func (self *RemotesController) fetchAndCheckout(remote *models.Remote, branchNam } refreshOptions := types.RefreshOptions{ Scope: []types.RefreshableView{types.BRANCHES, types.REMOTES}, - Mode: types.SYNC, } if branchName != "" { err = self.c.Git().Branch.New(branchName, remote.Name+"/"+branchName) diff --git a/pkg/gui/controllers/sub_commits_controller.go b/pkg/gui/controllers/sub_commits_controller.go index 8799cd3c6..d3d0c0b98 100644 --- a/pkg/gui/controllers/sub_commits_controller.go +++ b/pkg/gui/controllers/sub_commits_controller.go @@ -66,7 +66,7 @@ func (self *SubCommitsController) GetOnFocus() func(types.OnFocusOpts) { context := self.context() if context.GetSelectedLineIdx() > COMMIT_THRESHOLD && context.GetLimitCommits() { context.SetLimitCommits(false) - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.SUB_COMMITS}}) + self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.SUB_COMMITS}}) } } } diff --git a/pkg/gui/controllers/sync_controller.go b/pkg/gui/controllers/sync_controller.go index fafd4e7dd..61b92747b 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.RefreshFromWorker(types.RefreshOptions{Mode: types.SYNC}) + self.c.RefreshFromWorker(types.RefreshOptions{}) return nil }) } diff --git a/pkg/gui/controllers/tags_controller.go b/pkg/gui/controllers/tags_controller.go index 2a59af5ec..a6c0e7e14 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.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.COMMITS, types.TAGS}}) + self.c.RefreshFromWorker(types.RefreshOptions{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.RefreshFromWorker(types.RefreshOptions{Mode: types.SYNC, Scope: []types.RefreshableView{types.COMMITS, types.TAGS}}) + self.c.RefreshFromWorker(types.RefreshOptions{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.RefreshFromWorker(types.RefreshOptions{Mode: types.SYNC, Scope: []types.RefreshableView{types.COMMITS, types.TAGS}}) + self.c.RefreshFromWorker(types.RefreshOptions{Scope: []types.RefreshableView{types.COMMITS, types.TAGS}}) return nil }) }, diff --git a/pkg/gui/controllers/workspace_reset_controller.go b/pkg/gui/controllers/workspace_reset_controller.go index 9a9005254..27e736648 100644 --- a/pkg/gui/controllers/workspace_reset_controller.go +++ b/pkg/gui/controllers/workspace_reset_controller.go @@ -46,7 +46,7 @@ func (self *FilesController) createResetMenu() error { } self.c.Refresh( - types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.FILES}}, + types.RefreshOptions{Scope: []types.RefreshableView{types.FILES}}, ) return nil }, @@ -68,7 +68,7 @@ func (self *FilesController) createResetMenu() error { } self.c.Refresh( - types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.FILES}}, + types.RefreshOptions{Scope: []types.RefreshableView{types.FILES}}, ) return nil }, @@ -86,7 +86,7 @@ func (self *FilesController) createResetMenu() error { } self.c.Refresh( - types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.FILES}}, + types.RefreshOptions{Scope: []types.RefreshableView{types.FILES}}, ) return nil }, @@ -111,7 +111,7 @@ func (self *FilesController) createResetMenu() error { } self.c.Refresh( - types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.FILES}}, + types.RefreshOptions{Scope: []types.RefreshableView{types.FILES}}, ) return nil }, @@ -129,7 +129,7 @@ func (self *FilesController) createResetMenu() error { } self.c.Refresh( - types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.FILES}}, + types.RefreshOptions{Scope: []types.RefreshableView{types.FILES}}, ) return nil }, @@ -147,7 +147,7 @@ func (self *FilesController) createResetMenu() error { } self.c.Refresh( - types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.FILES}}, + types.RefreshOptions{Scope: []types.RefreshableView{types.FILES}}, ) return nil }, @@ -170,7 +170,7 @@ func (self *FilesController) createResetMenu() error { } self.c.Refresh( - types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.FILES}}, + types.RefreshOptions{Scope: []types.RefreshableView{types.FILES}}, ) return nil }, diff --git a/pkg/gui/gui.go b/pkg/gui/gui.go index 931b0909e..ce70cb2d5 100644 --- a/pkg/gui/gui.go +++ b/pkg/gui/gui.go @@ -391,7 +391,7 @@ func (gui *Gui) onNewRepo(startArgs appTypes.StartArgs, contextKey types.Context } gui.c.Log.Info("Receiving focus - refreshing") - gui.helpers.Refresh.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + gui.helpers.Refresh.Refresh(types.RefreshOptions{}) return reloadErr } @@ -815,13 +815,13 @@ func NewGui( func(ctx goContext.Context, opts types.CreatePopupPanelOpts) { gui.helpers.Confirmation.CreatePopupPanel(ctx, opts) }, - func() error { gui.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}); return nil }, + func() error { gui.c.Refresh(types.RefreshOptions{}); return nil }, func() { gui.State.ContextMgr.Pop() }, func() types.Context { return gui.State.ContextMgr.Current() }, gui.createMenu, func(message string, f func(gocui.Task) error) { gui.helpers.AppStatus.WithWaitingStatus(message, f) }, - func(message string, f func() error) error { - return gui.helpers.AppStatus.WithWaitingStatusSync(message, f) + func(message string, f func(gocui.Task) error) { + gui.helpers.AppStatus.WithWaitingStatusBlockingInput(message, f) }, func(message string, kind types.ToastKind) { gui.helpers.AppStatus.Toast(message, kind) }, func() string { return gui.Views.Prompt.TextArea.GetContent() }, @@ -1023,7 +1023,7 @@ func (gui *Gui) runSubprocessWithSuspenseAndRefresh(subprocess *oscommands.CmdOb return err } - gui.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + gui.c.Refresh(types.RefreshOptions{}) return nil } @@ -1100,7 +1100,7 @@ func (gui *Gui) loadNewRepo() error { return err } - gui.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + gui.c.Refresh(types.RefreshOptions{}) if err := gui.os.UpdateWindowTitle(); err != nil { return err diff --git a/pkg/gui/popup/popup_handler.go b/pkg/gui/popup/popup_handler.go index ab067410d..7c15c56ea 100644 --- a/pkg/gui/popup/popup_handler.go +++ b/pkg/gui/popup/popup_handler.go @@ -13,16 +13,16 @@ import ( type PopupHandler struct { *common.Common - createPopupPanelFn func(context.Context, types.CreatePopupPanelOpts) - onErrorFn func() error - popContextFn func() - currentContextFn func() types.Context - createMenuFn func(types.CreateMenuOptions) error - withWaitingStatusFn func(message string, f func(gocui.Task) error) - withWaitingStatusSyncFn func(message string, f func() error) error - toastFn func(message string, kind types.ToastKind) - getPromptInputFn func() string - inDemo func() bool + createPopupPanelFn func(context.Context, types.CreatePopupPanelOpts) + onErrorFn func() error + popContextFn func() + currentContextFn func() types.Context + createMenuFn func(types.CreateMenuOptions) error + withWaitingStatusFn func(message string, f func(gocui.Task) error) + withWaitingStatusBlockingInputFn func(message string, f func(gocui.Task) error) + toastFn func(message string, kind types.ToastKind) + getPromptInputFn func() string + inDemo func() bool } var _ types.IPopupHandler = &PopupHandler{} @@ -35,23 +35,23 @@ func NewPopupHandler( currentContextFn func() types.Context, createMenuFn func(types.CreateMenuOptions) error, withWaitingStatusFn func(message string, f func(gocui.Task) error), - withWaitingStatusSyncFn func(message string, f func() error) error, + withWaitingStatusBlockingInputFn func(message string, f func(gocui.Task) error), toastFn func(message string, kind types.ToastKind), getPromptInputFn func() string, inDemo func() bool, ) *PopupHandler { return &PopupHandler{ - Common: common, - createPopupPanelFn: createPopupPanelFn, - onErrorFn: onErrorFn, - popContextFn: popContextFn, - currentContextFn: currentContextFn, - createMenuFn: createMenuFn, - withWaitingStatusFn: withWaitingStatusFn, - withWaitingStatusSyncFn: withWaitingStatusSyncFn, - toastFn: toastFn, - getPromptInputFn: getPromptInputFn, - inDemo: inDemo, + Common: common, + createPopupPanelFn: createPopupPanelFn, + onErrorFn: onErrorFn, + popContextFn: popContextFn, + currentContextFn: currentContextFn, + createMenuFn: createMenuFn, + withWaitingStatusFn: withWaitingStatusFn, + withWaitingStatusBlockingInputFn: withWaitingStatusBlockingInputFn, + toastFn: toastFn, + getPromptInputFn: getPromptInputFn, + inDemo: inDemo, } } @@ -76,8 +76,9 @@ func (self *PopupHandler) WithWaitingStatus(message string, f func(gocui.Task) e return nil } -func (self *PopupHandler) WithWaitingStatusSync(message string, f func() error) error { - return self.withWaitingStatusSyncFn(message, f) +func (self *PopupHandler) WithWaitingStatusBlockingInput(message string, f func(gocui.Task) error) error { + self.withWaitingStatusBlockingInputFn(message, f) + return nil } func (self *PopupHandler) ErrorHandler(err error) error { diff --git a/pkg/gui/services/custom_commands/handler_creator.go b/pkg/gui/services/custom_commands/handler_creator.go index 4eb762019..6046d974a 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.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC}) + self.c.RefreshFromWorker(types.RefreshOptions{}) if err != nil { if customCommand.After != nil && customCommand.After.CheckForConflicts { diff --git a/pkg/gui/types/common.go b/pkg/gui/types/common.go index 08ff53bb0..5a256b434 100644 --- a/pkg/gui/types/common.go +++ b/pkg/gui/types/common.go @@ -161,7 +161,7 @@ type IPopupHandler interface { // Shows a popup prompting the user for input. Prompt(opts PromptOpts) WithWaitingStatus(message string, f func(gocui.Task) error) error - WithWaitingStatusSync(message string, f func() error) error + WithWaitingStatusBlockingInput(message string, f func(gocui.Task) error) error Menu(opts CreateMenuOptions) error Toast(message string) ErrorToast(message string) diff --git a/pkg/gui/types/refresh.go b/pkg/gui/types/refresh.go index f4041bb2e..937c3a30e 100644 --- a/pkg/gui/types/refresh.go +++ b/pkg/gui/types/refresh.go @@ -25,14 +25,6 @@ const ( PULL_REQUESTS ) -type RefreshMode int - -const ( - SYNC RefreshMode = iota // wait until everything is done before returning - ASYNC // return immediately, allowing each independent thing to update itself - BLOCK_UI // wrap code in an update call to ensure UI updates all at once and keybindings aren't executed till complete -) - // CommitSelectionBehavior controls which local commit is selected after the // commits list is reloaded by a refresh. type CommitSelectionBehavior int @@ -74,7 +66,11 @@ const ( type RefreshOptions struct { 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 + + // If true, hold off on updating the UI until all scopes have finished + // refreshing and then apply them together in a single frame, rather than + // letting each scope update the UI as soon as it's done. + BatchUIUpdates bool // Controls which local branch is selected after the refresh. Defaults to // KeepBranchSelectionByName. diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index 22b05e6c0..2e83fed9b 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -427,6 +427,7 @@ type TranslationSet struct { UndoingStatus string RedoingStatus string CheckingOutStatus string + CreatingBranchStatus string CommittingStatus string RewordingStatus string RevertingStatus string @@ -1576,6 +1577,7 @@ func EnglishTranslationSet() *TranslationSet { UndoingStatus: "Undoing", RedoingStatus: "Redoing", CheckingOutStatus: "Checking out", + CreatingBranchStatus: "Creating branch", CommittingStatus: "Committing", RewordingStatus: "Rewording", RevertingStatus: "Reverting",