Don't take the view over for "loading..." when the content isn't changing

A render that takes more than 200ms to produce its first line takes the view
over to say "loading...", which clears the buffer it was showing. That is
worth doing when the content coming is different — the view is showing
something the user has moved on from, and saying so beats leaving it there
silently. It is pure flicker when the content isn't changing: the view is
already showing exactly what the render is about to put back, and a slow
re-render of unchanged content is common (a background refresh over a repo
with submodules that have uncommitted changes, say).

So track whether the render in flight has content the view isn't already
showing, and only let the indicator take over when it does.

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

pkg/tasks/tasks.go

@@ -78,6 +78,13 @@ type ViewBufferManager struct {
 	taskKey   string
 	onNewKey  func()
 
+	// Whether the content the running task is rendering differs from what the
+	// view is currently showing (i.e. the command key changed). The loading
+	// indicator only takes the view over when it is set: there is no point
+	// clearing content we are about to render identically. Cleared once the
+	// task has rendered enough for the view to be showing the new content.
+	newContentPending atomic.Bool
+
 	// Whether a command task is currently reading content into the view. While
 	// this is true the content is still growing, so callers (e.g. the layout)
 	// must not clamp the view's scroll position to the amount loaded so far.
@@ -300,7 +307,14 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix
 				return
 			case <-ticker.C:
 				loadingMutex.Lock()
-				if !loaded {
+				// Only take the view over to say "loading..." when the content coming
+				// is different from what's on screen. A re-render of the same content
+				// leaves the view showing exactly what it should already, so clearing
+				// it for the message and then rendering the same thing back is a
+				// visible flicker for nothing — and a slow re-render of unchanged
+				// content is common (a background refresh over a repo with submodules
+				// that have uncommitted changes, say).
+				if !loaded && self.newContentPending.Load() {
 					self.beforeStart()
 					_, _ = self.writer.Write([]byte("loading..."))
 					self.refreshView()
@@ -399,6 +413,8 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix
 							// whether to scroll) and sets the origin, both of which
 							// are UI-thread-only, so run it there.
 							_ = self.onUIThread(self.onEndOfInput)
+							// Whatever there was to show is on screen now.
+							self.newContentPending.Store(false)
 							// The content is fully loaded now, so it's safe again for the
 							// layout to clamp the scroll position to it. We deliberately
 							// don't clear this when stopped (rather than EOF'd), because that
@@ -434,6 +450,7 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix
 							// We have read enough lines to fill the view, so do a first refresh
 							// here to show what we have. Continue reading and refresh again at
 							// the end to make sure the scrollbar has the right size.
+							self.newContentPending.Store(false)
 							refreshViewIfStale()
 						}
 					}
@@ -554,7 +571,11 @@ func (self *ViewBufferManager) NewTask(f func(TaskOpts) error, key string) error
 
 		// Read taskKey directly: we already hold the mutex that guards it, and
 		// GetTaskKey would take it again.
-		resetOrigin := self.taskKey != key && self.onNewKey != nil
+		newContent := self.taskKey != key
+		if newContent {
+			self.newContentPending.Store(true)
+		}
+		resetOrigin := newContent && self.onNewKey != nil
 		self.taskKey = key
 
 		self.taskIDMutex.Unlock()

pkg/tasks/tasks_test.go

@@ -7,6 +7,7 @@ import (
 	"reflect"
 	"strings"
 	"sync"
+	"sync/atomic"
 	"testing"
 	"time"
 
@@ -251,6 +252,80 @@ func TestNewCmdTaskQueuedReadAtEndOfInput(t *testing.T) {
 	assert.True(t, thenCalled)
 }
 
+// A writer that records whether the loading indicator was ever written to it.
+type LoadingIndicatorSpy struct {
+	sawLoadingIndicator atomic.Bool
+}
+
+func (self *LoadingIndicatorSpy) Write(p []byte) (n int, err error) {
+	if bytes.Contains(p, []byte("loading...")) {
+		self.sawLoadingIndicator.Store(true)
+	}
+	return len(p), nil
+}
+
+// A render that takes long enough to produce its first line takes the view over
+// to say "loading...", which means clearing whatever it was showing. That is only
+// worth doing when the content coming is different from what's on screen:
+// re-rendering the same content would otherwise clear the view and render the
+// same thing straight back, a visible flicker for nothing.
+func TestLoadingIndicatorOnlyTakesOverForNewContent(t *testing.T) {
+	writer := &LoadingIndicatorSpy{}
+
+	manager := NewViewBufferManager(
+		utils.NewDummyLog(),
+		writer,
+		func() {},
+		func() {},
+		func() {},
+		func() {},
+		func() gocui.Task { return gocui.NewFakeTask() },
+		// no UI thread in the test; run the view mutations inline
+		func(f func()) error { f(); return nil },
+	)
+
+	startTask := func(key string, reader io.Reader, onDone func()) {
+		start := func() (Cmd, io.Reader) {
+			// not actually starting this because it's not necessary
+			return ExecCmd{Cmd: exec.Command("blah")}, reader
+		}
+		_ = manager.NewTask(manager.NewCmdTask(start, "", LinesToRead{100, 50, nil}, onDone), key)
+	}
+	// Starts a task whose command produces nothing at all, so that it is still
+	// waiting for its first line when the loading indicator falls due. Returns
+	// the reader so the caller can let it finish.
+	startStalledTask := func(key string) *BlockingLineReader {
+		reader := &BlockingLineReader{
+			blocked: make(chan struct{}),
+			unblock: make(chan struct{}),
+		}
+		startTask(key, reader, nil)
+		<-reader.blocked
+		return reader
+	}
+
+	// Get some content on screen first: the indicator is only due when a render
+	// is slow, and this one isn't.
+	done := make(chan struct{})
+	startTask("cmd1", &BlankLineReader{totalLinesToYield: 3}, func() { close(done) })
+	<-done
+	assert.False(t, writer.sawLoadingIndicator.Load())
+
+	// A slow re-render of that same content must leave the view alone however
+	// long it takes. The indicator is due 200ms in, so give it well past that.
+	sameContent := startStalledTask("cmd1")
+	defer close(sameContent.unblock)
+	time.Sleep(500 * time.Millisecond)
+	assert.False(t, writer.sawLoadingIndicator.Load())
+
+	// Different content, though, is worth taking the view over for.
+	newContent := startStalledTask("cmd2")
+	defer close(newContent.unblock)
+	assert.Eventually(t,
+		writer.sawLoadingIndicator.Load,
+		2*time.Second, 10*time.Millisecond)
+}
+
 func TestNewCmdTaskRefresh(t *testing.T) {
 	type scenario struct {
 		name                        string