mirror of
https://github.com/jesseduffield/lazygit.git
synced 2026-09-10 15:46:26 -04:00
Let a cmd/pty task restore a saved scroll position at its first paint
When re-rendering content the user was already scrolled into, we want the saved scroll position applied exactly when the real content first paints — not before. Setting the origin up front instead paints it onto whatever placeholder is currently in the view (e.g. the shorter buffer CopyContent left there), which flickers: either a blank frame past the placeholder's end, or a jump to the top when the task resets the origin at startup. Add ViewBufferManager.ScrollToOriginYForNextTask: the next cmd/pty task then (a) does not reset the view to the top at startup even though the command key changed, so the placeholder stays put, (b) sizes its initial read to the saved position so enough content is loaded to fill the view there, and (c) scrolls to it as part of the first refresh, in the same paint that shows the real content. This is the cmd/pty analogue of RenderStringWithScrollTask. No caller sets it yet, so this is behaviour-preserving on its own. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
53a25a34f1
commit
3f542e7c35
|
|
@ -78,6 +78,11 @@ func (gui *Gui) newPtyTask(view *gocui.View, cmd *exec.Cmd, prefix string) error
|
|||
// not-yet-loaded content.
|
||||
gui.getManager(view).StartLoading()
|
||||
|
||||
// Read any requested scroll-restore now so we can size the initial read to it
|
||||
// in afterLayout; the task itself clears the request and applies the scroll at
|
||||
// its first paint.
|
||||
targetOriginY := gui.getManager(view).GetScrollToOriginYForNextTask()
|
||||
|
||||
// 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
|
||||
|
|
@ -142,7 +147,7 @@ func (gui *Gui) newPtyTask(view *gocui.View, cmd *exec.Cmd, prefix string) error
|
|||
gui.Mutexes.PtyMutex.Unlock()
|
||||
}
|
||||
|
||||
linesToRead := gui.linesToReadFromCmdTask(view)
|
||||
linesToRead := gui.linesToReadFromCmdTask(view, targetOriginY)
|
||||
return manager.NewTask(manager.NewCmdTask(start, prefix, linesToRead, onClose), cmdStr)
|
||||
})
|
||||
|
||||
|
|
|
|||
|
|
@ -30,6 +30,11 @@ func (gui *Gui) newCmdTask(view *gocui.View, cmd *exec.Cmd, prefix string) error
|
|||
// still-running writes (see View.SetContentWidth).
|
||||
contentWidth := view.InnerWidth()
|
||||
|
||||
// If a caller asked us to restore a scroll position for this render, size the
|
||||
// initial read to it (below) and let the task scroll there at its first paint.
|
||||
// The task clears the request and suppresses the origin reset when it starts.
|
||||
targetOriginY := manager.GetScrollToOriginYForNextTask()
|
||||
|
||||
var r io.ReadCloser
|
||||
start := func() (tasks.Cmd, io.Reader) {
|
||||
view.SetContentWidth(contentWidth)
|
||||
|
|
@ -46,7 +51,7 @@ func (gui *Gui) newCmdTask(view *gocui.View, cmd *exec.Cmd, prefix string) error
|
|||
}
|
||||
}
|
||||
|
||||
linesToRead := gui.linesToReadFromCmdTask(view)
|
||||
linesToRead := gui.linesToReadFromCmdTask(view, targetOriginY)
|
||||
if err := manager.NewTask(manager.NewCmdTask(start, prefix, linesToRead, onClose), cmdStr); err != nil {
|
||||
gui.c.Log.Error(err)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -19,9 +19,22 @@ func (gui *Gui) resetViewOrigin(v *gocui.View) {
|
|||
// Returns the number of lines that we should read initially from a cmd task so
|
||||
// that the scrollbar has the correct size, along with the number of lines after
|
||||
// which the view is filled and we can do a first refresh.
|
||||
func (gui *Gui) linesToReadFromCmdTask(v *gocui.View) tasks.LinesToRead {
|
||||
//
|
||||
// If targetOriginY is non-nil, the read is sized to that scroll position rather
|
||||
// than the view's current one, and the returned LinesToRead carries an
|
||||
// ApplyInitialScroll that scrolls the view there at the first refresh. This is
|
||||
// used when re-rendering content the user was already scrolled into, so the
|
||||
// saved position is applied exactly when the content first paints.
|
||||
func (gui *Gui) linesToReadFromCmdTask(v *gocui.View, targetOriginY *int) tasks.LinesToRead {
|
||||
height := v.InnerHeight()
|
||||
oy := v.OriginY()
|
||||
var applyInitialScroll func()
|
||||
if targetOriginY != nil {
|
||||
oy = *targetOriginY
|
||||
applyInitialScroll = func() {
|
||||
v.SetOrigin(v.OriginX(), *targetOriginY)
|
||||
}
|
||||
}
|
||||
|
||||
linesForFirstRefresh := height + oy + 10
|
||||
|
||||
|
|
@ -37,6 +50,7 @@ func (gui *Gui) linesToReadFromCmdTask(v *gocui.View) tasks.LinesToRead {
|
|||
return tasks.LinesToRead{
|
||||
Total: linesToReadForAccurateScrollbar,
|
||||
InitialRefreshAfter: linesForFirstRefresh,
|
||||
ApplyInitialScroll: applyInitialScroll,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -74,6 +74,22 @@ type ViewBufferManager struct {
|
|||
taskKey string
|
||||
onNewKey func()
|
||||
|
||||
// When non-nil, the next cmd/pty task restores this scroll position. Used
|
||||
// when re-rendering content the user was already scrolled into (e.g. when
|
||||
// returning to a focused main view on escape). It has two effects:
|
||||
//
|
||||
// - The task does not reset the view's origin to the top at start, even if
|
||||
// its command key differs from the previous task's. This keeps the
|
||||
// placeholder that CopyContent left in the view showing at its current
|
||||
// scroll position (i.e. "as if nothing changed") until the real content
|
||||
// is ready, rather than flicking it to the top.
|
||||
// - The task sizes its initial read to this position and scrolls there as
|
||||
// part of its first refresh, so the saved scroll is applied in the same
|
||||
// paint that shows the real content.
|
||||
//
|
||||
// Cleared once the next task has started.
|
||||
scrollToOriginYForNextTask *int
|
||||
|
||||
// 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.
|
||||
|
|
@ -117,6 +133,12 @@ type LinesToRead struct {
|
|||
// subsequent requests.
|
||||
InitialRefreshAfter int
|
||||
|
||||
// When set, called once, just before the view is first refreshed, to scroll
|
||||
// it to a saved position. Used so that content the user was scrolled into is
|
||||
// painted at the saved scroll position the first time it appears, rather than
|
||||
// at the top. Only set for the initial read request.
|
||||
ApplyInitialScroll func()
|
||||
|
||||
// Function to call after reading the lines is done
|
||||
Then func()
|
||||
}
|
||||
|
|
@ -160,6 +182,22 @@ func (self *ViewBufferManager) ReadLines(totalLines int) {
|
|||
}
|
||||
}
|
||||
|
||||
// ScrollToOriginYForNextTask makes the next cmd/pty task restore the given
|
||||
// scroll position instead of rendering at the top. Call this right before
|
||||
// triggering a re-render of content the view is already scrolled into (e.g.
|
||||
// when returning to a focused main view on escape). See the field doc for the
|
||||
// two effects this has. It is cleared once the next task starts.
|
||||
func (self *ViewBufferManager) ScrollToOriginYForNextTask(originY int) {
|
||||
self.scrollToOriginYForNextTask = &originY
|
||||
}
|
||||
|
||||
// GetScrollToOriginYForNextTask returns the scroll position requested by a
|
||||
// preceding ScrollToOriginYForNextTask call, or nil if none. It does not clear
|
||||
// it; the task clears it when it starts.
|
||||
func (self *ViewBufferManager) GetScrollToOriginYForNextTask() *int {
|
||||
return self.scrollToOriginYForNextTask
|
||||
}
|
||||
|
||||
// IsLoading reports whether a command task is currently reading content into the
|
||||
// view, meaning the content is still growing.
|
||||
func (self *ViewBufferManager) IsLoading() bool {
|
||||
|
|
@ -335,6 +373,18 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix
|
|||
// this to work out how many more lines, if any, we still need to read.
|
||||
linesRead := 0
|
||||
|
||||
// The initial read request carries an optional scroll-to function (see
|
||||
// LinesToRead.ApplyInitialScroll). We apply it exactly once, right before
|
||||
// the view is first refreshed, so that content the user was scrolled into
|
||||
// is painted at the saved position the first time it appears.
|
||||
initialScroll := linesToRead.ApplyInitialScroll
|
||||
var applyInitialScrollOnce sync.Once
|
||||
applyInitialScroll := func() {
|
||||
if initialScroll != nil {
|
||||
applyInitialScrollOnce.Do(initialScroll)
|
||||
}
|
||||
}
|
||||
|
||||
// Set LAZYGIT_SLOW_RENDER=<milliseconds> to sleep that long after each
|
||||
// line is written to the view, stretching async loads out so the frames
|
||||
// of a re-render become visible. Useful for debugging scroll/flicker
|
||||
|
|
@ -387,11 +437,14 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix
|
|||
|
||||
if !ok {
|
||||
// if we're here then there's nothing left to scan from the source
|
||||
// so we're at the EOF and can flush the stale content.
|
||||
// so we're at the EOF and can flush the stale content. Apply the
|
||||
// saved scroll first (if any) so that onEndOfInput clamps it back
|
||||
// into range when the new content turned out shorter than expected.
|
||||
// onEndOfInput reads the view's dimensions (to decide
|
||||
// whether to scroll) and sets the origin, both of which
|
||||
// are UI-thread-only, so run it there.
|
||||
_ = self.onUIThread(func() error {
|
||||
applyInitialScroll()
|
||||
self.onEndOfInput()
|
||||
return nil
|
||||
})
|
||||
|
|
@ -428,8 +481,11 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix
|
|||
|
||||
if linesRead == linesToRead.InitialRefreshAfter {
|
||||
// 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.
|
||||
// here to show what we have. Apply the saved scroll first (if any)
|
||||
// so the first paint already lands at it. Continue reading and
|
||||
// refresh again at the end to make sure the scrollbar has the right
|
||||
// size.
|
||||
applyInitialScroll()
|
||||
refreshViewIfStale()
|
||||
}
|
||||
}
|
||||
|
|
@ -548,7 +604,12 @@ func (self *ViewBufferManager) NewTask(f func(TaskOpts) error, key string) error
|
|||
return
|
||||
}
|
||||
|
||||
resetOrigin := self.GetTaskKey() != key && self.onNewKey != nil
|
||||
// Reset the origin to the top when the command changed, unless a caller
|
||||
// asked us to restore a scroll position: in that case we keep the
|
||||
// placeholder showing at its current scroll until the task scrolls to the
|
||||
// saved position as part of its first paint (see scrollToOriginYForNextTask).
|
||||
resetOrigin := self.GetTaskKey() != key && self.onNewKey != nil && self.scrollToOriginYForNextTask == nil
|
||||
self.scrollToOriginYForNextTask = nil
|
||||
self.taskKey = key
|
||||
|
||||
self.taskIDMutex.Unlock()
|
||||
|
|
|
|||
|
|
@ -54,7 +54,7 @@ func TestNewCmdTaskInstantStop(t *testing.T) {
|
|||
return ExecCmd{Cmd: cmd}, reader
|
||||
}
|
||||
|
||||
fn := manager.NewCmdTask(start, "prefix\n", LinesToRead{20, -1, nil}, onDone)
|
||||
fn := manager.NewCmdTask(start, "prefix\n", LinesToRead{Total: 20, InitialRefreshAfter: -1}, onDone)
|
||||
|
||||
_ = fn(TaskOpts{Stop: stop, InitialContentLoaded: func() { task.Done() }})
|
||||
|
||||
|
|
@ -119,7 +119,7 @@ func TestNewCmdTask(t *testing.T) {
|
|||
return ExecCmd{Cmd: cmd}, reader
|
||||
}
|
||||
|
||||
fn := manager.NewCmdTask(start, "prefix\n", LinesToRead{20, -1, nil}, onDone)
|
||||
fn := manager.NewCmdTask(start, "prefix\n", LinesToRead{Total: 20, InitialRefreshAfter: -1}, onDone)
|
||||
wg := sync.WaitGroup{}
|
||||
wg.Go(func() {
|
||||
time.Sleep(100 * time.Millisecond)
|
||||
|
|
@ -186,37 +186,37 @@ func TestNewCmdTaskRefresh(t *testing.T) {
|
|||
{
|
||||
"total < initialRefreshAfter",
|
||||
150,
|
||||
LinesToRead{100, 120, nil},
|
||||
LinesToRead{Total: 100, InitialRefreshAfter: 120},
|
||||
[]int{100},
|
||||
},
|
||||
{
|
||||
"total == initialRefreshAfter",
|
||||
150,
|
||||
LinesToRead{100, 100, nil},
|
||||
LinesToRead{Total: 100, InitialRefreshAfter: 100},
|
||||
[]int{100},
|
||||
},
|
||||
{
|
||||
"total > initialRefreshAfter",
|
||||
150,
|
||||
LinesToRead{100, 50, nil},
|
||||
LinesToRead{Total: 100, InitialRefreshAfter: 50},
|
||||
[]int{50, 100},
|
||||
},
|
||||
{
|
||||
"initialRefreshAfter == -1",
|
||||
150,
|
||||
LinesToRead{100, -1, nil},
|
||||
LinesToRead{Total: 100, InitialRefreshAfter: -1},
|
||||
[]int{100},
|
||||
},
|
||||
{
|
||||
"totalTaskLines < initialRefreshAfter",
|
||||
25,
|
||||
LinesToRead{100, 50, nil},
|
||||
LinesToRead{Total: 100, InitialRefreshAfter: 50},
|
||||
[]int{25},
|
||||
},
|
||||
{
|
||||
"totalTaskLines between total and initialRefreshAfter",
|
||||
75,
|
||||
LinesToRead{100, 50, nil},
|
||||
LinesToRead{Total: 100, InitialRefreshAfter: 50},
|
||||
[]int{50, 75},
|
||||
},
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue