From f86aca58c986d30fb6b1650c90578cf447f74b1c Mon Sep 17 00:00:00 2001 From: Swastik Date: Sat, 13 Jun 2026 00:46:37 +0530 Subject: [PATCH] chore(all): fix plethora of bugs --- box.go | 63 +++---- box_render_test.go | 252 +++++++++++++++++++++++++++- doc.go | 2 +- examples/colors_and_unicode/main.go | 10 +- examples/lolcat/main.go | 2 +- examples/title_alignments/main.go | 2 +- examples/title_positions/main.go | 2 +- go.mod | 1 - go.sum | 2 - types.go | 3 +- util.go | 153 ++++++++++------- util_test.go | 85 +++++++++- 12 files changed, 448 insertions(+), 129 deletions(-) diff --git a/box.go b/box.go index a03d9a0..3697713 100755 --- a/box.go +++ b/box.go @@ -6,7 +6,6 @@ import ( "strings" "github.com/charmbracelet/x/ansi" - "github.com/huandu/xstrings" "github.com/mattn/go-runewidth" ) @@ -298,30 +297,30 @@ func (b *Box) wrapContent(content string) (string, error) { // boxLayout holds the computed dimensions needed to render a box. type boxLayout struct { - innerWidth int - longestLine int - lineWidth int - horizontalWidth int - lines []expandedLine - sideMargin string + innerWidth int + longestLine int + lineWidth int + lines []expandedLine + sideMargin string } // prepareContentLines validates the title position and padding, splits the title // and content into display lines, and returns those lines along with the number // of title lines. func (b *Box) prepareContentLines(title, content string) ([]string, int, error) { - if b.titlePos == "" { - b.titlePos = Inside - } else if b.titlePos != Inside && b.titlePos != Top && b.titlePos != Bottom { - return nil, 0, fmt.Errorf("invalid TitlePosition %s", b.titlePos) + titlePos := b.titlePos + if titlePos == "" { + titlePos = Inside + } else if titlePos != Inside && titlePos != Top && titlePos != Bottom { + return nil, 0, fmt.Errorf("invalid TitlePosition %s", titlePos) } var contentLines []string if title != "" { - if b.titlePos != Inside && strings.Contains(title, "\n") { + if titlePos != Inside && strings.Contains(title, "\n") { return nil, 0, fmt.Errorf("multiline titles are only supported Inside title position only") } - if b.titlePos == Inside { + if titlePos == Inside { contentLines = append(contentLines, strings.Split(title, "\n")...) contentLines = append(contentLines, "") // empty line between title and content } @@ -353,7 +352,7 @@ func (b *Box) computeLayout(contentLines []string, title string) boxLayout { innerWidth := contentInnerWidth // Make sure the box is wide enough to fit the title when it's on Top/Bottom. - if b.titlePos != Inside && title != "" { + if (b.titlePos == Top || b.titlePos == Bottom) && title != "" { titleWidth := runewidth.StringWidth(ansi.Strip(title)) if minW := titleWidth + 2; minW > innerWidth { innerWidth = minW @@ -376,26 +375,21 @@ func (b *Box) computeLayout(contentLines []string, title string) boxLayout { } return boxLayout{ - innerWidth: innerWidth, - longestLine: longest, - lineWidth: innerWidth + 2*verticalWidth, - horizontalWidth: horizontalWidth, - lines: lines, - sideMargin: sideMargin, + innerWidth: innerWidth, + longestLine: longest, + lineWidth: innerWidth + 2*verticalWidth, + lines: lines, + sideMargin: sideMargin, } } // buildAndColorBars constructs the top and bottom bars (optionally embedding a title) // and applies both box-chrome and title coloring. func (b *Box) buildAndColorBars(title string, lay boxLayout) (string, string, error) { - tlw := charWidth(b.topLeft) - trw := charWidth(b.topRight) - blw := charWidth(b.bottomLeft) - brw := charWidth(b.bottomRight) - - topBar := buildPlainBar(b.topLeft, b.horizontal, b.topRight, tlw, trw, lay.lineWidth, lay.horizontalWidth) - bottomBar := buildPlainBar(b.bottomLeft, b.horizontal, b.bottomRight, blw, brw, lay.lineWidth, lay.horizontalWidth) + topBar := b.buildPlainBar(b.topLeft, b.topRight, lay.lineWidth) + bottomBar := b.buildPlainBar(b.bottomLeft, b.bottomRight, lay.lineWidth) + titleOffset := -1 if b.titlePos != Inside { switch b.titlePos { case Top: @@ -404,7 +398,7 @@ func (b *Box) buildAndColorBars(title string, lay boxLayout) (string, string, er if err != nil { return "", "", err } - topBar = buildTitledBar(b.topLeft, b.horizontal, b.topRight, tlw, trw, lay.lineWidth, lay.horizontalWidth, title, align) + topBar, titleOffset = b.buildTitledBar(b.topLeft, b.topRight, lay.lineWidth, title, align) } case Bottom: if title != "" { @@ -412,7 +406,7 @@ func (b *Box) buildAndColorBars(title string, lay boxLayout) (string, string, er if err != nil { return "", "", err } - bottomBar = buildTitledBar(b.bottomLeft, b.horizontal, b.bottomRight, blw, brw, lay.lineWidth, lay.horizontalWidth, title, align) + bottomBar, titleOffset = b.buildTitledBar(b.bottomLeft, b.bottomRight, lay.lineWidth, title, align) } } } @@ -425,12 +419,7 @@ func (b *Box) buildAndColorBars(title string, lay boxLayout) (string, string, er return "", "", err } - // Apply title coloring to the bars, expanding tabs in the title if needed. - titleForBar := title - if strings.Contains(titleForBar, "\t") { - titleForBar = xstrings.ExpandTabs(titleForBar, 4) - } - if topBar, bottomBar, err = b.applyColorBar(topBar, bottomBar, titleForBar); err != nil { + if topBar, bottomBar, err = b.applyColorBar(topBar, bottomBar, title, titleOffset); err != nil { return "", "", err } @@ -461,10 +450,12 @@ func (b *Box) Render(title, content string) (string, error) { return "", err } + title = expandTabs(title) title, err = applyColor(title, b.titleColor) if err != nil { return "", err } + content = expandTabs(content) content, err = applyColor(content, b.contentColor) if err != nil { return "", err @@ -520,7 +511,6 @@ func (b *Box) applyMargin(out string) string { prefix := strings.Repeat(" ", b.mx) var sb strings.Builder for range b.my { - sb.WriteString(prefix) sb.WriteByte('\n') } for line := range strings.SplitSeq(strings.TrimSuffix(out, "\n"), "\n") { @@ -529,7 +519,6 @@ func (b *Box) applyMargin(out string) string { sb.WriteByte('\n') } for range b.my { - sb.WriteString(prefix) sb.WriteByte('\n') } return sb.String() diff --git a/box_render_test.go b/box_render_test.go index 02d99f5..7487b94 100755 --- a/box_render_test.go +++ b/box_render_test.go @@ -851,17 +851,17 @@ func TestRenderMargin(t *testing.T) { t.Errorf("VMargin: expected 3 trailing newlines (2 blank lines), got %d", nTrailing) } - // Margin(mx, my): combines both; blank vertical-margin lines carry the horizontal prefix. + // Margin(mx, my): blank vertical-margin lines are bare newlines; only box lines carry the prefix. out, err = NewBox().Style(Single).Margin(3, 1).Render(title, content) if err != nil { t.Fatalf("Margin: unexpected error: %v", err) } - if !strings.HasPrefix(out, " \n") { - t.Errorf("Margin: output does not start with prefixed blank line, got %q", out[:min(len(out), 10)]) + if !strings.HasPrefix(out, "\n") { + t.Errorf("Margin: output does not start with blank line, got %q", out[:min(len(out), 10)]) } for line := range strings.SplitSeq(strings.TrimRight(out, "\n"), "\n") { - if !strings.HasPrefix(line, " ") { - t.Errorf("Margin: line missing 3-space prefix: %q", line) + if line != "" && !strings.HasPrefix(line, " ") { + t.Errorf("Margin: non-blank line missing 3-space prefix: %q", line) } } @@ -887,6 +887,248 @@ func TestRenderMargin(t *testing.T) { } } +// expandTabsStop4 simulates the old title tab behaviour (stop-4) so the before +// case can be reproduced without touching the library. +func expandTabsStop4(s string) string { + var b strings.Builder + col := 0 + for _, c := range s { + if c == '\t' { + spaces := 4 - (col & 3) + b.WriteString(strings.Repeat(" ", spaces)) + col += spaces + } else { + w := runewidth.StringWidth(string(c)) + b.WriteRune(c) + col += w + } + } + return b.String() +} + +// TestRenderTabTitleAndContentAlign verifies that tabs in the title and tabs in +// the content expand using the same tab stop (8), so columns line up visually. +// Previously the title used stop-4 while content used stop-8, causing misalignment. +func TestRenderTabTitleAndContentAlign(t *testing.T) { + title := "Name\tAge\tCity" + content := "Alice\t30\tLondon\nBob\t25\tParis\nCharlie\t35\tTokyo" + + tokenCol := func(out, titleToken, contentToken string) (int, int) { + lines := strings.Split(strings.TrimRight(out, "\n"), "\n") + topBar := ansi.Strip(lines[0]) + // skip top bar + 2 vertical padding lines to reach first data line + firstDataLine := ansi.Strip(lines[3]) + return strings.Index(topBar, titleToken), strings.Index(firstDataLine, contentToken) + } + + // Before: pre-expand title with stop-4, leave content for the library (stop-8). + // City (title) and London (content) should NOT align. + oldOut, err := NewBox().Style(Single).Padding(1, 2).TitlePosition(Top). + Render(expandTabsStop4(title), content) + if err != nil { + t.Fatalf("before Render error: %v", err) + } + cityColOld, londonColOld := tokenCol(oldOut, "City", "London") + if cityColOld == -1 || londonColOld == -1 { + t.Fatalf("before: tokens not found in output:\n%s", oldOut) + } + if cityColOld == londonColOld { + t.Errorf("before: expected misalignment with stop-4 title vs stop-8 content, but columns matched at %d", cityColOld) + } + + // After: pass raw tab title — library expands both title and content with stop-8. + // City (title) and London (content) must align. + newOut, err := NewBox().Style(Single).Padding(1, 2).TitlePosition(Top). + Render(title, content) + if err != nil { + t.Fatalf("after Render error: %v", err) + } + cityColNew, londonColNew := tokenCol(newOut, "City", "London") + if cityColNew == -1 || londonColNew == -1 { + t.Fatalf("after: tokens not found in output:\n%s", newOut) + } + if cityColNew != londonColNew { + t.Errorf("after: title and content tab columns differ: 'City' at col %d, 'London' at col %d", + cityColNew, londonColNew) + } +} + +// TestRenderContentColorWithTabs guards against expandTabs being called after +// applyColor for content: same root cause as the title bug, but for content lines. +func TestRenderContentColorWithTabs(t *testing.T) { + content := "Name\tAge\nAlice\t30" + + out, err := NewBox().Style(Single).Padding(1, 0).ContentColor(Red).Render("", content) + if err != nil { + t.Fatalf("Render error: %v", err) + } + + lines := strings.Split(strings.TrimRight(out, "\n"), "\n") + // lines[1] = header row "Name Age", lines[2] = data row "Alice 30" + headerLine := ansi.Strip(lines[1]) + dataLine := ansi.Strip(lines[2]) + + ageCol := strings.Index(headerLine, "Age") + col30 := strings.Index(dataLine, "30") + if ageCol == -1 || col30 == -1 { + t.Fatalf("tokens not found:\nheader: %q\ndata: %q", headerLine, dataLine) + } + if ageCol != col30 { + t.Errorf("tab columns differ: 'Age' at col %d, '30' at col %d\nheader: %q\ndata: %q", + ageCol, col30, headerLine, dataLine) + } +} + +// TestRenderInsideTitleNoExtraWidth guards against computeLayout applying the +// Top/Bottom min-width constraint (titleWidth+2) to Inside titles when +// b.titlePos == "" (default). With px=0 this made the box 2 cols wider than needed. +func TestRenderInsideTitleNoExtraWidth(t *testing.T) { + // Title and content are the same width; box inner width must equal that width, not width+2. + out, err := NewBox().Style(Single).Padding(0, 0).Render("Hello", "World") + if err != nil { + t.Fatalf("Render error: %v", err) + } + lines := strings.Split(strings.TrimRight(out, "\n"), "\n") + // "┌─────┐" = 7 runes; "┌───────┐" = 9 runes would indicate the bug. + wantWidth := len([]rune("┌─────┐")) + if got := len([]rune(lines[0])); got != wantWidth { + t.Errorf("top bar width: want %d, got %d\n%s", wantWidth, got, out) + } +} + +// TestRenderDefaultTitlePosMatchesExplicitInside guards against the default titlePos ("") +// being treated differently from explicit Inside in formatLine, causing title lines to +// use content alignment (Left) instead of title alignment (Center). +func TestRenderDefaultTitlePosMatchesExplicitInside(t *testing.T) { + title := "Title" + content := "Content" + + defaultOut, err := NewBox().Style(Single).Render(title, content) + if err != nil { + t.Fatalf("default Render error: %v", err) + } + explicitOut, err := NewBox().Style(Single).TitlePosition(Inside).Render(title, content) + if err != nil { + t.Fatalf("explicit Inside Render error: %v", err) + } + if defaultOut != explicitOut { + t.Errorf("default titlePos and explicit Inside produce different output:\ndefault:\n%s\nexplicit:\n%s", + defaultOut, explicitOut) + } +} + +// TestRenderTitleColorWithTabs guards against expandTabs being called after applyColor: +// when TitleColor is set, the title gains ANSI escape bytes whose ASCII characters +// (e.g. '[','3','8',';'…) were being counted as visible columns, shifting every +// subsequent tab stop. The fix is to expand tabs before applying color in Render. +func TestRenderTitleColorWithTabs(t *testing.T) { + title := "Name\tAge" + content := "Alice\t30" + + out, err := NewBox().Style(Single).Padding(1, 0). + TitlePosition(Top).TitleColor(Red). + Render(title, content) + if err != nil { + t.Fatalf("Render error: %v", err) + } + + lines := strings.Split(strings.TrimRight(out, "\n"), "\n") + topBar := ansi.Strip(lines[0]) + // No vertical padding: content is the first interior line. + contentLine := ansi.Strip(lines[1]) + + titleAgeCol := strings.Index(topBar, "Age") + contentAgeCol := strings.Index(contentLine, "30") + if titleAgeCol == -1 || contentAgeCol == -1 { + t.Fatalf("tokens not found in output:\n%s", out) + } + if titleAgeCol != contentAgeCol { + t.Errorf("tab columns differ: 'Age' in title at col %d, '30' in content at col %d\n%s", + titleAgeCol, contentAgeCol, out) + } +} + +// TestRenderIsIdempotent guards against B1: prepareContentLines previously mutated +// b.titlePos on the receiver, causing repeated Render calls on the same Box to diverge. +func TestRenderIsIdempotent(t *testing.T) { + b := NewBox().Style(Single).Padding(1, 2) + out1, err := b.Render("Title", "content") + if err != nil { + t.Fatalf("first Render error: %v", err) + } + out2, err := b.Render("Title", "content") + if err != nil { + t.Fatalf("second Render error: %v", err) + } + if out1 != out2 { + t.Errorf("Render mutated box state between calls:\nfirst:\n%s\nsecond:\n%s", out1, out2) + } +} + +// TestRenderColoredBorderConsistentAcrossLines guards against B2: applyColor for the +// vertical border was previously called once per content line instead of once per Render. +func TestRenderColoredBorderConsistentAcrossLines(t *testing.T) { + content := "a\nb\nc\nd\ne" + out, err := NewBox().Style(Single).Padding(0, 1).Color("Cyan").Render("", content) + if err != nil { + t.Fatalf("Render error: %v", err) + } + lines := strings.Split(strings.TrimRight(out, "\n"), "\n") + contentLines := lines[1 : len(lines)-1] + if len(contentLines) < 2 { + t.Fatal("need at least 2 content lines") + } + borderEnd := strings.Index(contentLines[0], "│") + len("│") + wantBorder := contentLines[0][:borderEnd] + for i, line := range contentLines[1:] { + if len(line) < borderEnd || line[:borderEnd] != wantBorder { + t.Errorf("line %d border differs:\ngot %q\nwant %q", i+1, line[:min(len(line), borderEnd)], wantBorder) + } + } +} + +// TestTitleColorNotDoubleApplied guards against B3: applyColorBar previously called +// applyColor(title, titleColor) again even though Render had already colored the title. +func TestTitleColorNotDoubleApplied(t *testing.T) { + out, err := NewBox(). + Style(Single). + Padding(1, 3). + TitlePosition(Top). + Color("Cyan"). + TitleColor("Red"). + Render("Hello", "world") + if err != nil { + t.Fatalf("Render error: %v", err) + } + topBar := strings.SplitN(strings.TrimRight(out, "\n"), "\n", 2)[0] + // Consecutive resets are the visible symptom of double-coloring. + if strings.Contains(topBar, "\x1b[m\x1b[m") { + t.Errorf("top bar contains consecutive ANSI resets — title color was applied twice: %q", topBar) + } +} + +// TestRenderANSIColoredContentWidth guards against B5: formatLine previously called +// runewidth.StringWidth(line.line) twice instead of reusing line.len, which could +// miscount visible width for lines containing ANSI escape sequences. +func TestRenderANSIColoredContentWidth(t *testing.T) { + colored, err := applyColor("hello", "Red") + if err != nil { + t.Fatalf("applyColor error: %v", err) + } + content := colored + "\nnormal line" + out, err := NewBox().Style(Single).Padding(1, 2).Render("", content) + if err != nil { + t.Fatalf("Render error: %v", err) + } + lines := strings.Split(strings.TrimRight(out, "\n"), "\n") + w0 := runewidth.StringWidth(ansi.Strip(lines[0])) + for i, line := range lines[1:] { + if w := runewidth.StringWidth(ansi.Strip(line)); w != w0 { + t.Errorf("line %d width %d != expected %d: %q", i+1, w, w0, ansi.Strip(line)) + } + } +} + func findLineContainingTitle(lines []string, title string) string { for _, line := range lines { if strings.Contains(line, title) { diff --git a/doc.go b/doc.go index ad7cf4a..9e78f2b 100644 --- a/doc.go +++ b/doc.go @@ -14,7 +14,7 @@ // Color(box.Cyan). // TitleColor(box.BrightYellow). // -// out, err := b.Render("Box CLI Maker", "Render highly customizable boxes\n in the terminal") +// out, err := b.Render("Box CLI Maker", "Render highly customizable boxes\nin the terminal") // if err != nil { // log.Fatal(err) // } diff --git a/examples/colors_and_unicode/main.go b/examples/colors_and_unicode/main.go index 7b9799f..55a64f8 100644 --- a/examples/colors_and_unicode/main.go +++ b/examples/colors_and_unicode/main.go @@ -72,14 +72,14 @@ func main() { "Κουτί CLI Maker", } lines := []string{ - "Render highly customizable boxes\n in the terminal", + "Render highly customizable boxes\nin the terminal", "端末で高度にカスタマイズ可能なボックスを\nターミナルでレンダリングする", "在终端中渲染高度可定制的盒子\n", "터미널에서 고도로 커스터마이즈 가능한 박스를\n렌더링하기", - "Rendre des boîtes hautement personnalisables\n dans le terminal", - "Renderiza cajas de cajas altamente personalizables\n en el terminal", - "Pyxides terminales maxime configurabiles\n in terminali redde", - "Απόδωσε εξαιρετικά προσαρμόσιμα κουτιά\n στο τερματικό", + "Rendre des boîtes hautement personnalisables\ndans le terminal", + "Renderiza cajas de cajas altamente personalizables\nen el terminal", + "Pyxides terminales maxime configurabiles\nin terminali redde", + "Απόδωσε εξαιρετικά προσαρμόσιμα κουτιά\nστο τερματικό", } for i := range titles { diff --git a/examples/lolcat/main.go b/examples/lolcat/main.go index 62c75eb..465a2e0 100755 --- a/examples/lolcat/main.go +++ b/examples/lolcat/main.go @@ -12,7 +12,7 @@ import ( func main() { b := box.NewBox().Padding(2, 5).Style(box.Single).Color(box.Cyan).ContentAlign(box.Center) - s, err := b.Render(lolcat("Box CLI Maker"), lolcat("Render highly customizable boxes\n in the terminal")) + s, err := b.Render(lolcat("Box CLI Maker"), lolcat("Render highly customizable boxes\nin the terminal")) if err != nil { panic(err) } diff --git a/examples/title_alignments/main.go b/examples/title_alignments/main.go index 38df3e7..4bd295e 100644 --- a/examples/title_alignments/main.go +++ b/examples/title_alignments/main.go @@ -26,7 +26,7 @@ func main() { TitlePosition(pos). TitleAlign(align) - out, err := b.Render("Box CLI Maker", "Render highly customizable boxes\n in the terminal") + out, err := b.Render("Box CLI Maker", "Render highly customizable boxes\nin the terminal") if err != nil { panic(err) } diff --git a/examples/title_positions/main.go b/examples/title_positions/main.go index 4035838..702230f 100644 --- a/examples/title_positions/main.go +++ b/examples/title_positions/main.go @@ -31,7 +31,7 @@ func main() { Style(style). TitlePosition(pos) - out, err := b.Render("Box CLI Maker", "Render highly customizable boxes\n in the terminal") + out, err := b.Render("Box CLI Maker", "Render highly customizable boxes\nin the terminal") if err != nil { panic(err) } diff --git a/go.mod b/go.mod index a43e50b..c58e474 100755 --- a/go.mod +++ b/go.mod @@ -3,7 +3,6 @@ module github.com/box-cli-maker/box-cli-maker/v3 go 1.24.2 require ( - github.com/huandu/xstrings v1.5.0 github.com/mattn/go-runewidth v0.0.19 github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect golang.org/x/sys v0.40.0 // indirect diff --git a/go.sum b/go.sum index 5500998..12bc5bc 100644 --- a/go.sum +++ b/go.sum @@ -10,8 +10,6 @@ github.com/clipperhouse/stringish v0.1.1 h1:+NSqMOr3GR6k1FdRhhnXrLfztGzuG+VuFDfa github.com/clipperhouse/stringish v0.1.1/go.mod h1:v/WhFtE1q0ovMta2+m+UbpZ+2/HEXNWYXQgCt4hdOzA= github.com/clipperhouse/uax29/v2 v2.5.0 h1:x7T0T4eTHDONxFJsL94uKNKPHrclyFI0lm7+w94cO8U= github.com/clipperhouse/uax29/v2 v2.5.0/go.mod h1:Wn1g7MK6OoeDT0vL+Q0SQLDz/KpfsVRgg6W7ihQeh4g= -github.com/huandu/xstrings v1.5.0 h1:2ag3IFq9ZDANvthTwTiqSSZLjDc+BedvHPAp5tJy2TI= -github.com/huandu/xstrings v1.5.0/go.mod h1:y5/lhBue+AyNmUVz9RLU9xbLR0o4KIIExikq4ovT0aE= github.com/lucasb-eyer/go-colorful v1.3.0 h1:2/yBRLdWBZKrf7gB40FoiKfAWYQ0lqNcbuQwVHXptag= github.com/lucasb-eyer/go-colorful v1.3.0/go.mod h1:R4dSotOR9KMtayYi1e77YzuveK+i7ruzyGqttikkLy0= github.com/mattn/go-runewidth v0.0.19 h1:v++JhqYnZuu5jSKrk9RbgF5v4CGUjqRfBm05byFGLdw= diff --git a/types.go b/types.go index 8f32cd7..67362d3 100755 --- a/types.go +++ b/types.go @@ -160,8 +160,7 @@ var ( vertical: "█", }, } -) -var ( + // colorToHex maps color names to their hexadecimal codes. // This includes both standard and bright ANSI colors. colorToHex = map[string]string{ diff --git a/util.go b/util.go index 40b84b5..c271bdc 100755 --- a/util.go +++ b/util.go @@ -7,7 +7,6 @@ import ( "github.com/charmbracelet/x/ansi" "github.com/charmbracelet/x/term" - "github.com/huandu/xstrings" "github.com/mattn/go-runewidth" ) @@ -46,33 +45,63 @@ func (b *Box) addVertPadding(innerWidth int) ([]string, error) { return texts, nil } +// expandTabs expands tab characters in s using tab stops at every 8 columns, +// consistent with POSIX terminal behavior. +// ANSI escape sequences are passed through without affecting the column count. +func expandTabs(s string) string { + if !strings.Contains(s, "\t") { + return s + } + var b strings.Builder + colPos := 0 + inEscape := false + for _, c := range s { + if c == '\033' { + inEscape = true + b.WriteRune(c) + continue + } + if inEscape { + b.WriteRune(c) + if (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') { + inEscape = false + } + continue + } + switch c { + case '\t': + spaces := 8 - (colPos & 7) + b.WriteString(strings.Repeat(" ", spaces)) + colPos += spaces + case '\n': + b.WriteRune(c) + colPos = 0 + default: + w := max(runewidth.RuneWidth(c), 0) + b.WriteRune(c) + colPos += w + } + } + return b.String() +} + // longestLine expands tabs in lines and determines longest visible // return longest length and array of expanded lines func longestLine(lines []string) (int, []expandedLine) { longest := 0 expandedLines := make([]expandedLine, 0, len(lines)) - var tmpLine strings.Builder - var lineLen int for _, line := range lines { - tmpLine.Reset() - for _, c := range line { - lineLen = runewidth.StringWidth(tmpLine.String()) - - if c == '\t' { - tmpLine.WriteString(strings.Repeat(" ", 8-(lineLen&7))) - } else { - tmpLine.WriteRune(c) - } - } - lineLen = runewidth.StringWidth(tmpLine.String()) - expandedLines = append(expandedLines, expandedLine{tmpLine.String(), lineLen}) + expanded := expandTabs(line) + lineLen := runewidth.StringWidth(expanded) - // Check if each line has ANSI Color Code then decrease the length accordingly - if runewidth.StringWidth(ansi.Strip(tmpLine.String())) < runewidth.StringWidth(tmpLine.String()) { - lineLen = runewidth.StringWidth(ansi.Strip(tmpLine.String())) + // Use visible width: strip ANSI codes before measuring. + if stripped := runewidth.StringWidth(ansi.Strip(expanded)); stripped < lineLen { + lineLen = stripped } + expandedLines = append(expandedLines, expandedLine{expanded, lineLen}) + if lineLen > longest { longest = lineLen } @@ -125,7 +154,11 @@ func buildAlignedSegment(fill string, width, horizontalWidth int, attachLeft boo // buildPlainBar builds a horizontal bar (without title) that matches the // specified visual line width. -func buildPlainBar(left, fill, right string, leftW, rightW, lineWidth, horizontalWidth int) string { +func (b *Box) buildPlainBar(left, right string, lineWidth int) string { + fill := b.horizontal + leftW := charWidth(left) + rightW := charWidth(right) + horizontalWidth := charWidth(fill) inner := max(lineWidth-leftW-rightW, 0) bar := buildSegment(fill, inner, horizontalWidth) return left + bar + right @@ -134,15 +167,19 @@ func buildPlainBar(left, fill, right string, leftW, rightW, lineWidth, horizonta // buildTitledBar builds a top or bottom bar containing a title with the given // alignment. Any leftover width that is not divisible by the glyph's width is // emitted as spaces so the fill glyph remains adjacent to the corners. -func buildTitledBar(left, fill, right string, leftW, rightW, lineWidth, horizontalWidth int, title string, align AlignType) string { +// buildTitledBar returns the assembled bar string and the byte offset within +// that string where plainTitle begins. The offset is -1 when title is empty. +func (b *Box) buildTitledBar(left, right string, lineWidth int, title string, align AlignType) (string, int) { + fill := b.horizontal + leftW := charWidth(left) + rightW := charWidth(right) + horizontalWidth := charWidth(fill) + if title == "" { - return buildPlainBar(left, fill, right, leftW, rightW, lineWidth, horizontalWidth) + return b.buildPlainBar(left, right, lineWidth), -1 } - plainTitle := title - if strings.Contains(plainTitle, "\t") { - plainTitle = xstrings.ExpandTabs(plainTitle, 4) - } + plainTitle := expandTabs(title) titleWidth := runewidth.StringWidth(ansi.Strip(plainTitle)) titleSegWidth := titleWidth + 2 // one space padding on each side @@ -166,22 +203,24 @@ func buildTitledBar(left, fill, right string, leftW, rightW, lineWidth, horizont leftSeg := buildAlignedSegment(fill, leftWidth, horizontalWidth, true) rightSeg := buildAlignedSegment(fill, rightWidth, horizontalWidth, false) - return left + leftSeg + " " + plainTitle + " " + rightSeg + right + // prefix contains no ANSI, so len(prefix) is the title offset in both + // the raw bar and the ANSI-stripped bar. + prefix := left + leftSeg + " " + return prefix + plainTitle + " " + rightSeg + right, len(prefix) } // formatLine formats the line according to the information passed. func (b *Box) formatLine(lines2 []expandedLine, longestLine, titleLen int, sideMargin, title string, texts []string) ([]string, error) { + sep, err := applyColor(b.vertical, b.color) + if err != nil { + return nil, err + } + for i, line := range lines2 { length := line.len var space, oddSpace string - // compute stripped width once - strippedWidth := runewidth.StringWidth(ansi.Strip(line.line)) - if strippedWidth < runewidth.StringWidth(line.line) { - length = strippedWidth - } - // If current text is shorter than the longest one // center the text, so it looks better if length < longestLine { @@ -204,7 +243,7 @@ func (b *Box) formatLine(lines2 []expandedLine, longestLine, titleLen int, sideM var format string var err error - if i < titleLen && title != "" && b.titlePos == Inside { + if i < titleLen && title != "" && (b.titlePos == Inside || b.titlePos == "") { format, err = b.findTitleAlignFormat(Center) } else { format, err = b.findContentAlign() @@ -213,11 +252,6 @@ func (b *Box) formatLine(lines2 []expandedLine, longestLine, titleLen int, sideM return nil, err } - sep, err := applyColor(b.vertical, b.color) - if err != nil { - return nil, err - } - formatted := fmt.Sprintf(format, sep, spacing, line.line, oddSpace, space, sideMargin) texts = append(texts, formatted) } @@ -363,12 +397,14 @@ func applyConvertedColor(str string, c color.Color) string { return sb.String() } -func (b *Box) applyColorBar(topBar, bottomBar, title string) (string, string, error) { +func (b *Box) applyColorBar(topBar, bottomBar, title string, titleOffset int) (string, string, error) { + if b.titlePos != Top && b.titlePos != Bottom { + return topBar, bottomBar, nil + } if b.titleColor == "" || title == "" { return topBar, bottomBar, nil } - - if strings.TrimSpace(b.color) == "" { + if b.color == "" { return topBar, bottomBar, nil } @@ -377,34 +413,23 @@ func (b *Box) applyColorBar(topBar, bottomBar, title string) (string, string, er return "", "", err } - if b.titlePos == Top { - strippedBar := ansi.Strip(topBar) + colorBarTitle := func(bar string) string { + strippedBar := ansi.Strip(bar) strippedTitle := ansi.Strip(title) - if idx := strings.Index(strippedBar, strippedTitle); idx != -1 { - // split around first occurrence to preserve any other repeats - b0 := applyConvertedColor(strippedBar[:idx], converted) - b1 := applyConvertedColor(strippedBar[idx+len(strippedTitle):], converted) - coloredTitle, err := applyColor(title, b.titleColor) - if err != nil { - return "", "", err - } - topBar = b0 + coloredTitle + b1 + end := titleOffset + len(strippedTitle) + if titleOffset < 0 || end > len(strippedBar) { + return bar } + b0 := applyConvertedColor(strippedBar[:titleOffset], converted) + b1 := applyConvertedColor(strippedBar[end:], converted) + return b0 + title + b1 } + if b.titlePos == Top { + topBar = colorBarTitle(topBar) + } if b.titlePos == Bottom { - strippedBar := ansi.Strip(bottomBar) - strippedTitle := ansi.Strip(title) - if idx := strings.Index(strippedBar, strippedTitle); idx != -1 { - // split around first occurrence to preserve any other repeats - b0 := applyConvertedColor(strippedBar[:idx], converted) - b1 := applyConvertedColor(strippedBar[idx+len(strippedTitle):], converted) - coloredTitle, err := applyColor(title, b.titleColor) - if err != nil { - return "", "", err - } - bottomBar = b0 + coloredTitle + b1 - } + bottomBar = colorBarTitle(bottomBar) } return topBar, bottomBar, nil diff --git a/util_test.go b/util_test.go index 6f9d845..14fef67 100755 --- a/util_test.go +++ b/util_test.go @@ -55,6 +55,13 @@ func TestLongestLineBasicAndTabs(t *testing.T) { t.Errorf("expected longest 9 for tab-expanded line, got %d", longest) } + // Multi-line string: colPos must reset at each newline so line 2 tabs align from col 0. + got := expandTabs("A\tZ\nB\tZ") + want := "A" + strings.Repeat(" ", 7) + "Z\nB" + strings.Repeat(" ", 7) + "Z" + if got != want { + t.Errorf("expandTabs multi-line: want %q, got %q", want, got) + } + // ANSI-colored line should be measured by visible width plain := "abc" colored := "\x1b[31mabc\x1b[0m" // same visible width as plain @@ -64,6 +71,29 @@ func TestLongestLineBasicAndTabs(t *testing.T) { } } +func TestExpandTabsWithANSI(t *testing.T) { + // "\033[31mName\033[0m\t" — "Name" is 4 visible cols; tab at col 4 should + // jump to col 8 (4 spaces), not be shifted by the ANSI escape bytes. + got := expandTabs("\033[31mName\033[0m\t|") + want := "\033[31mName\033[0m |" + if got != want { + t.Errorf("expandTabs with ANSI prefix: want %q, got %q", want, got) + } + + // No tabs: must pass through unchanged. + noTab := "\033[31mHello\033[0m" + if got := expandTabs(noTab); got != noTab { + t.Errorf("expandTabs no-tab: want %q, got %q", noTab, got) + } + + // Mixed: plain line followed by ANSI line — each tab aligns from col 0. + got = expandTabs("Name\tAge\n\033[31mAlice\033[0m\t30") + want = "Name Age\n\033[31mAlice\033[0m 30" + if got != want { + t.Errorf("expandTabs multiline ANSI: want %q, got %q", want, got) + } +} + func TestFormatLine_LeftAlignAndError(t *testing.T) { // Happy path: left-aligned content, no title b := &Box{vertical: "|"} @@ -300,7 +330,7 @@ func TestApplyColorBar(t *testing.T) { // Early return when titleColor is empty or title is empty. b := &Box{} - gotTop, gotBottom, err := b.applyColorBar(top, bottom, title) + gotTop, gotBottom, err := b.applyColorBar(top, bottom, title, -1) if err != nil { t.Fatalf("applyColorBar unexpected error: %v", err) } @@ -313,7 +343,8 @@ func TestApplyColorBar(t *testing.T) { b.titleColor = BrightRed b.color = BrightBlue b.titlePos = Top - gotTop, gotBottom, err = b.applyColorBar(top, bottom, title) + topTitleOffset := strings.Index(top, title) + gotTop, gotBottom, err = b.applyColorBar(top, bottom, title, topTitleOffset) if err != nil { t.Fatalf("applyColorBar unexpected error for top title: %v", err) } @@ -334,7 +365,8 @@ func TestApplyColorBar(t *testing.T) { b.titleColor = BrightRed b.color = BrightBlue b.titlePos = Bottom - gotTop, gotBottom, err = b.applyColorBar(topPlain, bottomWithTitle, title) + bottomTitleOffset := strings.Index(bottomWithTitle, title) + gotTop, gotBottom, err = b.applyColorBar(topPlain, bottomWithTitle, title, bottomTitleOffset) if err != nil { t.Fatalf("applyColorBar unexpected error for bottom title: %v", err) } @@ -358,7 +390,7 @@ func TestApplyColorBar(t *testing.T) { b.titleColor = BrightRed b.color = "" b.titlePos = Top - gotTop, gotBottom, err = b.applyColorBar(topWithColoredTitle, bottom, coloredTitle) + gotTop, gotBottom, err = b.applyColorBar(topWithColoredTitle, bottom, coloredTitle, -1) if err != nil { t.Fatalf("applyColorBar unexpected error when Color is empty: %v", err) } @@ -407,7 +439,8 @@ func TestBuildPlainBar(t *testing.T) { left := fill right := fill - bar := buildPlainBar(left, fill, right, hw, hw, lineWidth, hw) + b := &Box{horizontal: fill} + bar := b.buildPlainBar(left, right, lineWidth) if w := runewidth.StringWidth(bar); w != lineWidth { t.Fatalf("expected bar visual width %d, got %d", lineWidth, w) } @@ -416,6 +449,41 @@ func TestBuildPlainBar(t *testing.T) { } } +// TestApplyColorBarFillCharacterTitle guards against B4: applyColorBar previously used +// strings.Index to locate the title, which matched the first fill character when the +// title equalled the fill glyph (e.g., "─" with Single style), coloring the wrong region. +func TestApplyColorBarFillCharacterTitle(t *testing.T) { + fill := "─" + hw := charWidth(fill) + title := fill + left, right := "┌", "┐" + lw, rw := charWidth(left), charWidth(right) + lineWidth := 20*hw + lw + rw + + b := &Box{} + b.horizontal = fill + b.titleColor = BrightRed + b.color = BrightBlue + b.titlePos = Top + bar, titleOffset := b.buildTitledBar(left, right, lineWidth, title, Center) + topBar, _, err := b.applyColorBar(bar, "", title, titleOffset) + if err != nil { + t.Fatalf("applyColorBar error: %v", err) + } + + // Visual content must be unchanged after coloring. + if ansi.Strip(topBar) != bar { + t.Errorf("stripped top bar differs from original:\ngot %q\nwant %q", ansi.Strip(topBar), bar) + } + + // The title must be centered — there must be fill characters before it. + // With the bug, strings.Index matched position 1 (right after ┌). + titlePos := strings.Index(bar, " "+title+" ") + if titlePos <= runewidth.StringWidth(left) { + t.Errorf("title found at position %d — expected fill before title, bar: %q", titlePos, bar) + } +} + func TestBuildTitledBar_LeftAlignedWithEmojiFill(t *testing.T) { fill := "📦" hw := runewidth.StringWidth(fill) @@ -423,11 +491,10 @@ func TestBuildTitledBar_LeftAlignedWithEmojiFill(t *testing.T) { left := fill right := fill - leftW := hw - rightW := hw - lineWidth := hw*20 + leftW + rightW + lineWidth := hw*20 + hw + hw - bar := buildTitledBar(left, fill, right, leftW, rightW, lineWidth, hw, title, Left) + b := &Box{horizontal: fill} + bar, _ := b.buildTitledBar(left, right, lineWidth, title, Left) if w := runewidth.StringWidth(ansi.Strip(bar)); w != lineWidth { t.Fatalf("expected bar visual width %d, got %d", lineWidth, w) }