Merge branch 'johan/wide-chars'

This fixes display issues when there were wide characters in the input.

Fixes #243.

go.mod

@@ -6,6 +6,7 @@ require (
 	github.com/alecthomas/chroma/v2 v2.12.0
 	github.com/google/go-cmp v0.5.9
 	github.com/klauspost/compress v1.17.4
+	github.com/rivo/uniseg v0.4.7
 	github.com/sirupsen/logrus v1.8.1
 	github.com/ulikunitz/xz v0.5.11
 	golang.org/x/exp v0.0.0-20240103183307-be819d1f06fc

go.sum

@@ -15,6 +15,8 @@ github.com/klauspost/compress v1.17.4 h1:Ej5ixsIri7BrIjBkRZLTo6ghwrEtHFk7ijlczPW
 github.com/klauspost/compress v1.17.4/go.mod h1:/dCuZOvVtNoHsyb+cuJD3itjs3NbnF6KH9zAO4BDxPM=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
+github.com/rivo/uniseg v0.4.7 h1:WUdvkW8uEhrYfLC4ZzdpI2ztxP1I582+49Oc5Mq64VQ=
+github.com/rivo/uniseg v0.4.7/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUcx88=
 github.com/sirupsen/logrus v1.8.1 h1:dJKuHgqk1NNQlqoA6BTlM1Wf9DOH3NBjQyu0h9+AZZE=
 github.com/sirupsen/logrus v1.8.1/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0=
 github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4=

m/line.go

@@ -24,15 +24,15 @@ func NewLine(raw string) Line {
 
 // Returns a representation of the string split into styled tokens. Any regexp
 // matches are highlighted. A nil regexp means no highlighting.
-func (line *Line) HighlightedTokens(linePrefix string, search *regexp.Regexp, lineNumber *linenumbers.LineNumber) textstyles.CellsWithTrailer {
+func (line *Line) HighlightedTokens(linePrefix string, search *regexp.Regexp, lineNumber *linenumbers.LineNumber) textstyles.StyledRunesWithTrailer {
 	plain := line.Plain(lineNumber)
 	matchRanges := getMatchRanges(&plain, search)
 
-	fromString := textstyles.CellsFromString(linePrefix, line.raw, lineNumber)
-	returnCells := make([]twin.Cell, 0, len(fromString.Cells))
-	for _, token := range fromString.Cells {
+	fromString := textstyles.StyledRunesFromString(linePrefix, line.raw, lineNumber)
+	returnRunes := make([]twin.StyledRune, 0, len(fromString.StyledRunes))
+	for _, token := range fromString.StyledRunes {
 		style := token.Style
-		if matchRanges.InRange(len(returnCells)) {
+		if matchRanges.InRange(len(returnRunes)) {
 			if standoutStyle != nil {
 				style = *standoutStyle
 			} else {
@@ -42,15 +42,15 @@ func (line *Line) HighlightedTokens(linePrefix string, search *regexp.Regexp, li
 			}
 		}
 
-		returnCells = append(returnCells, twin.Cell{
+		returnRunes = append(returnRunes, twin.StyledRune{
 			Rune:  token.Rune,
 			Style: style,
 		})
 	}
 
-	return textstyles.CellsWithTrailer{
-		Cells:   returnCells,
-		Trailer: fromString.Trailer,
+	return textstyles.StyledRunesWithTrailer{
+		StyledRunes: returnRunes,
+		Trailer:     fromString.Trailer,
 	}
 }
 

m/line_test.go

@@ -25,8 +25,8 @@ func TestHighlightedTokensWithManPageHeading(t *testing.T) {
 	line := NewLine(manPageHeading)
 	highlighted := line.HighlightedTokens(prefix, nil, nil)
 
-	assert.Equal(t, len(highlighted.Cells), len(headingText))
-	for i, cell := range highlighted.Cells {
+	assert.Equal(t, len(highlighted.StyledRunes), len(headingText))
+	for i, cell := range highlighted.StyledRunes {
 		assert.Equal(t, cell.Rune, rune(headingText[i]))
 		assert.Equal(t, cell.Style, textstyles.ManPageHeading)
 	}

m/linewrapper.go

@@ -12,57 +12,92 @@ import (
 //revive:disable-next-line:var-naming
 const NO_BREAK_SPACE = '\xa0'
 
-func getWrapWidth(line []twin.Cell, maxWrapWidth int) int {
-	if len(line) <= maxWrapWidth {
-		panic(fmt.Errorf("cannot compute wrap width when input isn't longer than max (%d<=%d)",
-			len(line), maxWrapWidth))
+// Given some text and a maximum width in screen cells, find the best point at
+// which to wrap the text. Return value is in number of runes.
+func getWrapCount(line []twin.StyledRune, maxScreenCellsCount int) int {
+	if screenLength(line) <= maxScreenCellsCount {
+		panic(fmt.Errorf("cannot compute wrap width when input isn't wider than max (%d<=%d)",
+			len(line), maxScreenCellsCount))
 	}
 
-	// Find the last whitespace in the input. Since we want to break *before*
-	// whitespace, we loop through characters to the right of the current one.
-	for nextIndex := maxWrapWidth; nextIndex > 0; nextIndex-- {
-		next := line[nextIndex].Rune
-		if unicode.IsSpace(next) && next != NO_BREAK_SPACE {
+	screenCells := 0
+	bestCutPoint := maxScreenCellsCount
+	inLeadingWhitespace := true
+	for cutBeforeThisIndex := 0; cutBeforeThisIndex <= maxScreenCellsCount; cutBeforeThisIndex++ {
+		canBreakHere := false
+
+		char := line[cutBeforeThisIndex].Rune
+		onBreakableSpace := unicode.IsSpace(char) && char != NO_BREAK_SPACE
+		if onBreakableSpace && !inLeadingWhitespace {
 			// Break-OK whitespace, cut before this one!
-			return nextIndex
+			canBreakHere = true
 		}
 
-		current := line[nextIndex-1].Rune
-		if current == ']' && next == '(' {
-			// Looks like the split in a Markdown link: [text](http://127.0.0.1)
-			return nextIndex
+		if !onBreakableSpace {
+			inLeadingWhitespace = false
 		}
 
-		if nextIndex < 2 {
-			// Can't check for single slashes
-			continue
+		// Accept cutting inside "]("" in Markdown links: [home](http://127.0.0.1)
+		if cutBeforeThisIndex > 0 {
+			previousChar := line[cutBeforeThisIndex-1].Rune
+			if previousChar == ']' && char == '(' {
+				canBreakHere = true
+			}
 		}
 
 		// Break after single slashes, this is to enable breaking inside URLs / paths
-		previous := line[nextIndex-2].Rune
-		if previous != '/' && current == '/' && next != '/' {
-			return nextIndex
+		if cutBeforeThisIndex > 1 {
+			beforeSlash := line[cutBeforeThisIndex-2].Rune
+			slash := line[cutBeforeThisIndex-1].Rune
+			afterSlash := char
+			if beforeSlash != '/' && slash == '/' && afterSlash != '/' {
+				canBreakHere = true
+			}
+		}
+
+		if canBreakHere {
+			bestCutPoint = cutBeforeThisIndex
+		}
+
+		screenCells += line[cutBeforeThisIndex].Width()
+		if screenCells > maxScreenCellsCount {
+			// We went too far
+			if bestCutPoint > cutBeforeThisIndex {
+				// We have to cut here
+				bestCutPoint = cutBeforeThisIndex
+			}
+			break
 		}
 	}
 
-	// No breakpoint found, give up
-	return maxWrapWidth
+	return bestCutPoint
+}
+
+func screenLength(runes []twin.StyledRune) int {
+	length := 0
+	for _, cell := range runes {
+		length += cell.Width()
+	}
+
+	return length
 }
 
-func wrapLine(width int, line []twin.Cell) [][]twin.Cell {
+// Wrap one line of text to a maximum width
+func wrapLine(width int, line []twin.StyledRune) [][]twin.StyledRune {
 	// Trailing space risks showing up by itself on a line, which would just
 	// look weird.
 	line = twin.TrimSpaceRight(line)
 
-	if len(line) == 0 {
-		return [][]twin.Cell{{}}
+	if screenLength(line) == 0 {
+		return [][]twin.StyledRune{{}}
 	}
 
-	wrapped := make([][]twin.Cell, 0, len(line)/width)
-	for len(line) > width {
-		wrapWidth := getWrapWidth(line, width)
+	wrapped := make([][]twin.StyledRune, 0, len(line)/width)
+	for screenLength(line) > width {
+		wrapWidth := getWrapCount(line, width)
 		firstPart := line[:wrapWidth]
-		if len(wrapped) > 0 {
+		isOnFirstLine := len(wrapped) == 0
+		if !isOnFirstLine {
 			// Leading whitespace on wrapped lines would just look like
 			// indentation, which would be weird for wrapped text.
 			firstPart = twin.TrimSpaceLeft(firstPart)
@@ -73,7 +108,8 @@ func wrapLine(width int, line []twin.Cell) [][]twin.Cell {
 		line = twin.TrimSpaceLeft(line[wrapWidth:])
 	}
 
-	if len(wrapped) > 0 {
+	isOnFirstLine := len(wrapped) == 0
+	if !isOnFirstLine {
 		// Leading whitespace on wrapped lines would just look like
 		// indentation, which would be weird for wrapped text.
 		line = twin.TrimSpaceLeft(line)

m/linewrapper_test.go

@@ -4,15 +4,17 @@ import (
 	"reflect"
 	"testing"
 
+	"gotest.tools/v3/assert"
+
 	"github.com/walles/moar/twin"
 )
 
-func tokenize(input string) []twin.Cell {
+func tokenize(input string) []twin.StyledRune {
 	line := NewLine(input)
-	return line.HighlightedTokens("", nil, nil).Cells
+	return line.HighlightedTokens("", nil, nil).StyledRunes
 }
 
-func rowsToString(cellLines [][]twin.Cell) string {
+func rowsToString(cellLines [][]twin.StyledRune) string {
 	returnMe := ""
 	for _, cellLine := range cellLines {
 		lineString := ""
@@ -29,11 +31,11 @@ func rowsToString(cellLines [][]twin.Cell) string {
 	return returnMe
 }
 
-func assertWrap(t *testing.T, input string, width int, wrappedLines ...string) {
+func assertWrap(t *testing.T, input string, widthInScreenCells int, wrappedLines ...string) {
 	toWrap := tokenize(input)
-	actual := wrapLine(width, toWrap)
+	actual := wrapLine(widthInScreenCells, toWrap)
 
-	expected := [][]twin.Cell{}
+	expected := [][]twin.StyledRune{}
 	for _, wrappedLine := range wrappedLines {
 		expected = append(expected, tokenize(wrappedLine))
 	}
@@ -42,8 +44,8 @@ func assertWrap(t *testing.T, input string, width int, wrappedLines ...string) {
 		return
 	}
 
-	t.Errorf("When wrapping <%s> at width %d:\n--Expected--\n%s\n\n--Actual--\n%s",
-		input, width, rowsToString(expected), rowsToString(actual))
+	t.Errorf("When wrapping <%s> at cell count %d:\n--Expected--\n%s\n\n--Actual--\n%s",
+		input, widthInScreenCells, rowsToString(expected), rowsToString(actual))
 }
 
 func TestEnoughRoomNoWrapping(t *testing.T) {
@@ -81,6 +83,8 @@ func TestWordWrap(t *testing.T) {
 	assertWrap(t, "abc 123", 4, "abc", "123")
 	assertWrap(t, "abc 123", 3, "abc", "123")
 	assertWrap(t, "abc 123", 2, "ab", "c", "12", "3")
+
+	assertWrap(t, "here's the last line", 10, "here's the", "last line")
 }
 
 func TestWordWrapUrl(t *testing.T) {
@@ -109,3 +113,21 @@ func TestWordWrapMarkdownLink(t *testing.T) {
 	// This doesn't look great, room for tuning!
 	assertWrap(t, "[something](http://apa/bepa)", 10, "[something", "]", "(http://ap", "a/bepa)")
 }
+
+func TestWordWrapWideChars(t *testing.T) {
+	// The width is in cells, and there are wide chars in here using multiple cells.
+	assertWrap(t, "x上午y", 6, "x上午y")
+	assertWrap(t, "x上午y", 5, "x上午", "y")
+	assertWrap(t, "x上午y", 4, "x上", "午y")
+	assertWrap(t, "x上午y", 3, "x上", "午y")
+	assertWrap(t, "x上午y", 2, "x", "上", "午", "y")
+}
+
+func TestGetWrapCountWideChars(t *testing.T) {
+	line := tokenize("x上午y")
+	assert.Equal(t, getWrapCount(line, 5), 3)
+	assert.Equal(t, getWrapCount(line, 4), 2)
+	assert.Equal(t, getWrapCount(line, 3), 2)
+	assert.Equal(t, getWrapCount(line, 2), 1)
+	assert.Equal(t, getWrapCount(line, 1), 1)
+}

m/pager.go

@@ -77,8 +77,8 @@ type Pager struct {
 	QuitIfOneScreen bool
 
 	// Ref: https://github.com/walles/moar/issues/94
-	ScrollLeftHint  twin.Cell
-	ScrollRightHint twin.Cell
+	ScrollLeftHint  twin.StyledRune
+	ScrollRightHint twin.StyledRune
 
 	SideScrollAmount int // Should be positive
 
@@ -184,8 +184,8 @@ func NewPager(r *Reader) *Pager {
 		ShowStatusBar:    true,
 		DeInit:           true,
 		SideScrollAmount: 16,
-		ScrollLeftHint:   twin.NewCell('<', twin.StyleDefault.WithAttr(twin.AttrReverse)),
-		ScrollRightHint:  twin.NewCell('>', twin.StyleDefault.WithAttr(twin.AttrReverse)),
+		ScrollLeftHint:   twin.NewStyledRune('<', twin.StyleDefault.WithAttr(twin.AttrReverse)),
+		ScrollRightHint:  twin.NewStyledRune('>', twin.StyleDefault.WithAttr(twin.AttrReverse)),
 		scrollPosition:   newScrollPosition(name),
 	}
 
@@ -210,12 +210,11 @@ func (p *Pager) setFooter(footer string) {
 
 	pos := 0
 	for _, token := range footer {
-		p.screen.SetCell(pos, height-1, twin.NewCell(token, statusbarStyle))
-		pos++
+		pos += p.screen.SetCell(pos, height-1, twin.NewStyledRune(token, statusbarStyle))
 	}
 
-	for ; pos < width; pos++ {
-		p.screen.SetCell(pos, height-1, twin.NewCell(' ', statusbarStyle))
+	for pos < width {
+		pos += p.screen.SetCell(pos, height-1, twin.NewStyledRune(' ', statusbarStyle))
 	}
 }