Show what was asked for last in the main view

If a diff and then a message are asked for in the main view before the
next layout, the main view shows the diff. One way this happens is
switching tabs twice in rapid succession. Another is discarding the
changes of the only changed file. Closing the menu asks for the file's
diff, and if the files are refreshed before the layout, "No changed
files" is asked for next. The main view then stays empty, because by
the time the diff runs, the file has no changes. This made the
hide_selection_when_changes_vanish test fail now and then.

A diff's task is only created after the layout, since the layout settles
the width that the diff is laid out to. A diff renderer's task has been
created there since 8b8343b8a9, and the task for git's own diff since
0afb94e97b. A view shows the task that was created last, so the diff's
task replaced the message's.

Reserve the diff's place among the view's tasks when the diff is asked
for, and give its task that place when it is created after the layout.
If another task has been asked for in the meantime, don't create the
diff's task at all.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

pkg/gui/main_view_render.go

@@ -56,11 +56,16 @@ func (gui *Gui) newRenderTask(view *gocui.View, cmd *exec.Cmd, prefix string) er
 	// it before anything else can touch the command's arguments.
 	cmdStr := strings.Join(cmd.Args, " ")
 
+	manager := gui.getManager(view)
+	// The task takes its place among the view's tasks now, although it is only
+	// created after the layout (see the matching call in newCmdTask).
+	reservation := manager.ReserveTask()
+
 	// Mark the view as loading synchronously now, before the layout pass: the
 	// actual task is created in afterLayout (below), which runs after layout, so
 	// without this the next layout pass would clamp the scroll position to the
 	// not-yet-loaded content.
-	gui.getManager(view).StartLoading()
+	manager.StartLoading()
 	// Hold the scrollbar at its current height while the re-render loads, so the
 	// thumb doesn't shrink and snap back when the first partial paint swaps in
 	// (see the matching call in newCmdTask).
@@ -68,6 +73,10 @@ func (gui *Gui) newRenderTask(view *gocui.View, cmd *exec.Cmd, prefix string) er
 
 	// Run the render after layout so that it gets the correct size
 	gui.afterLayout(func() error {
+		if manager.IsSuperseded(reservation) {
+			return nil
+		}
+
 		// The layout may have changed the size of the view, so only now is the
 		// width to render at known, and with it the renderer command.
 		width := gui.renderWidth(view)
@@ -113,7 +122,7 @@ func (gui *Gui) newRenderTask(view *gocui.View, cmd *exec.Cmd, prefix string) er
 		if rendersThroughAPipe() {
 			run = gui.pipedRender
 		}
-		return gui.newTaskForRender(spec, prefix, cmdStr, run)
+		return gui.newTaskForRender(reservation, spec, prefix, cmdStr, run)
 	})
 
 	return nil
@@ -144,16 +153,17 @@ type (
 type runRender func(spec renderSpec) (startRender, onCloseRender)
 
 // newTaskForRender creates the task that reads the render's output into its
-// view, running the command the given way. key names what is rendered, so that
+// view, running the command the given way. The task takes the place that the
+// reservation holds among the view's tasks. key names what is rendered, so that
 // a re-render of the same content can be told from a render of other content.
-func (gui *Gui) newTaskForRender(spec renderSpec, prefix string, key string, run runRender) error {
+func (gui *Gui) newTaskForRender(reservation tasks.TaskReservation, spec renderSpec, prefix string, key string, run runRender) error {
 	setColumnsEnvVar(spec.cmd, spec.width)
 
 	start, onClose := run(spec)
 
 	manager := gui.getManager(spec.view)
 	linesToRead := gui.linesToReadFromCmdTask(spec.view)
-	return manager.NewTask(manager.NewCmdTask(start, prefix, linesToRead, onClose), key)
+	return manager.NewReservedTask(reservation, manager.NewCmdTask(start, prefix, linesToRead, onClose), key)
 }
 
 // renderWithoutPtyEnvVar makes a render take the piped path on a platform that

pkg/gui/tasks_adapter.go

@@ -18,10 +18,16 @@ func (gui *Gui) newCmdTask(view *gocui.View, cmd *exec.Cmd, prefix string) error
 		cmdStr,
 	).Debug("RunCommand")
 
+	manager := gui.getManager(view)
+	// The task is only created after the layout (see below), but it has to take
+	// its place among the view's tasks now. Otherwise a task asked for after this
+	// one, but created before the layout, would be replaced by it.
+	reservation := manager.ReserveTask()
+
 	// Mark the view as loading synchronously (before the task's goroutine runs
 	// and before the next layout pass) so the layout doesn't clamp the scroll
 	// position to the not-yet-loaded content.
-	gui.getManager(view).StartLoading()
+	manager.StartLoading()
 	// Hold the scrollbar at the height the view has now (the previous render),
 	// while it still shows that render: once the re-render swaps in its first
 	// partial paint the displayed buffer is briefly short, and we don't want the
@@ -34,8 +40,12 @@ func (gui *Gui) newCmdTask(view *gocui.View, cmd *exec.Cmd, prefix string) error
 	// keeps the task goroutine from reading the view's live dimensions while it
 	// streams output.
 	gui.afterLayout(func() error {
+		if manager.IsSuperseded(reservation) {
+			return nil
+		}
+
 		spec := renderSpec{view: view, cmd: cmd, width: gui.renderWidth(view)}
-		return gui.newTaskForRender(spec, prefix, cmdStr, gui.plainRender)
+		return gui.newTaskForRender(reservation, spec, prefix, cmdStr, gui.plainRender)
 	})
 
 	return nil

pkg/integration/tests/main_view/show_the_tab_switched_to_last.go

@@ -34,9 +34,6 @@ var ShowTheTabSwitchedToLast = NewIntegrationTest(NewIntegrationTestArgs{
 			IsFocused()
 
 		t.Views().Main().
-			/* EXPECTED:
 			Content(Contains("Path:"))
-			ACTUAL: */
-			Content(Contains("diff --git a/file1 b/file1"))
 	},
 })

pkg/tasks/tasks.go

@@ -64,9 +64,10 @@ type ViewBufferManager struct {
 
 	waitingMutex deadlock.Mutex
 	// Guards newTaskID and taskKey, which identify the most recently requested
-	// task. Both are written on the goroutine NewTask spawns, and taskKey is
-	// read from the UI thread (GetTaskKey), so neither may be touched without
-	// holding this.
+	// task. newTaskID is written wherever a task is asked for (ReserveTask) and
+	// read on the goroutine NewReservedTask spawns. taskKey is written on that
+	// goroutine and read from the UI thread (GetTaskKey). So neither may be
+	// touched without holding this.
 	taskIDMutex deadlock.Mutex
 	Log         *logrus.Entry
 	newTaskID   int
@@ -817,7 +818,43 @@ type TaskOpts struct {
 	InitialContentLoaded func()
 }
 
+// A TaskReservation holds a task's place in the order of the view's tasks, from
+// when the task is asked for until it is created. See ReserveTask.
+type TaskReservation struct {
+	taskID int
+}
+
+// ReserveTask gives a task its place in the order of the view's tasks, for a task
+// that is only created later, with NewReservedTask. The view shows the task that
+// was asked for last. If another task is asked for after the reservation, that
+// task replaces the reserved one, even when it is created first.
+func (self *ViewBufferManager) ReserveTask() TaskReservation {
+	self.taskIDMutex.Lock()
+	defer self.taskIDMutex.Unlock()
+
+	self.newTaskID++
+	return TaskReservation{taskID: self.newTaskID}
+}
+
+// IsSuperseded reports whether another task has been asked for since the
+// reservation was made. The reserved task would then stop as soon as it was
+// created, so there is no point in creating it.
+func (self *ViewBufferManager) IsSuperseded(reservation TaskReservation) bool {
+	self.taskIDMutex.Lock()
+	defer self.taskIDMutex.Unlock()
+
+	return reservation.taskID < self.newTaskID
+}
+
+// NewTask creates a task that takes its place in the order of the view's tasks
+// right away.
 func (self *ViewBufferManager) NewTask(f func(TaskOpts) error, key string) error {
+	return self.NewReservedTask(self.ReserveTask(), f, key)
+}
+
+// NewReservedTask creates the task that the reservation was made for. It doesn't
+// run if another task has been asked for since the reservation was made.
+func (self *ViewBufferManager) NewReservedTask(reservation TaskReservation, f func(TaskOpts) error, key string) error {
 	gocuiTask := self.newGocuiTask()
 
 	var completeTaskOnce sync.Once
@@ -828,15 +865,7 @@ func (self *ViewBufferManager) NewTask(f func(TaskOpts) error, key string) error
 		})
 	}
 
-	// Assign the taskID synchronously so it reflects NewTask call order
-	// rather than the order in which the spawned goroutines happen to be
-	// scheduled. Otherwise two NewTask calls in quick succession can have
-	// their goroutines race, with the later-called task ending up with the
-	// lower taskID and losing the staleness check below.
-	self.taskIDMutex.Lock()
-	self.newTaskID++
-	taskID := self.newTaskID
-	self.taskIDMutex.Unlock()
+	taskID := reservation.taskID
 
 	go utils.Safe(func() {
 		defer completeGocuiTask()

pkg/tasks/tasks_test.go

@@ -813,3 +813,60 @@ func TestReadToEndHoldsItsTaskUntilThenReturns(t *testing.T) {
 	assert.Equal(t, gocui.TaskStatusBusy, statusDuringThen)
 	assert.Equal(t, gocui.TaskStatusDone, task.Status())
 }
+
+// doneSignallingTask lets a test wait for the tasks of a view to finish, including
+// those that never get to run.
+type doneSignallingTask struct {
+	*gocui.FakeTask
+	done func()
+}
+
+func (self *doneSignallingTask) Done() {
+	self.FakeTask.Done()
+	self.done()
+}
+
+// A render whose task is only created after the layout takes its place among the
+// view's tasks when it is asked for. A message asked for after it, before the
+// layout, is created first, and the render's task mustn't replace it.
+func TestReservedTaskDoesntReplaceATaskAskedForLater(t *testing.T) {
+	var tasksDone sync.WaitGroup
+	manager := NewViewBufferManager(
+		utils.NewDummyLog(),
+		bytes.NewBuffer(nil),
+		func() {},
+		func() {},
+		func() {},
+		func() {},
+		func() {},
+		func() {},
+		func() gocui.Task {
+			tasksDone.Add(1)
+			return &doneSignallingTask{FakeTask: gocui.NewFakeTask(), done: tasksDone.Done}
+		},
+		// no UI thread in the test; run the view mutations inline
+		func(f func()) error { f(); return nil },
+	)
+
+	var mutex sync.Mutex
+	var tasksRun []string
+	task := func(name string) func(TaskOpts) error {
+		return func(TaskOpts) error {
+			mutex.Lock()
+			defer mutex.Unlock()
+			tasksRun = append(tasksRun, name)
+			return nil
+		}
+	}
+
+	reservation := manager.ReserveTask()
+	assert.False(t, manager.IsSuperseded(reservation))
+
+	_ = manager.NewTask(task("message"), "message")
+	assert.True(t, manager.IsSuperseded(reservation))
+
+	_ = manager.NewReservedTask(reservation, task("render"), "render")
+	tasksDone.Wait()
+
+	assert.Equal(t, []string{"message"}, tasksRun)
+}