Merge branch 'walles/test-line-creation'

This fixes highlighting in the presence of unicode chars.

README.md

@@ -102,8 +102,6 @@ Execute `release.sh` and follow instructions.
 TODO
 ----
 
-* Showing unicode search hits should highlight the correct chars
-
 * Searching for something above us should wrap the search.
 
 * Enable exiting using ^c (without restoring the screen).
@@ -163,3 +161,5 @@ Done
 
 * Make `tail -f /dev/null` exit properly, fix
   <https://github.com/walles/moar/issues/7>.
+
+* Showing unicode search hits should highlight the correct chars

m/matchRanges.go

@@ -4,20 +4,65 @@ import "regexp"
 
 // MatchRanges collects match indices
 type MatchRanges struct {
-	Matches [][]int
+	Matches [][2]int
 }
 
-// GetMatchRanges locates a regexp in a string
+// GetMatchRanges locates one or more regexp matches in a string
 func GetMatchRanges(String *string, Pattern *regexp.Regexp) *MatchRanges {
 	if Pattern == nil {
 		return nil
 	}
 
 	return &MatchRanges{
-		Matches: Pattern.FindAllStringIndex(*String, -1),
+		Matches: _ToRunePositions(Pattern.FindAllStringIndex(*String, -1), String),
 	}
 }
 
+// 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})
+
+			fromRune = -1
+
+			if len(returnMe) >= len(byteIndices) {
+				// No more byte indices
+				break
+			}
+
+			fromByte = byteIndices[len(returnMe)][0]
+			toByte = byteIndices[len(returnMe)][1]
+		}
+
+		runePosition++
+	}
+
+	if fromRune != -1 {
+		toRune := runePosition
+		returnMe = append(returnMe, [2]int{fromRune, toRune})
+	}
+
+	return returnMe
+}
+
 // InRange says true if the index is part of a regexp match
 func (mr *MatchRanges) InRange(index int) bool {
 	if mr == nil {

m/matchRanges_test.go

@@ -39,3 +39,37 @@ func TestInRange(t *testing.T) {
 	assert.Assert(t, !matchRanges.InRange(4))  // a
 	assert.Assert(t, !matchRanges.InRange(5))  // After end
 }
+
+func TestUtf8(t *testing.T) {
+	// This test verifies that the match ranges are by rune rather than by byte
+	unicodes := "-ä-ä-"
+	matchRanges := GetMatchRanges(&unicodes, regexp.MustCompile("ä"))
+
+	assert.Assert(t, !matchRanges.InRange(0)) // -
+	assert.Assert(t, matchRanges.InRange(1))  // ä
+	assert.Assert(t, !matchRanges.InRange(2)) // -
+	assert.Assert(t, matchRanges.InRange(3))  // ä
+	assert.Assert(t, !matchRanges.InRange(4)) // -
+}
+
+func TestNoMatch(t *testing.T) {
+	// This test verifies that the match ranges are by rune rather than by byte
+	unicodes := "gris"
+	matchRanges := GetMatchRanges(&unicodes, regexp.MustCompile("apa"))
+
+	assert.Assert(t, !matchRanges.InRange(0))
+	assert.Assert(t, !matchRanges.InRange(1))
+	assert.Assert(t, !matchRanges.InRange(2))
+	assert.Assert(t, !matchRanges.InRange(3))
+	assert.Assert(t, !matchRanges.InRange(4))
+}
+
+func TestEndMatch(t *testing.T) {
+	// This test verifies that the match ranges are by rune rather than by byte
+	unicodes := "-ä"
+	matchRanges := GetMatchRanges(&unicodes, regexp.MustCompile("ä"))
+
+	assert.Assert(t, !matchRanges.InRange(0)) // -
+	assert.Assert(t, matchRanges.InRange(1))  // ä
+	assert.Assert(t, !matchRanges.InRange(2)) // Past the end
+}

m/pager.go

@@ -99,45 +99,63 @@ func NewPager(r *Reader) *Pager {
 }
 
 func (p *Pager) _AddLine(logger *log.Logger, lineNumber int, line string) {
-	pos := 0
-	stringIndexAtColumnZero := p.leftColumnZeroBased
-	if p.leftColumnZeroBased > 0 {
-		// Indicate that it's possible to scroll left
-		p.screen.SetContent(pos, lineNumber, '<', nil, tcell.StyleDefault.Reverse(true))
-		pos++
-
-		// This code can be verified by searching for "monkeys" in
-		// sample-files/long-and-wide.txt and scrolling right. If the
-		// "monkeys" highlight is in the right place both before and
-		// after scrolling right then this code is good.
-		stringIndexAtColumnZero--
+	width, _ := p.screen.Size()
+	tokens := _CreateScreenLine(logger, lineNumber, p.leftColumnZeroBased, width, line, p.searchPattern)
+	for column, token := range tokens {
+		p.screen.SetContent(column, lineNumber, token.Rune, nil, token.Style)
 	}
+}
 
-	tokens, plainString := TokensFromString(logger, line)
-	if p.leftColumnZeroBased >= len(tokens) {
-		// Nothing to display, never mind
-		return
+func _CreateScreenLine(
+	logger *log.Logger,
+	lineNumber int,
+	stringIndexAtColumnZero int,
+	screenColumnsCount int,
+	line string,
+	search *regexp.Regexp,
+) []Token {
+	var returnMe []Token
+	searchHitDelta := 0
+	if stringIndexAtColumnZero > 0 {
+		// Indicate that it's possible to scroll left
+		returnMe = append(returnMe, Token{
+			Rune:  '<',
+			Style: tcell.StyleDefault.Reverse(true),
+		})
+		searchHitDelta = -1
 	}
 
-	matchRanges := GetMatchRanges(plainString, p.searchPattern)
-	for _, token := range tokens[p.leftColumnZeroBased:] {
-		width, _ := p.screen.Size()
-		if pos >= width {
-			// Indicate that this line continues to the right
-			p.screen.SetContent(pos-1, lineNumber, '>', nil, tcell.StyleDefault.Reverse(true))
+	tokens, plainString := TokensFromString(logger, line)
+	if stringIndexAtColumnZero >= len(tokens) {
+		// Nothing (more) to display, never mind
+		return returnMe
+	}
+
+	matchRanges := GetMatchRanges(plainString, search)
+	for _, token := range tokens[stringIndexAtColumnZero:] {
+		if len(returnMe) >= screenColumnsCount {
+			// We are trying to add a character to the right of the screen.
+			// Indicate that this line continues to the right.
+			returnMe[len(returnMe)-1] = Token{
+				Rune:  '>',
+				Style: tcell.StyleDefault.Reverse(true),
+			}
 			break
 		}
 
 		style := token.Style
-		if matchRanges.InRange(pos + stringIndexAtColumnZero) {
+		if matchRanges.InRange(len(returnMe) + stringIndexAtColumnZero + searchHitDelta) {
 			// Search hits in reverse video
 			style = style.Reverse(true)
 		}
 
-		p.screen.SetContent(pos, lineNumber, token.Rune, nil, style)
-
-		pos++
+		returnMe = append(returnMe, Token{
+			Rune:  token.Rune,
+			Style: style,
+		})
 	}
+
+	return returnMe
 }
 
 func (p *Pager) _AddSearchFooter() {

m/pager_test.go

@@ -3,6 +3,7 @@ package m
 import (
 	"log"
 	"os"
+	"regexp"
 	"runtime"
 	"strings"
 	"testing"
@@ -17,7 +18,7 @@ func TestUnicodeRendering(t *testing.T) {
 		panic(err)
 	}
 
-	var answers = []_ExpectedCell{
+	var answers = []Token{
 		_CreateExpectedCell('å', tcell.StyleDefault),
 		_CreateExpectedCell('ä', tcell.StyleDefault),
 		_CreateExpectedCell('ö', tcell.StyleDefault),
@@ -29,12 +30,7 @@ func TestUnicodeRendering(t *testing.T) {
 	}
 }
 
-type _ExpectedCell struct {
-	Rune  rune
-	Style tcell.Style
-}
-
-func (expected _ExpectedCell) LogDifference(t *testing.T, actual tcell.SimCell) {
+func (expected Token) LogDifference(t *testing.T, actual tcell.SimCell) {
 	if actual.Runes[0] == expected.Rune && actual.Style == expected.Style {
 		return
 	}
@@ -44,8 +40,8 @@ func (expected _ExpectedCell) LogDifference(t *testing.T, actual tcell.SimCell)
 		string(actual.Runes[0]), actual.Style)
 }
 
-func _CreateExpectedCell(Rune rune, Style tcell.Style) _ExpectedCell {
-	return _ExpectedCell{
+func _CreateExpectedCell(Rune rune, Style tcell.Style) Token {
+	return Token{
 		Rune:  Rune,
 		Style: Style,
 	}
@@ -58,7 +54,7 @@ func TestFgColorRendering(t *testing.T) {
 		panic(err)
 	}
 
-	var answers = []_ExpectedCell{
+	var answers = []Token{
 		_CreateExpectedCell('a', tcell.StyleDefault.Foreground(0)),
 		_CreateExpectedCell('b', tcell.StyleDefault.Foreground(1)),
 		_CreateExpectedCell('c', tcell.StyleDefault.Foreground(2)),
@@ -84,7 +80,7 @@ func TestBrokenUtf8(t *testing.T) {
 		panic(err)
 	}
 
-	var answers = []_ExpectedCell{
+	var answers = []Token{
 		_CreateExpectedCell('a', tcell.StyleDefault),
 		_CreateExpectedCell('b', tcell.StyleDefault),
 		_CreateExpectedCell('c', tcell.StyleDefault),
@@ -179,7 +175,7 @@ func TestCodeHighlighting(t *testing.T) {
 		panic(err)
 	}
 
-	var answers = []_ExpectedCell{
+	var answers = []Token{
 		_CreateExpectedCell('p', tcell.StyleDefault.Foreground(3)),
 		_CreateExpectedCell('a', tcell.StyleDefault.Foreground(3)),
 		_CreateExpectedCell('c', tcell.StyleDefault.Foreground(3)),
@@ -197,7 +193,7 @@ func TestCodeHighlighting(t *testing.T) {
 	}
 }
 
-func _TestManPageFormatting(t *testing.T, input string, expected _ExpectedCell) {
+func _TestManPageFormatting(t *testing.T, input string, expected Token) {
 	reader := NewReaderFromStream(strings.NewReader(input), nil)
 	if err := reader._Wait(); err != nil {
 		panic(err)
@@ -245,3 +241,104 @@ func TestToPattern(t *testing.T) {
 	assert.Assert(t, ToPattern(")g").MatchString(")G"))
 	assert.Assert(t, ToPattern(")g").MatchString(")g"))
 }
+
+func assertTokenRangesEqual(t *testing.T, actual []Token, expected []Token) {
+	if len(actual) != len(expected) {
+		t.Errorf("String lengths mismatch; expected %d but got %d",
+			len(expected), len(actual))
+	}
+
+	for pos, expectedToken := range expected {
+		if pos >= len(expected) || pos >= len(actual) {
+			break
+		}
+
+		actualToken := actual[pos]
+		if actualToken.Rune == expectedToken.Rune && actualToken.Style == expectedToken.Style {
+			// Actual == Expected, keep checking
+			continue
+		}
+
+		t.Errorf("At (0-based) position %d: Expected '%s'/0x%x, got '%s'/0x%x",
+			pos,
+			string(expectedToken.Rune), expectedToken.Style,
+			string(actualToken.Rune), actualToken.Style)
+	}
+}
+
+func TestCreateScreenLineBase(t *testing.T) {
+	line := _CreateScreenLine(nil, 0, 0, 3, "", nil)
+	assert.Assert(t, len(line) == 0)
+}
+
+func TestCreateScreenLineOverflowRight(t *testing.T) {
+	line := _CreateScreenLine(nil, 0, 0, 3, "012345", nil)
+	assertTokenRangesEqual(t, line, []Token{
+		_CreateExpectedCell('0', tcell.StyleDefault),
+		_CreateExpectedCell('1', tcell.StyleDefault),
+		_CreateExpectedCell('>', tcell.StyleDefault.Reverse(true)),
+	})
+}
+
+func TestCreateScreenLineUnderflowLeft(t *testing.T) {
+	line := _CreateScreenLine(nil, 0, 1, 3, "012", nil)
+	assertTokenRangesEqual(t, line, []Token{
+		_CreateExpectedCell('<', tcell.StyleDefault.Reverse(true)),
+		_CreateExpectedCell('1', tcell.StyleDefault),
+		_CreateExpectedCell('2', tcell.StyleDefault),
+	})
+}
+
+func TestCreateScreenLineSearchHit(t *testing.T) {
+	pattern, err := regexp.Compile("b")
+	if err != nil {
+		panic(err)
+	}
+
+	line := _CreateScreenLine(nil, 0, 0, 3, "abc", pattern)
+	assertTokenRangesEqual(t, line, []Token{
+		_CreateExpectedCell('a', tcell.StyleDefault),
+		_CreateExpectedCell('b', tcell.StyleDefault.Reverse(true)),
+		_CreateExpectedCell('c', tcell.StyleDefault),
+	})
+}
+
+func TestCreateScreenLineUtf8SearchHit(t *testing.T) {
+	pattern, err := regexp.Compile("ä")
+	if err != nil {
+		panic(err)
+	}
+
+	line := _CreateScreenLine(nil, 0, 0, 3, "åäö", pattern)
+	assertTokenRangesEqual(t, line, []Token{
+		_CreateExpectedCell('å', tcell.StyleDefault),
+		_CreateExpectedCell('ä', tcell.StyleDefault.Reverse(true)),
+		_CreateExpectedCell('ö', tcell.StyleDefault),
+	})
+}
+
+func TestCreateScreenLineScrolledUtf8SearchHit(t *testing.T) {
+	pattern := regexp.MustCompile("ä")
+
+	line := _CreateScreenLine(nil, 0, 1, 4, "ååäö", pattern)
+
+	assertTokenRangesEqual(t, line, []Token{
+		_CreateExpectedCell('<', tcell.StyleDefault.Reverse(true)),
+		_CreateExpectedCell('å', tcell.StyleDefault),
+		_CreateExpectedCell('ä', tcell.StyleDefault.Reverse(true)),
+		_CreateExpectedCell('ö', tcell.StyleDefault),
+	})
+}
+
+func TestCreateScreenLineScrolled2Utf8SearchHit(t *testing.T) {
+	pattern := regexp.MustCompile("ä")
+
+	line := _CreateScreenLine(nil, 0, 2, 4, "åååäö", pattern)
+
+	assertTokenRangesEqual(t, line, []Token{
+		_CreateExpectedCell('<', tcell.StyleDefault.Reverse(true)),
+		_CreateExpectedCell('å', tcell.StyleDefault),
+		_CreateExpectedCell('ä', tcell.StyleDefault.Reverse(true)),
+		_CreateExpectedCell('ö', tcell.StyleDefault),
+	})
+}