diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index a85c58cf2..cf097b67e 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -81,14 +81,20 @@ func NewRefreshHelper( } func (self *RefreshHelper) Refresh(options types.RefreshOptions) { - self.performRefresh(options, false) + self.performRefresh(options, false, false) +} + +// RefreshBlockingInput is Refresh for handlers whose next keypress may depend +// on the state the refresh produces. See IGuiCommon.RefreshBlockingInput. +func (self *RefreshHelper) RefreshBlockingInput(options types.RefreshOptions) { + self.performRefresh(options, false, true) } // RefreshFromWorker is Refresh for callers already running on a worker // goroutine (e.g. inside a WithWaitingStatus handler) rather than the UI // thread. See IGuiCommon.RefreshFromWorker. func (self *RefreshHelper) RefreshFromWorker(options types.RefreshOptions) { - self.performRefresh(options, true) + self.performRefresh(options, true, false) } type refreshEnv struct { @@ -159,7 +165,7 @@ func (self *refreshBounceBatch) close() []func() { return self.funcs } -func (self *RefreshHelper) performRefresh(options types.RefreshOptions, calledFromWorker bool) { +func (self *RefreshHelper) performRefresh(options types.RefreshOptions, calledFromWorker bool, blockInput bool) { startTime := time.Now() // A refresh from a worker blocks that worker until it's done; one from the @@ -192,6 +198,17 @@ func (self *RefreshHelper) performRefresh(options types.RefreshOptions, calledFr panic("a refresh with a Then callback must not set DontBlockRepoSwitch") } + // A RefreshBlockingInput caller wants keyboard input withheld until the + // refreshed state is in place (see IGuiCommon.RefreshBlockingInput). Begin + // the block synchronously here in the calling handler, so that no keypress + // can slip through before it; the finishing step ends it from a callback + // queued behind the refresh's own updates (see waitAndFinalize). Demos + // take the blocking inline path below and need none of this. + blockInputUntilDone := blockInput && !self.c.InDemo() + if blockInputUntilDone { + self.c.GocuiGui().BeginBlockingEvents() + } + // Capture the refresh's baseline once, here at the start: the repo // generation that every scope's bounce is guarded against, and the git // command instance the scopes run their commands through. The two are @@ -498,6 +515,15 @@ func (self *RefreshHelper) performRefresh(options types.RefreshOptions, calledFr self.onUIThread(env.background, options.Then) } + if blockInputUntilDone { + // Queued after the scopes' model bounces and Then, so by the time + // this runs — and the keys buffered during the refresh replay — + // the refreshed state is in place. + self.c.OnUIThread(func() error { + return self.c.GocuiGui().EndBlockingEvents() + }) + } + self.c.Log.Infof("Refresh took %s", time.Since(startTime)) } diff --git a/pkg/gui/controllers/staging_controller.go b/pkg/gui/controllers/staging_controller.go index 8d876acda..505a07fc4 100644 --- a/pkg/gui/controllers/staging_controller.go +++ b/pkg/gui/controllers/staging_controller.go @@ -229,7 +229,10 @@ func (self *StagingController) applySelectionAndRefresh(reverse bool) error { return err } - self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.FILES, types.STAGING}}) + // Block input until the refresh has landed: it rebuilds the staging panel + // and moves the selection to the next stageable change, and a quick second + // keypress must act on that, not on the stale pre-refresh diff. + self.c.RefreshBlockingInput(types.RefreshOptions{Scope: []types.RefreshableView{types.FILES, types.STAGING}}) return nil } @@ -284,7 +287,9 @@ func (self *StagingController) EditHunkAndRefresh() error { return err } - self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.FILES, types.STAGING}}) + // Block input like applySelectionAndRefresh does; the refresh rebuilds the + // staging panel from the post-edit diff. + self.c.RefreshBlockingInput(types.RefreshOptions{Scope: []types.RefreshableView{types.FILES, types.STAGING}}) return nil } diff --git a/pkg/gui/gui_common.go b/pkg/gui/gui_common.go index 80b2b9ded..e7b14ba04 100644 --- a/pkg/gui/gui_common.go +++ b/pkg/gui/gui_common.go @@ -30,6 +30,10 @@ func (self *guiCommon) Refresh(opts types.RefreshOptions) { self.gui.helpers.Refresh.Refresh(opts) } +func (self *guiCommon) RefreshBlockingInput(opts types.RefreshOptions) { + self.gui.helpers.Refresh.RefreshBlockingInput(opts) +} + func (self *guiCommon) RefreshFromWorker(opts types.RefreshOptions) { self.gui.helpers.Refresh.RefreshFromWorker(opts) } diff --git a/pkg/gui/types/common.go b/pkg/gui/types/common.go index 6d11e29db..87bd9ef50 100644 --- a/pkg/gui/types/common.go +++ b/pkg/gui/types/common.go @@ -29,6 +29,17 @@ type IGuiCommon interface { LogCommand(cmdStr string, isCommandLine bool) // we call this when we want to refetch some models and render the result. Internally calls PostRefreshUpdate Refresh(RefreshOptions) + // Like Refresh, but withholds keyboard input until the refreshed state is + // in place: keys pressed while the refresh is in flight are buffered and + // replayed once its model and view updates have run, instead of being + // handled against the stale, pre-refresh state. Use it when the very next + // keypress may depend on what the refresh produces — e.g. staging a hunk, + // where the refresh moves the selection to the next stageable hunk that + // the next press is meant to stage. Keep it to quick, narrow-scoped + // refreshes: one that includes COMMITS (or refreshes everything) can take + // very long in large repos and should usually not block input unless + // there's a very good reason (switching repos is one such example). + RefreshBlockingInput(RefreshOptions) // Like Refresh, but for callers running on a worker goroutine (e.g. inside // a WithWaitingStatus handler) rather than the UI thread. The refresh // captures the model/context state it needs on the UI thread before doing diff --git a/pkg/integration/tests/staging/stage_hunks_with_rapid_keypresses.go b/pkg/integration/tests/staging/stage_hunks_with_rapid_keypresses.go index 74dbb4b6f..5b41073e2 100644 --- a/pkg/integration/tests/staging/stage_hunks_with_rapid_keypresses.go +++ b/pkg/integration/tests/staging/stage_hunks_with_rapid_keypresses.go @@ -36,7 +36,6 @@ var StageHunksWithRapidKeypresses = NewIntegrationTest(NewIntegrationTestArgs{ IsFocused(). PressRapidly(keys.Universal.Select, keys.Universal.Select) - /* EXPECTED: t.Views().StagingSecondary(). IsFocused(). ContainsLines( @@ -47,23 +46,5 @@ var StageHunksWithRapidKeypresses = NewIntegrationTest(NewIntegrationTestArgs{ Contains("+3b"), Contains("+4b"), ) - ACTUAL: */ - t.ExpectPopup().Alert(). - Title(Equals("Error")). - Content(Contains("patch does not apply")). - Confirm() - - t.Views().Staging(). - IsFocused(). - ContainsLines( - Contains("+3b"), - Contains("+4b"), - ) - - t.Views().StagingSecondary(). - ContainsLines( - Contains("+1b"), - Contains("+2b"), - ) }, })