Merge branch 'johan/fix-line-highlight'

m/ansiTokenizer.go

@@ -40,8 +40,6 @@ func NewLine(raw string) Line {
 // Returns a representation of the string split into styled tokens. Any regexp
 // matches are highlighted in inverse video. A nil regexp means no highlighting.
 func (line *Line) HighlightedTokens(search *regexp.Regexp) []twin.Cell {
-	searchHitDelta := 0
-
 	plain := line.Plain()
 	matchRanges := getMatchRanges(&plain, search)
 
@@ -49,7 +47,7 @@ func (line *Line) HighlightedTokens(search *regexp.Regexp) []twin.Cell {
 	returnMe := make([]twin.Cell, 0, len(cells))
 	for _, token := range cells {
 		style := token.Style
-		if matchRanges.InRange(len(returnMe) + searchHitDelta) {
+		if matchRanges.InRange(len(returnMe)) {
 			// Search hits in reverse video
 			style = style.WithAttr(twin.AttrReverse)
 		}

m/matchRanges.go

@@ -20,44 +20,29 @@ func getMatchRanges(String *string, Pattern *regexp.Regexp) *MatchRanges {
 
 // Convert byte indices to rune indices
 func toRunePositions(byteIndices [][]int, matchedString *string) [][2]int {
-	// FIXME: Will this function need to handle overlapping ranges?
-
 	var returnMe [][2]int
-
 	if len(byteIndices) == 0 {
 		// Nothing to see here, move along
 		return returnMe
 	}
 
-	fromByte := byteIndices[len(returnMe)][0]
-	toByte := byteIndices[len(returnMe)][1]
-	fromRune := -1
-	runePosition := 0
-	for bytePosition := range *matchedString {
-		if fromByte == bytePosition {
-			fromRune = runePosition
-		}
-		if toByte == bytePosition {
-			toRune := runePosition
-			returnMe = append(returnMe, [2]int{fromRune, toRune})
+	runeIndex := 0
+	byteIndicesToRuneIndices := make(map[int]int, 0)
+	for byteIndex := range *matchedString {
+		byteIndicesToRuneIndices[byteIndex] = runeIndex
 
-			fromRune = -1
-
-			if len(returnMe) >= len(byteIndices) {
-				// No more byte indices
-				break
-			}
-
-			fromByte = byteIndices[len(returnMe)][0]
-			toByte = byteIndices[len(returnMe)][1]
-		}
-
-		runePosition++
+		runeIndex++
 	}
 
-	if fromRune != -1 {
-		toRune := runePosition
-		returnMe = append(returnMe, [2]int{fromRune, toRune})
+	// If a match touches the end of the string, that will be encoded as one
+	// byte past the end of the string. Therefore we must add a mapping for
+	// first-index-after-the-end.
+	byteIndicesToRuneIndices[len(*matchedString)] = runeIndex
+
+	for _, bytePair := range byteIndices {
+		fromRuneIndex := byteIndicesToRuneIndices[bytePair[0]]
+		toRuneIndex := byteIndicesToRuneIndices[bytePair[1]]
+		returnMe = append(returnMe, [2]int{fromRuneIndex, toRuneIndex})
 	}
 
 	return returnMe

m/matchRanges_test.go

@@ -73,3 +73,17 @@ func TestEndMatch(t *testing.T) {
 	assert.Assert(t, matchRanges.InRange(1))  // ä
 	assert.Assert(t, !matchRanges.InRange(2)) // Past the end
 }
+
+func TestRealWorldBug(t *testing.T) {
+	// Verify a real world bug found in v1.9.8
+
+	testString := "anna"
+	matchRanges := getMatchRanges(&testString, regexp.MustCompile("n"))
+	assert.Equal(t, len(matchRanges.Matches), 2) // Two matches
+
+	assert.DeepEqual(t, matchRanges.Matches[0][0], 1) // First match starts at 1
+	assert.DeepEqual(t, matchRanges.Matches[0][1], 2) // And ends on 2 exclusive
+
+	assert.DeepEqual(t, matchRanges.Matches[1][0], 2) // Second match starts at 2
+	assert.DeepEqual(t, matchRanges.Matches[1][1], 3) // And ends on 3 exclusive
+}

m/screenLines_test.go

@@ -1,9 +1,11 @@
 package m
 
 import (
+	"regexp"
 	"strings"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
 	"github.com/walles/moar/twin"
 	"gotest.tools/v3/assert"
 )
@@ -61,6 +63,31 @@ func TestEmpty(t *testing.T) {
 	assert.Equal(t, pager.lineNumberOneBased(), 0)
 }
 
+// Repro case for a search bug discovered in v1.9.8.
+func TestSearchHighlight(t *testing.T) {
+	line := Line{
+		raw: "x\"\"x",
+	}
+	pager := Pager{
+		screen:        twin.NewFakeScreen(100, 10),
+		searchPattern: regexp.MustCompile("\""),
+	}
+
+	rendered := pager.renderLine(&line, 1)
+	assert.DeepEqual(t, []renderedLine{
+		{
+			inputLineOneBased: 1,
+			wrapIndex:         0,
+			cells: []twin.Cell{
+				{Rune: 'x', Style: twin.StyleDefault},
+				{Rune: '"', Style: twin.StyleDefault.WithAttr(twin.AttrReverse)},
+				{Rune: '"', Style: twin.StyleDefault.WithAttr(twin.AttrReverse)},
+				{Rune: 'x', Style: twin.StyleDefault},
+			},
+		},
+	}, rendered, cmp.AllowUnexported(twin.Style{}), cmp.AllowUnexported(renderedLine{}))
+}
+
 func TestOverflowDown(t *testing.T) {
 	pager := Pager{
 		screen: twin.NewFakeScreen(