mirror of
https://github.com/jesseduffield/lazygit.git
synced 2026-08-24 10:13:41 -05:00
Add RefreshBlockingInput to buffer keypresses until a refresh has landed
A refresh from the UI thread returns immediately and applies its model and view updates as queued UI-thread callbacks. A key pressed before those have run is handled against the stale, pre-refresh state. For most keys that's harmless, but some handlers turn that state into git commands: pressing space twice in quick succession in the staging panel builds the second patch from the already-applied diff and fails with 'patch does not apply', because the refresh after the first press is what moves the selection to the next stageable hunk. Notably, this is not just a regression of the recent change that made UI-thread refreshes non-blocking; the window was merely much narrower before. A blocking refresh parked the UI thread while the scopes' bounces were queued, and the event loop drains pending keyboard input with priority over queued user events, so a key pressed during the blocked window still beat the queued state updates. The guarantee that the next keypress sees post-refresh state had already ended when the scopes' state updates moved from worker-side mutex-guarded writes to UI-thread bounces. Fix it with the input-blocking mechanism we already use for commit surgery, exposed as a new RefreshBlockingInput entry point: it begins blocking events synchronously in the calling handler, and ends the block from a callback that the finishing step queues behind the refresh's own updates. Keys pressed while the refresh is in flight are buffered and replayed, in order, against the fully refreshed state; since a replayed key's handler re-enters this same path, a burst of keypresses applies sequentially, each one seeing the previous one's refresh. Unlike the old blocking refreshes, this doesn't freeze the UI thread: rendering, spinners, resizing, and mouse scrolling keep working while input is withheld. Blocking input is opt-in per call site rather than the default for all UI-thread refreshes, because most refreshes (the focus-in and startup refreshes, say) don't produce state that the next keypress depends on, and blocking on them would delay typing for no reason. It should also be limited to quick, narrow-scoped refreshes: a full refresh, or any scope that pulls in COMMITS, can take very long in large repos and should usually not hold up input. The staging panel's stage/discard/edit-hunk refreshes use it now.
This commit is contained in:
@@ -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))
|
||||
}
|
||||
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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"),
|
||||
)
|
||||
},
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user