From ab0e62123b01103390424a9a917331c142a26748 Mon Sep 17 00:00:00 2001 From: Johan Walles Date: Tue, 5 Nov 2019 21:09:45 +0100 Subject: [PATCH 1/9] WIP: Line rendering tests Implement search hits tests as well. --- m/pager.go | 68 ++++++++++++++++++++++++++++------------------ m/pager_test.go | 72 ++++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 101 insertions(+), 39 deletions(-) diff --git a/m/pager.go b/m/pager.go index 36aad7ca..95f84c57 100644 --- a/m/pager.go +++ b/m/pager.go @@ -99,45 +99,61 @@ 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 + if stringIndexAtColumnZero > 0 { + // Indicate that it's possible to scroll left + returnMe = append(returnMe, Token{ + Rune: '<', + Style: tcell.StyleDefault.Reverse(true), + }) } - 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) { // 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() { diff --git a/m/pager_test.go b/m/pager_test.go index 9c74a766..5079e948 100644 --- a/m/pager_test.go +++ b/m/pager_test.go @@ -17,7 +17,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 +29,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 +39,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 +53,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 +79,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 +174,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 +192,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 +240,54 @@ 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) { + assert.Assert(t, len("Unimplemented") == 0) +} From 81fe99f366559158672be0652f2c3b24ae277330 Mon Sep 17 00:00:00 2001 From: Johan Walles Date: Tue, 5 Nov 2019 21:16:56 +0100 Subject: [PATCH 2/9] WIP: Test search highlights --- m/pager_test.go | 42 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/m/pager_test.go b/m/pager_test.go index 5079e948..932dcb5c 100644 --- a/m/pager_test.go +++ b/m/pager_test.go @@ -3,6 +3,7 @@ package m import ( "log" "os" + "regexp" "runtime" "strings" "testing" @@ -289,5 +290,44 @@ func TestCreateScreenLineUnderflowLeft(t *testing.T) { } func TestCreateScreenLineSearchHit(t *testing.T) { - assert.Assert(t, len("Unimplemented") == 0) + 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, err := regexp.Compile("ä") + if err != nil { + panic(err) + } + + 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), + }) } From 109f40e7ede716d911f4fecfdf128bb3d1f5bbfc Mon Sep 17 00:00:00 2001 From: Johan Walles Date: Tue, 5 Nov 2019 21:30:04 +0100 Subject: [PATCH 3/9] Prettify --- m/pager_test.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/m/pager_test.go b/m/pager_test.go index 932dcb5c..42900c77 100644 --- a/m/pager_test.go +++ b/m/pager_test.go @@ -318,12 +318,10 @@ func TestCreateScreenLineUtf8SearchHit(t *testing.T) { } func TestCreateScreenLineScrolledUtf8SearchHit(t *testing.T) { - pattern, err := regexp.Compile("ä") - if err != nil { - panic(err) - } + pattern := regexp.MustCompile("ä") line := _CreateScreenLine(nil, 0, 1, 4, "ååäö", pattern) + assertTokenRangesEqual(t, line, []Token{ _CreateExpectedCell('<', tcell.StyleDefault.Reverse(true)), _CreateExpectedCell('å', tcell.StyleDefault), From df1702ac217d07b53bf1fea51e03a7d92a5ea781 Mon Sep 17 00:00:00 2001 From: Johan Walles Date: Wed, 6 Nov 2019 06:50:53 +0100 Subject: [PATCH 4/9] Test that match ranges are by rune not by byte --- m/matchRanges_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/m/matchRanges_test.go b/m/matchRanges_test.go index 3d99e4f5..cef0c500 100644 --- a/m/matchRanges_test.go +++ b/m/matchRanges_test.go @@ -39,3 +39,15 @@ 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)) // - +} From 494032278b9fd80b30c5a33988e2708bba5c9f39 Mon Sep 17 00:00:00 2001 From: Johan Walles Date: Wed, 6 Nov 2019 18:50:34 +0100 Subject: [PATCH 5/9] Fix some failing tests --- m/matchRanges.go | 46 +++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 43 insertions(+), 3 deletions(-) diff --git a/m/matchRanges.go b/m/matchRanges.go index 46da9d25..aae381f7 100644 --- a/m/matchRanges.go +++ b/m/matchRanges.go @@ -4,20 +4,60 @@ 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 + + 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 { From fb3b6abbe7b940e96631579c0f9eb9c9d22bfbe0 Mon Sep 17 00:00:00 2001 From: Johan Walles Date: Wed, 6 Nov 2019 20:30:59 +0100 Subject: [PATCH 6/9] Pass the tests But crash the app. Do: * ./moar.sh sample-files/long-and-wide.txt * Search (/) for "monkey" --- m/pager.go | 4 +++- m/pager_test.go | 13 +++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/m/pager.go b/m/pager.go index 95f84c57..178c8283 100644 --- a/m/pager.go +++ b/m/pager.go @@ -115,12 +115,14 @@ func _CreateScreenLine( 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 } tokens, plainString := TokensFromString(logger, line) @@ -142,7 +144,7 @@ func _CreateScreenLine( } style := token.Style - if matchRanges.InRange(len(returnMe) + stringIndexAtColumnZero) { + if matchRanges.InRange(len(returnMe) + stringIndexAtColumnZero + searchHitDelta) { // Search hits in reverse video style = style.Reverse(true) } diff --git a/m/pager_test.go b/m/pager_test.go index 42900c77..5911cbc7 100644 --- a/m/pager_test.go +++ b/m/pager_test.go @@ -329,3 +329,16 @@ func TestCreateScreenLineScrolledUtf8SearchHit(t *testing.T) { _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), + }) +} From ea3dcf3ca5285b4d44bad9d9c449e0458b0671f4 Mon Sep 17 00:00:00 2001 From: Johan Walles Date: Wed, 6 Nov 2019 21:38:39 +0100 Subject: [PATCH 7/9] Fix a crash --- m/matchRanges.go | 5 +++++ m/matchRanges_test.go | 12 ++++++++++++ 2 files changed, 17 insertions(+) diff --git a/m/matchRanges.go b/m/matchRanges.go index aae381f7..6ac64559 100644 --- a/m/matchRanges.go +++ b/m/matchRanges.go @@ -24,6 +24,11 @@ func _ToRunePositions(byteIndices [][]int, matchedString *string) [][2]int { 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 diff --git a/m/matchRanges_test.go b/m/matchRanges_test.go index cef0c500..574ee87e 100644 --- a/m/matchRanges_test.go +++ b/m/matchRanges_test.go @@ -51,3 +51,15 @@ func TestUtf8(t *testing.T) { 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)) +} From ad4c545df006ca0aefdc26109c9fe799d48a2b07 Mon Sep 17 00:00:00 2001 From: Johan Walles Date: Wed, 6 Nov 2019 21:40:14 +0100 Subject: [PATCH 8/9] Add another match ranges test --- m/matchRanges_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/m/matchRanges_test.go b/m/matchRanges_test.go index 574ee87e..b702c161 100644 --- a/m/matchRanges_test.go +++ b/m/matchRanges_test.go @@ -63,3 +63,13 @@ func TestNoMatch(t *testing.T) { 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 +} From 9fcc2354308ed576254953600cb6e1227f617b8f Mon Sep 17 00:00:00 2001 From: Johan Walles Date: Wed, 6 Nov 2019 21:42:20 +0100 Subject: [PATCH 9/9] README: Unicode search hits done --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 9fdcf5ca..d31c82e0 100644 --- a/README.md +++ b/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 . + +* Showing unicode search hits should highlight the correct chars