From 0483b128642dd41517d5ce0e76b927c1a3a31eda Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 4 Jun 2026 10:43:21 +0200 Subject: [PATCH] Don't scroll a view up to fill blank space while its content is loading The layout scrolls a view up if its origin is past the bottom of its content, to avoid showing blank space (e.g. after a resize). But it measures content height by the lines loaded so far, and command/pty tasks load asynchronously. So when a view is re-rendered while scrolled down, the layout would yank it to the top because only a fraction of the content has been read yet, then leave it there once loading finished. Track whether a command task is actively reading (set synchronously when the task is created, so a layout pass in between sees it; cleared at EOF, but not when stopped, since that means a newer task is taking over) and skip the scroll-up clamp for such views. onEndOfInput already re-clamps once loading completes. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/gui/layout.go | 8 +++++++- pkg/gui/pty.go | 6 ++++++ pkg/gui/tasks_adapter.go | 4 ++++ pkg/tasks/tasks.go | 25 +++++++++++++++++++++++++ 4 files changed, 42 insertions(+), 1 deletion(-) diff --git a/pkg/gui/layout.go b/pkg/gui/layout.go index bcdc0edfc..67e695f2b 100644 --- a/pkg/gui/layout.go +++ b/pkg/gui/layout.go @@ -88,7 +88,13 @@ func (gui *Gui) layout(g *gocui.Gui) error { if !view.CanScrollPastBottom { maxOriginY -= newHeight - 1 } - if oldOriginY := view.OriginY(); oldOriginY > maxOriginY { + // Don't scroll up while the view's content is still being loaded: its + // height only reflects what has been read so far, so clamping to it now + // would yank the view to the top even though more content is on the way + // (e.g. when re-rendering a diff the user was scrolled into). + manager := gui.getViewBufferManagerForView(view) + stillLoading := manager != nil && manager.IsLoading() + if oldOriginY := view.OriginY(); oldOriginY > maxOriginY && !stillLoading { view.ScrollUp(oldOriginY - maxOriginY) // the view might not have scrolled actually (if it was at the limit // already), so we need to check if it did diff --git a/pkg/gui/pty.go b/pkg/gui/pty.go index 719f5c348..1cb8d9412 100644 --- a/pkg/gui/pty.go +++ b/pkg/gui/pty.go @@ -72,6 +72,12 @@ func (gui *Gui) newPtyTask(view *gocui.View, cmd *exec.Cmd, prefix string) error cmd.Args = withPtyGitConfig(cmd.Args, runtime.GOOS) + // 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() + // Run the pty after layout so that it gets the correct size gui.afterLayout(func() error { // Need to get the width and the pager command again because the layout might have diff --git a/pkg/gui/tasks_adapter.go b/pkg/gui/tasks_adapter.go index 3dce93874..3c9eef353 100644 --- a/pkg/gui/tasks_adapter.go +++ b/pkg/gui/tasks_adapter.go @@ -18,6 +18,10 @@ func (gui *Gui) newCmdTask(view *gocui.View, cmd *exec.Cmd, prefix string) error ).Debug("RunCommand") manager := gui.getManager(view) + // 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. + manager.StartLoading() // Snapshot the view width here, on the UI thread, so the task goroutine // doesn't read the view's live dimensions while it streams output. It's diff --git a/pkg/tasks/tasks.go b/pkg/tasks/tasks.go index 0bc05f3d0..059bf2173 100644 --- a/pkg/tasks/tasks.go +++ b/pkg/tasks/tasks.go @@ -74,6 +74,11 @@ type ViewBufferManager struct { taskKey string onNewKey func() + // 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. + loading atomic.Bool + // beforeStart is the function that is called before starting a new task beforeStart func() refreshView func() @@ -155,6 +160,21 @@ func (self *ViewBufferManager) ReadLines(totalLines int) { } } +// IsLoading reports whether a command task is currently reading content into the +// view, meaning the content is still growing. +func (self *ViewBufferManager) IsLoading() bool { + return self.loading.Load() +} + +// StartLoading marks the view as loading content. It must be called +// synchronously when a command/pty task is started, before the task's goroutine +// runs, so that a layout pass happening in between doesn't clamp the scroll +// position to the not-yet-loaded content. It is cleared when the task reaches +// the end of its input. +func (self *ViewBufferManager) StartLoading() { + self.loading.Store(true) +} + func (self *ViewBufferManager) ReadToEnd(then func()) { if ch := self.readLines.Load(); ch != nil { readLines := *ch @@ -375,6 +395,11 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix self.onEndOfInput() return nil }) + // 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 + // means a newer task is taking over and is still loading. + self.loading.Store(false) callThen() break outer }