Merge branch 'johan/issue-166'

Fixes #166.

.gitignore

@@ -1,3 +1,4 @@
 /moar
 /moar.exe
 /releases/moar-*
+/m/__debug_bin*

m/screenLines.go

@@ -153,7 +153,7 @@ func (p *Pager) renderLines() ([]renderedLine, string, overflowState) {
 	for lineIndex, line := range inputLines.lines {
 		lineNumber := inputLines.firstLineOneBased + lineIndex
 
-		rendering, lineOverflow := p.renderLine(line, lineNumber)
+		rendering, lineOverflow := p.renderLine(line, lineNumber, p.scrollPosition.internalDontTouch)
 		if lineOverflow == didOverflow {
 			// Everything did not fit
 			screenOverflow = didOverflow
@@ -203,13 +203,13 @@ func (p *Pager) renderLines() ([]renderedLine, string, overflowState) {
 //
 // lineNumber and numberPrefixLength are required for knowing how much to
 // indent, and to (optionally) render the line number.
-func (p *Pager) renderLine(line *Line, lineNumber int) ([]renderedLine, overflowState) {
+func (p *Pager) renderLine(line *Line, lineNumber int, scrollPosition scrollPositionInternal) ([]renderedLine, overflowState) {
 	highlighted := line.HighlightedTokens(p.linePrefix, p.searchPattern, &lineNumber)
 	var wrapped [][]twin.Cell
 	overflow := didFit
 	if p.WrapLongLines {
 		width, _ := p.screen.Size()
-		wrapped = wrapLine(width-p.numberPrefixLength(), highlighted.Cells)
+		wrapped = wrapLine(width-numberPrefixLength(p, scrollPosition), highlighted.Cells)
 	} else {
 		// All on one line
 		wrapped = [][]twin.Cell{highlighted.Cells}
@@ -226,7 +226,7 @@ func (p *Pager) renderLine(line *Line, lineNumber int) ([]renderedLine, overflow
 			visibleLineNumber = nil
 		}
 
-		decorated, localOverflow := p.decorateLine(visibleLineNumber, inputLinePart)
+		decorated, localOverflow := p.decorateLine(visibleLineNumber, inputLinePart, scrollPosition)
 		if localOverflow == didOverflow {
 			overflow = didOverflow
 		}
@@ -251,10 +251,10 @@ func (p *Pager) renderLine(line *Line, lineNumber int) ([]renderedLine, overflow
 // * Line number, or leading whitespace for wrapped lines
 // * Scroll left indicator
 // * Scroll right indicator
-func (p *Pager) decorateLine(lineNumberToShow *int, contents []twin.Cell) ([]twin.Cell, overflowState) {
+func (p *Pager) decorateLine(lineNumberToShow *int, contents []twin.Cell, scrollPosition scrollPositionInternal) ([]twin.Cell, overflowState) {
 	width, _ := p.screen.Size()
 	newLine := make([]twin.Cell, 0, width)
-	numberPrefixLength := p.numberPrefixLength()
+	numberPrefixLength := numberPrefixLength(p, scrollPosition)
 	newLine = append(newLine, createLinePrefix(lineNumberToShow, numberPrefixLength)...)
 	overflow := didFit
 

m/screenLines_test.go

@@ -19,7 +19,7 @@ func testHorizontalCropping(t *testing.T, contents string, firstIndex int, lastI
 	pager.scrollPosition = newScrollPosition("testHorizontalCropping")
 
 	lineContents := NewLine(contents)
-	screenLine, didOverflow := pager.renderLine(&lineContents, 0)
+	screenLine, didOverflow := pager.renderLine(&lineContents, 0, pager.scrollPosition.internalDontTouch)
 	assert.Equal(t, rowToString(screenLine[0].cells), expected)
 	assert.Equal(t, didOverflow, expectedOverflow)
 }
@@ -75,7 +75,7 @@ func TestSearchHighlight(t *testing.T) {
 		searchPattern: regexp.MustCompile("\""),
 	}
 
-	rendered, overflow := pager.renderLine(&line, 1)
+	rendered, overflow := pager.renderLine(&line, 1, pager.scrollPosition.internalDontTouch)
 	assert.DeepEqual(t, []renderedLine{
 		{
 			inputLineOneBased: 1,

m/scrollPosition.go

@@ -97,7 +97,7 @@ func (si *scrollPositionInternal) handleNegativeDeltaScreenLines(pager *Pager) {
 	for si.lineNumberOneBased > 1 && si.deltaScreenLines < 0 {
 		// Render the previous line
 		previousLine := pager.reader.GetLine(si.lineNumberOneBased - 1)
-		previousSubLines, _ := pager.renderLine(previousLine, 0)
+		previousSubLines, _ := pager.renderLine(previousLine, 0, *si)
 
 		// Adjust lineNumberOneBased and deltaScreenLines to move up into the
 		// previous screen line
@@ -127,14 +127,14 @@ func (si *scrollPositionInternal) handlePositiveDeltaScreenLines(pager *Pager) {
 			if line == nil {
 				panic(fmt.Errorf("Last line is nil"))
 			}
-			subLines, _ := pager.renderLine(line, 0)
+			subLines, _ := pager.renderLine(line, 0, *si)
 
 			// ... and go to the bottom of that.
 			si.deltaScreenLines = len(subLines) - 1
 			return
 		}
 
-		subLines, _ := pager.renderLine(line, 0)
+		subLines, _ := pager.renderLine(line, 0, *si)
 		if si.deltaScreenLines < len(subLines) {
 			// Sublines are within bounds!
 			return
@@ -165,7 +165,7 @@ func (si *scrollPositionInternal) emptyBottomLinesCount(pager *Pager) int {
 			break
 		}
 
-		subLines, _ := pager.renderLine(line, lineNumberOneBased)
+		subLines, _ := pager.renderLine(line, lineNumberOneBased, *si)
 		unclaimedViewportLines -= len(subLines)
 		if unclaimedViewportLines <= 0 {
 			return 0
@@ -337,7 +337,7 @@ func (p *Pager) isScrolledToEnd() bool {
 	// Last line is on screen, now we need to figure out whether we can see all
 	// of it
 	lastInputLine := p.reader.GetLine(lastInputLineNumberOneBased)
-	lastInputLineRendered, _ := p.renderLine(lastInputLine, lastInputLineNumberOneBased)
+	lastInputLineRendered, _ := p.renderLine(lastInputLine, lastInputLineNumberOneBased, p.scrollPosition.internalDontTouch)
 	lastRenderedSubLine := lastInputLineRendered[len(lastInputLineRendered)-1]
 
 	// If the last visible subline is the same as the last possible subline then
@@ -362,25 +362,30 @@ func (p *Pager) getLastVisiblePosition() *scrollPosition {
 	}
 }
 
-func (p *Pager) numberPrefixLength() int {
+func numberPrefixLength(pager *Pager, scrollPosition scrollPositionInternal) int {
 	// This method used to live in screenLines.go, but I moved it here because
 	// it touches scroll position internals.
-	if !p.ShowLineNumbers {
+
+	if !pager.ShowLineNumbers {
 		return 0
 	}
 
-	_, height := p.screen.Size()
-	contentHeight := height - 1 // Full screen height minus the status bar
-	maxPossibleLineNumber := p.reader.GetLineCount()
+	_, height := pager.screen.Size()
+	contentHeight := height
+	if pager.ShowStatusBar {
+		contentHeight--
+	}
+	maxPossibleLineNumber := pager.reader.GetLineCount()
 
 	// This is an approximation assuming we don't do any wrapping. Finding the
 	// real answer while wrapping requires rendering, which requires the real
 	// answer and so on, so we do an approximation here to save us from
 	// recursion.
 	//
 	// Let's improve on demand.
-	maxVisibleLineNumber := (p.scrollPosition.internalDontTouch.lineNumberOneBased +
-		p.scrollPosition.internalDontTouch.deltaScreenLines + contentHeight - 1)
+	maxVisibleLineNumber := (scrollPosition.lineNumberOneBased +
+		scrollPosition.deltaScreenLines +
+		contentHeight - 1)
 	if maxVisibleLineNumber > maxPossibleLineNumber {
 		maxVisibleLineNumber = maxPossibleLineNumber
 	}

m/scrollPosition_test.go

@@ -0,0 +1,55 @@
+package m
+
+import (
+	"fmt"
+	"strings"
+	"testing"
+
+	"github.com/walles/moar/twin"
+	"gotest.tools/v3/assert"
+)
+
+const screenHeight = 60
+
+// Repro for: https://github.com/walles/moar/issues/166
+func testCanonicalize1000(t *testing.T, withStatusBar bool, currentStartLine int, lastVisibleLine int) {
+	pager := Pager{}
+	pager.screen = twin.NewFakeScreen(100, screenHeight)
+	pager.reader = NewReaderFromText("test", strings.Repeat("a\n", 2000))
+	pager.ShowLineNumbers = true
+	pager.ShowStatusBar = withStatusBar
+	pager.scrollPosition = scrollPosition{
+		internalDontTouch: scrollPositionInternal{
+			lineNumberOneBased: currentStartLine,
+			deltaScreenLines:   0,
+			name:               "findFirstHit",
+			canonicalizing:     false,
+		},
+	}
+
+	lastVisiblePosition := scrollPosition{
+		internalDontTouch: scrollPositionInternal{
+			lineNumberOneBased: lastVisibleLine,
+			deltaScreenLines:   0,
+			name:               "Last Visible Position",
+		},
+	}
+
+	assert.Equal(t, lastVisiblePosition.lineNumberOneBased(&pager), lastVisibleLine)
+}
+
+func TestCanonicalize1000WithStatusBar(t *testing.T) {
+	for startLine := 0; startLine < 1500; startLine++ {
+		t.Run(fmt.Sprint("startLine=", startLine), func(t *testing.T) {
+			testCanonicalize1000(t, true, startLine, startLine+screenHeight-2)
+		})
+	}
+}
+
+func TestCanonicalize1000WithoutStatusBar(t *testing.T) {
+	for startLine := 0; startLine < 1500; startLine++ {
+		t.Run(fmt.Sprint("startLine=", startLine), func(t *testing.T) {
+			testCanonicalize1000(t, false, startLine, startLine+screenHeight-1)
+		})
+	}
+}