Skip to content

Commit

Permalink
[chore] remove empty ansi sequences, improve tests
Browse files Browse the repository at this point in the history
  • Loading branch information
robinovitch61 committed Dec 22, 2024
1 parent 1f0f563 commit 69b9b55
Show file tree
Hide file tree
Showing 8 changed files with 458 additions and 342 deletions.
2 changes: 2 additions & 0 deletions internal/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ var AttemptUpdateSinceTimeInterval = 500 * time.Millisecond

var AnsiRegex = regexp.MustCompile("\x1b\\[[0-9;]*m")

var EmptySequenceRegex = regexp.MustCompile("\x1b\\[[0-9;]+m\x1b\\[m")

// LeftPageWidthFraction controls the width of the left page as a fraction of the terminal width
const LeftPageWidthFraction = 2. / 5.

Expand Down
17 changes: 11 additions & 6 deletions internal/filterable_viewport/filterable_viewport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"github.com/robinovitch61/kl/internal/filter"
"github.com/robinovitch61/kl/internal/keymap"
"github.com/robinovitch61/kl/internal/style"
"github.com/robinovitch61/kl/internal/util"
"regexp"
"strings"
"testing"
Expand Down Expand Up @@ -148,15 +149,19 @@ func TestFilterableViewport_FilterNoContext(t *testing.T) {

// check filter correctly identifies lines in view
lines := getTestLines(fv)
if lines[0] != "Test Header \x1b[48;2;225;225;225m \x1b[m\x1b[38;2;0;0;0;48;2;225;225;225m\x1b[38;2;0;0;0;48;2;225;225;225mfilter: \x1b[m\x1b[38;2;0;0;0;48;2;225;225;225mone\x1b[m\x1b[38;2;0;0;0;48;2;225;225;225m \x1b[m\x1b[38;2;0;0;0;48;2;225;225;225m(matches only) \x1b[m\x1b[m" {
t.Errorf("unexpected header with filter\n%q", fv.View())
}
util.CmpStr(
t,
"Test Header \x1b[48;2;225;225;225m \x1b[m\x1b[38;2;0;0;0;48;2;225;225;225m\x1b[38;2;0;0;0;48;2;225;225;225mfilter: \x1b[m\x1b[38;2;0;0;0;48;2;225;225;225mone\x1b[m\x1b[38;2;0;0;0;48;2;225;225;225m \x1b[m\x1b[38;2;0;0;0;48;2;225;225;225m(matches only) \x1b[m\x1b[m",
lines[0],
)
if len(lines) != 2 { // 1 for header
t.Errorf("expected 1 visible item, got %d", len(lines)-1)
}
if lines[1] != "item \x1b[38;2;0;0;0;48;2;255;255;255mone\x1b[m" {
t.Errorf("expected 'item one', got '%q'", lines[1])
}
util.CmpStr(
t,
"item \x1b[38;2;0;0;0;48;2;255;255;255mone\x1b[m",
lines[1],
)

// check adding matching lines
newItems := []TestItem{
Expand Down
3 changes: 2 additions & 1 deletion internal/linebuffer/linebuffer.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,8 @@ func (l LineBuffer) reapplyANSI(truncated string, startBytes int) string {
result = append(result, l.line[codeStart:codeEnd]...)
}

return string(result)
// removing empty sequences may hurt performance, but helps legibility
return constants.EmptySequenceRegex.ReplaceAllString(string(result), "")
}

func initByteOffsets(runes []rune) []int {
Expand Down
12 changes: 5 additions & 7 deletions internal/linebuffer/linebuffer_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package linebuffer

import (
"github.com/google/go-cmp/cmp"
"github.com/robinovitch61/kl/internal/util"
"testing"
)

Expand Down Expand Up @@ -156,15 +156,15 @@ func TestTruncateLine(t *testing.T) {
xOffset: 41,
width: 10,
lineContinuationIndicator: "...",
expected: "\x1b[38;2;0;0;255m\x1b[m",
expected: "",
},
{
name: "offset overflow, ansi 2",
s: "\x1b[38;2;0;0;255mfourth\x1b[m",
xOffset: 41,
width: 10,
lineContinuationIndicator: "...",
expected: "\x1b[38;2;0;0;255m\x1b[m",
expected: "",
},
{
name: "offset start space ansi",
Expand All @@ -174,7 +174,7 @@ func TestTruncateLine(t *testing.T) {
xOffset: 15,
width: 15,
lineContinuationIndicator: "",
expected: "\x1b[38;2;255;0;0m\x1b[m long line",
expected: " long line",
},
{
name: "ansi short",
Expand Down Expand Up @@ -214,9 +214,7 @@ func TestTruncateLine(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
lb := New(tt.s, tt.lineContinuationIndicator)
actual := lb.Truncate(tt.xOffset, tt.width)
if diff := cmp.Diff(tt.expected, actual); diff != "" {
t.Errorf("Mismatch (-want +got):\n%s", diff)
}
util.CmpStr(t, tt.expected, actual)
})
}
}
12 changes: 12 additions & 0 deletions internal/util/strings.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@ package util

import (
"github.com/charmbracelet/lipgloss/v2"
"github.com/google/go-cmp/cmp"
"regexp"
"runtime"
"strings"
"testing"
)

var ansiRegex = regexp.MustCompile(`(\x1b\[[0-9;]*m.*?\x1b\[0?m)`)
Expand Down Expand Up @@ -151,3 +154,12 @@ func StyleStyledString(s string, st lipgloss.Style) string {
}
return finalResult
}

// CmpStr compares two strings and fails the test if they are not equal
func CmpStr(t *testing.T, expected, actual string) {
_, file, line, _ := runtime.Caller(1)
testName := t.Name()
if diff := cmp.Diff(expected, actual); diff != "" {
t.Errorf("\nTest %q failed at %s:%d\nDiff (-expected +actual):\n%s", testName, file, line, diff)
}
}
18 changes: 3 additions & 15 deletions internal/viewport/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,9 @@ package viewport

import (
"github.com/charmbracelet/lipgloss/v2"
"github.com/google/go-cmp/cmp"
"github.com/robinovitch61/kl/internal/constants"
"github.com/robinovitch61/kl/internal/linebuffer"
"strings"
"testing"
"unicode"
)

Expand All @@ -22,7 +21,6 @@ func wrap(line string, width int, maxLinesEachEnd int) []string {
if strings.TrimSpace(line) != "" {
line = strings.TrimRightFunc(line, unicode.IsSpace)
}
//println(fmt.Sprintf("after trim time: %v", time.Since(start)))

// preserve empty lines
if line == "" {
Expand All @@ -36,16 +34,12 @@ func wrap(line string, width int, maxLinesEachEnd int) []string {
lineBuffer := linebuffer.New(line, "")

if maxLinesEachEnd > 0 && totalLines > maxLinesEachEnd*2 {
i := 0
for xOffset := 0; xOffset < width*maxLinesEachEnd; xOffset += width {
i += 1
res = append(res, lineBuffer.Truncate(xOffset, width))
}

startOffset := lineWidth - (maxLinesEachEnd * width)
i = 0
for xOffset := startOffset; xOffset < lineWidth; xOffset += width {
i += 1
res = append(res, lineBuffer.Truncate(xOffset, width))
}
} else {
Expand Down Expand Up @@ -126,7 +120,8 @@ func highlightLine(line, highlight string, highlightStyle lipgloss.Style) string
i++
}

return result.String()
// removing empty sequences may hurt performance, but helps legibility
return constants.EmptySequenceRegex.ReplaceAllString(result.String(), "")
}

// pad is a test helper function that pads the given lines to the given width and height.
Expand All @@ -153,13 +148,6 @@ func pad(width, height int, lines []string) string {
return strings.Join(res, "\n")
}

// compare is a test helper function that compares two strings and fails the test if they are different
func compare(t *testing.T, expected, actual string) {
if diff := cmp.Diff(expected, actual); diff != "" {
t.Errorf("Mismatch (-want +got):\n%s", diff)
}
}

func safeSliceUpToIdx[T any](s []T, i int) []T {
if i > len(s) {
return s
Expand Down
124 changes: 110 additions & 14 deletions internal/viewport/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ package viewport

import (
"github.com/charmbracelet/lipgloss/v2"
"github.com/google/go-cmp/cmp"
"github.com/robinovitch61/kl/internal/util"
"strings"
"testing"
)

Expand Down Expand Up @@ -40,14 +41,14 @@ func TestHighlightLine(t *testing.T) {
line: "\x1b[38;2;255;0;0mfirst line\x1b[m",
highlight: "first",
highlightStyle: lipgloss.NewStyle().Foreground(blue),
expected: "\x1b[38;2;255;0;0m\x1b[m\x1b[38;2;0;0;255mfirst\x1b[m\x1b[38;2;255;0;0m line\x1b[m",
expected: "\x1b[38;2;0;0;255mfirst\x1b[m\x1b[38;2;255;0;0m line\x1b[m",
},
{
name: "highlight already partially styled line",
line: "hi a \x1b[38;2;255;0;0mstyled line\x1b[m cool \x1b[38;2;255;0;0mand styled\x1b[m more",
highlight: "style",
highlightStyle: lipgloss.NewStyle().Foreground(blue),
expected: "hi a \x1b[38;2;255;0;0m\x1b[m\x1b[38;2;0;0;255mstyle\x1b[m\x1b[38;2;255;0;0md line\x1b[m cool \x1b[38;2;255;0;0mand \x1b[m\x1b[38;2;0;0;255mstyle\x1b[m\x1b[38;2;255;0;0md\x1b[m more",
expected: "hi a \x1b[38;2;0;0;255mstyle\x1b[m\x1b[38;2;255;0;0md line\x1b[m cool \x1b[38;2;255;0;0mand \x1b[m\x1b[38;2;0;0;255mstyle\x1b[m\x1b[38;2;255;0;0md\x1b[m more",
},
{
name: "dont highlight ansi escape codes themselves",
Expand All @@ -72,9 +73,7 @@ func TestHighlightLine(t *testing.T) {
},
} {
t.Run(tc.name, func(t *testing.T) {
if diff := cmp.Diff(tc.expected, highlightLine(tc.line, tc.highlight, tc.highlightStyle)); diff != "" {
t.Errorf("Mismatch (-want +got):\n%s", diff)
}
util.CmpStr(t, tc.expected, highlightLine(tc.line, tc.highlight, tc.highlightStyle))
})
}
}
Expand All @@ -86,9 +85,7 @@ func TestPad(t *testing.T) {
b
c
`
if diff := cmp.Diff(expected, pad(width, height, lines)); diff != "" {
t.Errorf("Mismatch (-want +got):\n%s", diff)
}
util.CmpStr(t, expected, pad(width, height, lines))
}

func TestPad_OverflowWidth(t *testing.T) {
Expand All @@ -98,16 +95,115 @@ func TestPad_OverflowWidth(t *testing.T) {
b
c
`
if diff := cmp.Diff(expected, pad(width, height, lines)); diff != "" {
t.Errorf("Mismatch (-want +got):\n%s", diff)
}
util.CmpStr(t, expected, pad(width, height, lines))
}

func TestPad_Ansi(t *testing.T) {
width, height := 5, 4
lines := []string{lipgloss.NewStyle().Foreground(red).Render("a"), "b", "c"}
expected := "\x1b[38;2;255;0;0ma\x1b[m \nb \nc \n "
if diff := cmp.Diff(expected, pad(width, height, lines)); diff != "" {
t.Errorf("Mismatch (-want +got):\n%s", diff)
util.CmpStr(t, expected, pad(width, height, lines))
}

func TestWrap(t *testing.T) {
tests := []struct {
name string
input string
width int
maxLinesEachEnd int
want []string
}{
{
name: "Empty string",
input: "",
width: 10,
maxLinesEachEnd: 2,
want: []string{""},
},
{
name: "Single line within width",
input: "Hello",
width: 10,
maxLinesEachEnd: 2,
want: []string{"Hello"},
},
{
name: "Zero width",
input: "Hello",
width: 0,
maxLinesEachEnd: 2,
want: []string{},
},
{
name: "Zero maxLinesEachEnd",
input: "This is a very long line that needs wrapping",
width: 10,
maxLinesEachEnd: 0,
want: []string{"This is a ", "very long ", "line that ", "needs wrap", "ping"},
},
{
name: "Negative maxLinesEachEnd",
input: "This is a very long line that needs wrapping",
width: 10,
maxLinesEachEnd: -1,
want: []string{"This is a ", "very long ", "line that ", "needs wrap", "ping"},
},
// TODO LEO: profile this
{
name: "Long input with truncation",
input: strings.Repeat("This is a \x1b[38;2;0;0;255mtest\x1b[0m sentence. ", 200),
width: 1,
maxLinesEachEnd: 0,
want: []string{},
},
{
name: "Input with trailing spaces",
input: "Hello ",
width: 10,
maxLinesEachEnd: 2,
want: []string{"Hello"},
},
{
name: "Input with only spaces",
input: " ",
width: 10,
maxLinesEachEnd: 2,
want: []string{" "},
},
//{
// name: "Unicode characters",
// input: "Hello 世界! This is a test with unicode characters 🌟",
// width: 10,
// maxLinesEachEnd: 3,
// want: []string{"Hello 世界! ", "This is a ", "test with ", "unicode ch", "aracters 🌟"},
//},
{
name: "Width exactly matches input length",
input: "Hello World",
width: 11,
maxLinesEachEnd: 2,
want: []string{"Hello World"},
},
{
name: "Very large maxLinesEachEnd",
input: "Short text",
width: 5,
maxLinesEachEnd: 100,
want: []string{"Short", " text"},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := wrap(tt.input, tt.width, tt.maxLinesEachEnd)

for i := range got {
if i < len(tt.want) {
if got[i] != tt.want[i] {
t.Errorf("wrap() line %d = %q, want %q", i, got[i], tt.want[i])
}
}
}
})
}
}
Loading

0 comments on commit 69b9b55

Please sign in to comment.