Merge branch 'fix-slow-scroll-down'

Fixes: #471

Author: walles

internal/pager.go

@@ -68,6 +68,12 @@ type Pager struct {
 	// Written and read only by the main loop, like quit above, so no locking.
 	sawUserInput bool
 
+	// Set when the user scrolls down. On the next redraw, we start following
+	// the end of the input if the user scrolled all the way there.
+	//
+	// Written and read only by the main loop, like quit above, so no locking.
+	pendingFollowCheck bool
+
 	scrollPosition scrollPosition
 
 	// How far right we have scrolled horizontally. The unit is visual screen
@@ -469,13 +475,28 @@ func (p *Pager) handleScrolledUp() {
 	p.setTargetLine(nil)
 }
 
+// Call after scrolling down. Cheap. If we ended up at the end of the input,
+// following it starts on the next redraw.
 func (p *Pager) handleScrolledDown() {
-	if p.isScrolledToEnd() {
-		// Follow output
-		p.setTargetLine(ptr.To(linemetadata.IndexMax()))
-	} else {
-		p.setTargetLine(nil)
+	p.setTargetLine(nil)
+
+	// After setTargetLine(), which clears it
+	p.pendingFollowCheck = true
+}
+
+// Start following the end of the input if the user scrolled down to it since
+// the last call. Expensive when a check is pending, cheap otherwise.
+func (p *Pager) followIfScrolledToEnd() {
+	if !p.pendingFollowCheck {
+		return
+	}
+	p.pendingFollowCheck = false
+
+	if !p.isScrolledToEnd() {
+		return
 	}
+
+	p.setTargetLine(ptr.To(linemetadata.IndexMax()))
 }
 
 func (p *Pager) handleMoreLinesAvailable() {
@@ -513,6 +534,11 @@ func (p *Pager) setTargetLine(targetLine *linemetadata.Index) {
 
 	log.Trace("Pager: Setting target line to ", targetLine, "...")
 	p.TargetLine = targetLine
+
+	// An explicit target wins over any not-yet-done follow check from an
+	// earlier scroll down
+	p.pendingFollowCheck = false
+
 	if targetLine == nil {
 		// No target, just do your thing
 		r.SetPauseAfterLines(reader.DEFAULT_PAUSE_AFTER_LINES)

internal/pagermode-viewing_test.go

@@ -4,6 +4,7 @@ import (
 	"os"
 	"testing"
 
+	"github.com/walles/moor/v2/internal/linemetadata"
 	"github.com/walles/moor/v2/internal/reader"
 	"github.com/walles/twin"
 	"gotest.tools/v3/assert"
@@ -35,6 +36,83 @@ func TestErrUnlessExecutable_no(t *testing.T) {
 	}
 }
 
+// Counts how many times lines are fetched from the wrapped reader
+type lineReadCountingReader struct {
+	reader.Reader
+	lineReads int
+}
+
+func (r *lineReadCountingReader) GetLine(index linemetadata.Index) *reader.NumberedLine {
+	r.lineReads++
+	return r.Reader.GetLine(index)
+}
+
+func (r *lineReadCountingReader) GetLines(firstLine linemetadata.Index, wantedLineCount int) reader.InputLines {
+	r.lineReads++
+	return r.Reader.GetLines(firstLine, wantedLineCount)
+}
+
+func (r *lineReadCountingReader) GetLinesPreallocated(firstLine linemetadata.Index, resultLines *[]reader.NumberedLine) reader.Status {
+	r.lineReads++
+	return r.Reader.GetLinesPreallocated(firstLine, resultLines)
+}
+
+// Inertial scrolling can queue hundreds of scroll-down events, and the screen
+// is redrawn only after all of them have been handled. So handling one must not
+// touch the input lines, all such work waits for the next redraw.
+func TestViewingScrollDownIsCheap(t *testing.T) {
+	r := reader.NewFromTextForTesting("test", "a\nb\nc\nd\ne\nf\ng\nh\ni\nj\n")
+
+	pager := NewPager(r)
+	pager.screen = twin.NewFakeScreen(20, 5)
+	countingReader := &lineReadCountingReader{Reader: r}
+	pager.filteringReader.BackingReader = countingReader
+
+	mode := PagerModeViewing{pager: pager}
+	mode.onKey(twin.KeyDown)
+
+	assert.Equal(t, 0, countingReader.lineReads)
+}
+
+// Scrolling down to the last line starts following the end of the input.
+func TestViewingScrollDownToEndFollows(t *testing.T) {
+	r := reader.NewFromTextForTesting("test", "a\nb\nc\nd\ne\nf\ng\nh\ni\nj\n")
+
+	pager := NewPager(r)
+	pager.screen = twin.NewFakeScreen(20, 5)
+	assert.Assert(t, pager.TargetLine == nil)
+
+	// One line per press is guaranteed to get us to the end
+	mode := PagerModeViewing{pager: pager}
+	for range r.GetLineCount() {
+		mode.onKey(twin.KeyDown)
+	}
+	pager.redraw("")
+
+	assert.Assert(t, pager.TargetLine != nil)
+	assert.Equal(t, linemetadata.IndexMax(), *pager.TargetLine)
+}
+
+// Choosing a target line after scrolling down to the end keeps that target.
+func TestViewingScrollDownToEndThenTargetLine(t *testing.T) {
+	r := reader.NewFromTextForTesting("test", "a\nb\nc\nd\ne\nf\ng\nh\ni\nj\n")
+
+	pager := NewPager(r)
+	pager.screen = twin.NewFakeScreen(20, 5)
+
+	// One line per press is guaranteed to get us to the end
+	mode := PagerModeViewing{pager: pager}
+	for range r.GetLineCount() {
+		mode.onKey(twin.KeyDown)
+	}
+	target := linemetadata.IndexFromOneBased(2)
+	pager.setTargetLine(&target)
+	pager.redraw("")
+
+	assert.Assert(t, pager.TargetLine != nil)
+	assert.Equal(t, target, *pager.TargetLine)
+}
+
 func TestViewingFooter_WithSpinner(t *testing.T) {
 	r := reader.NewFromTextForTesting("", "text")
 

internal/screenLines.go

@@ -43,6 +43,7 @@ type renderedScreen struct {
 // Refresh the whole pager display, both contents lines and the status line at
 // the bottom, and show the result on the terminal
 func (p *Pager) redraw(spinner string) {
+	p.followIfScrolledToEnd()
 	p.renderIntoCells(spinner)
 	p.screen.Show()
 }

internal/scrollPosition.go

@@ -380,8 +380,8 @@ func (p *Pager) scrollToEnd() {
 	}
 }
 
-// Can be either because Pager.scrollToEnd() was just called or because the user
-// has pressed the down arrow enough times.
+// True if the end of the input is visible on screen. Expensive, don't call too
+// often.
 func (p *Pager) isScrolledToEnd() bool {
 	inputLineCount := p.Reader().GetLineCount()
 	if inputLineCount == 0 {