Add recursion guard to canonicalize()

m/pager.go

@@ -127,11 +127,18 @@ Available at https://github.com/walles/moar/.
 
 // NewPager creates a new Pager
 func NewPager(r *Reader) *Pager {
+	var name string
+	if r == nil || r.name == nil || len(*r.name) == 0 {
+		name = "Pager"
+	} else {
+		name = "Pager " + *r.name
+	}
 	return &Pager{
 		reader:          r,
 		quit:            false,
 		ShowLineNumbers: true,
 		DeInit:          true,
+		scrollPosition:  newScrollPosition(name),
 	}
 }
 
@@ -219,7 +226,7 @@ func (p *Pager) findFirstHit(startPosition scrollPosition, backwards bool) *scro
 
 		lineText := line.Plain()
 		if p.searchPattern.MatchString(lineText) {
-			return scrollPositionFromLineNumber(searchPosition.lineNumberOneBased(p))
+			return scrollPositionFromLineNumber("findFirstHit", searchPosition.lineNumberOneBased(p))
 		}
 
 		if backwards {
@@ -251,7 +258,7 @@ func (p *Pager) scrollToNextSearchHit() {
 	case _NotFound:
 		// Restart searching from the top
 		p.mode = _Viewing
-		firstSearchPosition = scrollPosition{}
+		firstSearchPosition = newScrollPosition("firstSearchPosition")
 
 	default:
 		panic(fmt.Sprint("Unknown search mode when finding next: ", p.mode))
@@ -447,7 +454,7 @@ func (p *Pager) onKey(keyCode twin.KeyCode) {
 		p.moveRight(-16)
 
 	case twin.KeyHome:
-		p.scrollPosition = scrollPosition{}
+		p.scrollPosition = newScrollPosition("Pager scroll position")
 
 	case twin.KeyEnd:
 		p.scrollToEnd()
@@ -491,7 +498,7 @@ func (p *Pager) onRune(char rune) {
 				leftColumnZeroBased: p.leftColumnZeroBased,
 			}
 			p.reader = _HelpReader
-			p.scrollPosition = scrollPosition{}
+			p.scrollPosition = newScrollPosition("Pager scroll position")
 			p.leftColumnZeroBased = 0
 			p.isShowingHelp = true
 		}
@@ -513,7 +520,7 @@ func (p *Pager) onRune(char rune) {
 		p.moveRight(-16)
 
 	case '<', 'g':
-		p.scrollPosition = scrollPosition{}
+		p.scrollPosition = newScrollPosition("Pager scroll position")
 
 	case '>', 'G':
 		p.scrollToEnd()

m/pager_test.go

@@ -249,7 +249,7 @@ func TestFindFirstHitSimple(t *testing.T) {
 
 	pager.searchPattern = toPattern("AB")
 
-	hit := pager.findFirstHit(scrollPosition{}, false)
+	hit := pager.findFirstHit(newScrollPosition("TestFindFirstHitSimple"), false)
 	assert.Equal(t, hit.internalDontTouch.lineNumberOneBased, 1)
 	assert.Equal(t, hit.internalDontTouch.deltaScreenLines, 0)
 }
@@ -263,7 +263,7 @@ func TestFindFirstHitAnsi(t *testing.T) {
 
 	pager.searchPattern = toPattern("AB")
 
-	hit := pager.findFirstHit(scrollPosition{}, false)
+	hit := pager.findFirstHit(newScrollPosition("TestFindFirstHitSimple"), false)
 	assert.Equal(t, hit.internalDontTouch.lineNumberOneBased, 1)
 	assert.Equal(t, hit.internalDontTouch.deltaScreenLines, 0)
 }
@@ -405,7 +405,7 @@ func benchmarkSearch(b *testing.B, highlighted bool) {
 	b.ResetTimer()
 
 	// This test will search through all the N copies we made of our file
-	hit := pager.findFirstHit(scrollPosition{}, false)
+	hit := pager.findFirstHit(newScrollPosition("benchmarkSearch"), false)
 
 	if hit != nil {
 		panic(fmt.Errorf("This test is meant to scan the whole file without finding anything"))

m/screenLines_test.go

@@ -12,6 +12,7 @@ func testHorizontalCropping(t *testing.T, contents string, firstIndex int, lastI
 	pager := Pager{
 		screen:              twin.NewFakeScreen(1+lastIndex-firstIndex, 99),
 		leftColumnZeroBased: firstIndex,
+		scrollPosition:      newScrollPosition("testHorizontalCropping"),
 	}
 	lineContents := NewLine(contents)
 	screenLine := pager.renderLine(lineContents, 0)
@@ -48,6 +49,8 @@ func TestEmpty(t *testing.T) {
 
 		// No lines available
 		reader: NewReaderFromText("test", ""),
+
+		scrollPosition: newScrollPosition("TestEmpty"),
 	}
 
 	rendered, statusText := pager.renderScreenLines()
@@ -67,7 +70,7 @@ func TestOverflowDown(t *testing.T) {
 		reader: NewReaderFromText("test", "hej"),
 
 		// This value can be anything and should be clipped, that's what we're testing
-		scrollPosition: *scrollPositionFromLineNumber(42),
+		scrollPosition: *scrollPositionFromLineNumber("TestOverflowDown", 42),
 	}
 
 	rendered, statusText := pager.renderScreenLines()

m/scrollPosition.go

@@ -2,10 +2,22 @@ package m
 
 import "fmt"
 
+// Please create using newScrollPosition(name)
 type scrollPosition struct {
 	internalDontTouch scrollPositionInternal
 }
 
+func newScrollPosition(name string) scrollPosition {
+	if len(name) == 0 {
+		panic("Non-empty name required")
+	}
+	return scrollPosition{
+		internalDontTouch: scrollPositionInternal{
+			name: name,
+		},
+	}
+}
+
 type scrollPositionInternal struct {
 	// Line number in the input stream
 	lineNumberOneBased int
@@ -13,7 +25,9 @@ type scrollPositionInternal struct {
 	// Scroll this many screen lines before rendering. Can be negative.
 	deltaScreenLines int
 
-	canonical scrollPositionCanonical
+	name           string
+	canonicalizing bool
+	canonical      scrollPositionCanonical
 }
 
 // If any of these change, we have to recompute the scrollPositionInternal values
@@ -44,6 +58,7 @@ func canonicalFromPager(pager *Pager) scrollPositionCanonical {
 func (s scrollPosition) PreviousLine(scrollDistance int) scrollPosition {
 	return scrollPosition{
 		internalDontTouch: scrollPositionInternal{
+			name:               s.internalDontTouch.name,
 			lineNumberOneBased: s.internalDontTouch.lineNumberOneBased,
 			deltaScreenLines:   s.internalDontTouch.deltaScreenLines - scrollDistance,
 		},
@@ -54,6 +69,7 @@ func (s scrollPosition) PreviousLine(scrollDistance int) scrollPosition {
 func (s scrollPosition) NextLine(scrollDistance int) scrollPosition {
 	return scrollPosition{
 		internalDontTouch: scrollPositionInternal{
+			name:               s.internalDontTouch.name,
 			lineNumberOneBased: s.internalDontTouch.lineNumberOneBased,
 			deltaScreenLines:   s.internalDontTouch.deltaScreenLines + scrollDistance,
 		},
@@ -142,8 +158,15 @@ func (si *scrollPositionInternal) canonicalize(pager *Pager) {
 	if si.canonical == canonicalFromPager(pager) {
 		return
 	}
+
+	if si.canonicalizing {
+		panic(fmt.Errorf("Scroll position canonicalize() called recursively for %s", si.name))
+	}
+	si.canonicalizing = true
+
 	defer func() {
 		si.canonical = canonicalFromPager(pager)
+		si.canonicalizing = false
 	}()
 
 	if pager.reader.GetLineCount() == 0 {
@@ -164,9 +187,10 @@ func (si *scrollPositionInternal) canonicalize(pager *Pager) {
 	}
 }
 
-func scrollPositionFromLineNumber(lineNumberOneBased int) *scrollPosition {
+func scrollPositionFromLineNumber(name string, lineNumberOneBased int) *scrollPosition {
 	return &scrollPosition{
 		internalDontTouch: scrollPositionInternal{
+			name:               name,
 			lineNumberOneBased: lineNumberOneBased,
 		},
 	}
@@ -218,6 +242,7 @@ func (p *Pager) getLastVisiblePosition() *scrollPosition {
 	lastRenderedLine := renderedLines[len(renderedLines)-1]
 	return &scrollPosition{
 		internalDontTouch: scrollPositionInternal{
+			name:               "Last Visible Position",
 			lineNumberOneBased: lastRenderedLine.inputLineOneBased,
 			deltaScreenLines:   lastRenderedLine.wrapIndex,
 		},