Restore the focused main view's selection via the re-render task, not a post-hoc ReadToEnd

Escaping to a focused main view restored the selection by scheduling, on
the next UI tick, a ReadToEnd whose callback re-selected the saved line.
But ReadToEnd fires its callback synchronously when the manager has no
live read channel, and the re-render task triggered by the push creates
that channel later, inside its own goroutine (after stopping the previous
task). If the UI tick won that race, the restore ran before any content
was loaded, FocusPoint no-oped against the unloaded line, and the
selection was silently dropped — intermittently, and more often under
load.

Thread the restore through the task instead: a thenForNextTask hook on
the buffer manager, folded into the next cmd/pty task's initial-read Then,
mirroring scrollToOriginYForNextTask. It runs once the task has read
enough to place the selection, and can't fire before the task exists. The
scroll restore already applies at the task's first paint, which precedes
the initial read's end, so the origin is in place when the selection is
restored.

This needs interactive verification (LAZYGIT_SLOW_RENDER + a real pager);
see focused-main-view-notes.md §13.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Stefan Haller 2026-06-08 10:29:33 +02:00
parent 3af439b17f
commit 23474716ad
4 changed files with 69 additions and 32 deletions

View file

@ -61,15 +61,34 @@ func EscapeFromPatchExplorer(c *HelperCommon, context types.IPatchExplorerContex
view := snapshot.MainView.GetView()
// Ask the upcoming re-render to restore the scroll position. Pushing the side
// panel re-renders its content into the main view via a cmd/pty task. Until
// that content is ready, the main view keeps showing the placeholder that
// CopyContent left in it (the view we're leaving) at its current scroll; the
// task then scrolls to the saved position as part of the first paint that
// shows the real content. Doing it this way (rather than setting the origin up
// front) avoids both a jump to the top and a misplaced placeholder frame.
if manager := c.GetViewBufferManagerForView(view); manager != nil {
restore := func() {
view.FocusPoint(0, snapshot.SelectedLineIdx, false)
view.Highlight = true
view.HighlightInactive = false
}
// Ask the upcoming re-render to restore the scroll position and selection.
// Pushing the side panel re-renders its content into the main view via a
// cmd/pty task. Until that content is ready, the main view keeps showing the
// placeholder that CopyContent left in it (the view we're leaving) at its
// current scroll; the task then scrolls to the saved position as part of the
// first paint that shows the real content (rather than setting the origin up
// front, which would jump to the top or show a misplaced placeholder frame).
// The selection needs the content loaded down to the selected line, so it
// rides the same task and fires at the end of its initial read. Threading it
// through the task (rather than a ReadToEnd issued after the pushes) avoids a
// race: ReadToEnd fires synchronously when the freshly-created task's read
// channel isn't live yet, which would run FocusPoint before the content is
// loaded and silently drop the selection.
manager := c.GetViewBufferManagerForView(view)
if manager != nil {
manager.ScrollToOriginYForNextTask(snapshot.OriginY)
manager.ThenForNextTask(func() {
c.OnUIThread(func() error {
restore()
return nil
})
})
}
// Land on the side panel first (this re-renders the original content into the
@ -77,31 +96,10 @@ func EscapeFromPatchExplorer(c *HelperCommon, context types.IPatchExplorerContex
c.Context().Push(snapshot.SidePanel, types.OnFocusOpts{})
c.Context().Push(snapshot.MainView, types.OnFocusOpts{})
restore := func() {
view.FocusPoint(0, snapshot.SelectedLineIdx, false)
view.Highlight = true
view.HighlightInactive = false
// Without a buffer manager there is no re-render task to ride, so restore now.
if manager == nil {
restore()
}
// The scroll position is handled by the re-render above, but the selection
// still needs the content loaded down to the selected line, which happens
// asynchronously. Wait until the diff has been fully read before restoring
// it. We do this on the next UI tick, by which point the re-render task is
// live and ReadToEnd can hook into it.
c.OnUIThread(func() error {
manager := c.GetViewBufferManagerForView(view)
if manager == nil {
restore()
return nil
}
manager.ReadToEnd(func() {
c.OnUIThread(func() error {
restore()
return nil
})
})
return nil
})
}
// kills the custom patch and returns us back to the commit files panel if needed

View file

@ -155,6 +155,10 @@ func (gui *Gui) newPtyTask(view *gocui.View, cmd *exec.Cmd, prefix string) error
}
linesToRead := gui.linesToReadFromCmdTask(view, targetOriginY)
// As in newCmdTask: let the task run any requested after-load callback at
// the end of its initial read (e.g. restoring a focused main view's
// selection on escape). The task clears the request when it starts.
linesToRead.Then = manager.GetThenForNextTask()
return manager.NewTask(manager.NewCmdTask(start, prefix, linesToRead, onClose), cmdStr)
})

View file

@ -52,6 +52,11 @@ func (gui *Gui) newCmdTask(view *gocui.View, cmd *exec.Cmd, prefix string) error
}
linesToRead := gui.linesToReadFromCmdTask(view, targetOriginY)
// If a caller asked us to run something once this re-render has loaded (e.g.
// restoring a focused main view's selection on escape), let the task own it,
// firing at the end of its initial read. The task clears the request when it
// starts.
linesToRead.Then = manager.GetThenForNextTask()
if err := manager.NewTask(manager.NewCmdTask(start, prefix, linesToRead, onClose), cmdStr); err != nil {
gui.c.Log.Error(err)
}

View file

@ -90,6 +90,19 @@ type ViewBufferManager struct {
// Cleared once the next task has started.
scrollToOriginYForNextTask *int
// When non-nil, run once after the next cmd/pty task has read enough of its
// content (the end of its initial read). Used to restore state that depends
// on the re-rendered content being loaded — e.g. the selection in a focused
// main view we're returning to on escape — by riding the task's own
// lifecycle rather than a separate post-hoc ReadToEnd. A ReadToEnd issued
// right after triggering the re-render can fire synchronously, because the
// freshly-created task's read channel isn't live yet (it is created inside
// the task goroutine, after the previous task has been stopped); that would
// run the restore before any content is loaded and silently drop it.
//
// Cleared once the next task has started.
thenForNextTask 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.
@ -198,6 +211,22 @@ func (self *ViewBufferManager) GetScrollToOriginYForNextTask() *int {
return self.scrollToOriginYForNextTask
}
// ThenForNextTask makes the next cmd/pty task run the given function once it has
// read enough of its content (the end of its initial read). Call this right
// before triggering a re-render whose loaded content the function depends on.
// See the field doc for why this is preferable to a separate ReadToEnd. It is
// cleared once the next task starts.
func (self *ViewBufferManager) ThenForNextTask(then func()) {
self.thenForNextTask = then
}
// GetThenForNextTask returns the function requested by a preceding
// ThenForNextTask call, or nil if none. It does not clear it; the task clears it
// when it starts.
func (self *ViewBufferManager) GetThenForNextTask() func() {
return self.thenForNextTask
}
// IsLoading reports whether a command task is currently reading content into the
// view, meaning the content is still growing.
func (self *ViewBufferManager) IsLoading() bool {
@ -610,6 +639,7 @@ func (self *ViewBufferManager) NewTask(f func(TaskOpts) error, key string) error
// saved position as part of its first paint (see scrollToOriginYForNextTask).
resetOrigin := self.GetTaskKey() != key && self.onNewKey != nil && self.scrollToOriginYForNextTask == nil
self.scrollToOriginYForNextTask = nil
self.thenForNextTask = nil
self.taskKey = key
self.taskIDMutex.Unlock()