Replay withheld keys on a later pass of the event loop

EndBlockingEvents dispatches the keys buffered during a block from inside its
own call, so a keybinding handler runs in the middle of whatever the caller was
doing. If a caller ends the block partway through updating the screen, that
handler acts on state the caller has yet to finish writing.

No caller does that today; both of them end their block from a UI-thread
callback of their own, with nothing left to do afterwards. The commit after this
one adds a caller that can. It ends the block from a render restore, and a
render of the two main panes resolves that restore halfway through laying the
panes out.

Queue the replay through Update instead, so the buffered keys arrive on a later
pass of the loop, as they would have if the user had pressed them then. Input
stays withheld until that pass runs. Gui events are dispatched in preference to
queued work, so a key pressed in between would otherwise be handled ahead of the
keys buffered before it.

The replay's error now reaches the error handler along with every other
handler's, and EndBlockingEvents has nothing left to return.

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)
 }

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 {
@@ -930,12 +933,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 {
@@ -1206,7 +1230,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)
@@ -1235,6 +1259,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).

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()
 }

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
 			})
 		}