diff --git a/pkg/gocui/block_events_test.go b/pkg/gocui/block_events_test.go index 277bac89a..e95b834ff 100644 --- a/pkg/gocui/block_events_test.go +++ b/pkg/gocui/block_events_test.go @@ -50,6 +50,22 @@ func setupKeyRecorder(t *testing.T, g *Gui) (GocuiEvent, *[]int) { return GocuiEvent{Type: eventKey, Key: key}, &fired } +// runQueuedWork runs what the gui has queued for the following passes of the +// event loop, which is where EndBlockingEvents leaves the replay of the keys it +// buffered. +func runQueuedWork(t *testing.T, g *Gui) { + t.Helper() + + for { + ev, ok := g.userEvents.dequeue() + if !ok { + return + } + assert.NoError(t, ev.f(g)) + ev.task.Done() + } +} + func TestBlockingEvents_KeysBufferedAndReplayed(t *testing.T) { g := newTestGui(t) keyEvent, fired := setupKeyRecorder(t, g) @@ -64,12 +80,37 @@ func TestBlockingEvents_KeysBufferedAndReplayed(t *testing.T) { 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()) + // Unblocking queues the replay rather than dispatching from here. + g.EndBlockingEvents() + assert.Len(t, *fired, 1, "the replay must wait for the event loop") + + runQueuedWork(t, g) assert.Len(t, *fired, 3, "both buffered keys should replay on unblock") assert.Empty(t, g.bufferedKeyEvents) } +func TestBlockingEvents_KeysArrivingBeforeTheReplayGoBehindIt(t *testing.T) { + g := newTestGui(t) + keyEvent, fired := setupKeyRecorder(t, g) + other := GocuiEvent{Type: eventKey, Key: NewKeyRune('y')} + g.SetKeybinding("main", other.Key, func(*Gui, *View) error { + *fired = append(*fired, 0) + return nil + }) + + g.BeginBlockingEvents() + assert.NoError(t, g.handleEvent(&keyEvent)) + g.EndBlockingEvents() + + // A key pressed while the replay is still queued joins the end of the buffer: + // dispatching it now would put it ahead of the keys buffered before it. + assert.NoError(t, g.handleEvent(&other)) + assert.Empty(t, *fired) + + runQueuedWork(t, g) + assert.Equal(t, []int{1, 0}, *fired, "the keys should arrive in the order they were pressed") +} + func TestBlockingEvents_NestsWithCounter(t *testing.T) { g := newTestGui(t) keyEvent, fired := setupKeyRecorder(t, g) @@ -79,11 +120,13 @@ func TestBlockingEvents_NestsWithCounter(t *testing.T) { assert.NoError(t, g.handleEvent(&keyEvent)) // The inner block ending still leaves us blocked: no replay yet. - assert.NoError(t, g.EndBlockingEvents()) + g.EndBlockingEvents() + runQueuedWork(t, g) assert.Empty(t, *fired) // Only the outermost block ending replays. - assert.NoError(t, g.EndBlockingEvents()) + g.EndBlockingEvents() + runQueuedWork(t, g) assert.Len(t, *fired, 1) } @@ -94,5 +137,6 @@ func TestBlockingEvents_MouseClicksDroppedNotBuffered(t *testing.T) { 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()) + g.EndBlockingEvents() + runQueuedWork(t, g) } diff --git a/pkg/gocui/gui.go b/pkg/gocui/gui.go index 5af5a7cca..144fae11f 100644 --- a/pkg/gocui/gui.go +++ b/pkg/gocui/gui.go @@ -236,10 +236,13 @@ type Gui struct { // 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. + // dropped outright. It's a counter so blocking can nest. replayPending says + // that the replay is queued but hasn't run yet, and input is withheld until + // it has. All three fields are only touched on the UI thread. See + // BeginBlockingEvents. blockInputCount int bufferedKeyEvents []GocuiEvent + replayPending bool } type NewGuiOpts struct { @@ -936,12 +939,33 @@ func (g *Gui) BeginBlockingEvents() { // 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 { +// +// The replay is queued rather than run here, so that the buffered keys arrive on +// a later pass of the event loop, as they would have if the user had pressed them +// then. Running them here dispatches them from the middle of whatever the caller +// was doing. If a caller ends the block partway through updating the screen, a +// handler then acts on state the caller has yet to finish writing. +func (g *Gui) EndBlockingEvents() { g.blockInputCount-- if g.blockInputCount > 0 { - return nil + return } + // Input stays withheld until the replay has run. Gui events are dispatched in + // preference to queued work (see processRemainingEvents), so a key pressed + // before the replay gets its turn would otherwise be handled ahead of the keys + // buffered before it. + g.replayPending = true + g.Update(func(*Gui) error { return g.replayBufferedKeys() }) +} + +// replayBufferedKeys dispatches the keys withheld while input was blocked, and +// lets input through again. One of their handlers may block input afresh, and +// then the keys after it are withheld in their turn, to be replayed when that +// block ends. +func (g *Gui) replayBufferedKeys() error { + g.replayPending = false + buffered := g.bufferedKeyEvents g.bufferedKeyEvents = nil for i := range buffered { @@ -1212,7 +1236,7 @@ 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 g.withholdingInput() && eventWithheldWhileBlocking(ev) { if ev.Type == eventKey { // Buffer keys so they replay against fresh state on unblock. g.bufferedKeyEvents = append(g.bufferedKeyEvents, *ev) @@ -1241,6 +1265,13 @@ func (g *Gui) handleEvent(ev *GocuiEvent) error { } } +// withholdingInput reports whether events are being kept from the handlers. They +// are while a block is in force, and on until the keys it buffered have been +// replayed. +func (g *Gui) withholdingInput() bool { + return g.blockInputCount > 0 || g.replayPending +} + // 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). diff --git a/pkg/gui/controllers/helpers/app_status_helper.go b/pkg/gui/controllers/helpers/app_status_helper.go index 44de70546..20e5824e9 100644 --- a/pkg/gui/controllers/helpers/app_status_helper.go +++ b/pkg/gui/controllers/helpers/app_status_helper.go @@ -99,21 +99,27 @@ func (self *AppStatusHelper) WithWaitingStatusBlockingInput(opts types.WaitingSt self.modeHelper.SetSuppressWorkingTreeStateMode(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 working tree state shows correctly. - defer self.c.OnUIThread(func() error { - if opts.HideWorkingTreeState { - self.modeHelper.SetSuppressWorkingTreeStateMode(false) - } - return self.c.GocuiGui().EndBlockingEvents() - }) + defer self.endBlockingInput(opts.HideWorkingTreeState) return self.WithWaitingStatusImpl(opts.Message, f, task) }) } +// endBlockingInput lets input through again once the operation and its refresh +// have applied their UI updates, and restores the mode indicator with it. +// OnUIThread queues this after the refresh's model bounces and Then (which +// RefreshFromWorker has already enqueued by the time the operation returns), so +// the replayed keys act on the refreshed state and any resulting working tree +// state shows correctly. +func (self *AppStatusHelper) endBlockingInput(hideWorkingTreeState bool) { + self.c.OnUIThread(func() error { + if hideWorkingTreeState { + self.modeHelper.SetSuppressWorkingTreeStateMode(false) + } + self.c.GocuiGui().EndBlockingEvents() + return nil + }) +} + func (self *AppStatusHelper) HasStatus() bool { return self.statusMgr().HasStatus() } diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index 6c96d79c6..92b0b0e84 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -589,7 +589,8 @@ func (self *RefreshHelper) performRefresh(options types.RefreshOptions, calledFr // 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.GocuiGui().EndBlockingEvents() + return nil }) }