From e0de969f719215eedfe65b580c092f611999f723 Mon Sep 17 00:00:00 2001 From: Aditya <205600203+Rohilalala@users.noreply.github.com> Date: Sat, 22 Aug 2026 09:16:51 +0530 Subject: [PATCH] fix(ansi): do not break a line that is still empty MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wrapping a grapheme cluster wider than the limit put a blank line ahead of it. Hardwrap("中", 1, false) returned "\n中", and Wrap the same; anything starting with a double-width character wrapped into a single column came out with an empty first line. The cluster cannot be split and cannot fit either, so it takes a line of its own — but the line it is already at the start of is empty, and breaking an empty line only emits a blank one. Both wrappers now break only when the line has something on it. In Wrap the break also had to keep doing what it did with pending whitespace. addNewline() resets the space buffer, so skipping it left an indent waiting to be written, which then landed on the following line and pushed it over the limit. Suppressing the break drops that whitespace instead, which is what the break would have done with it: Wrap("ab\n 中文", 4, "") gives "ab\n中文" rather than a blank line and a six-column one. Newlines the input actually contains are untouched; the suppression is only about breaks the wrapper adds. FuzzHardwrapWidth pins the property behind this: a wrapped line is never wider than the limit unless it holds a single cluster that is itself wider. It builds inputs from whole tokens rather than random bytes, because random bytes are mostly unterminated escape sequences, where a "line" is a fragment of a sequence payload and measuring its width in isolation means nothing. The fuzz covers Hardwrap only. Wrap does not hold that property today and did not before this change: Wrap(" bc", 2, "") returns " bc\n", a three-column line with no wide character involved. Leading whitespace escaping the limit that way is a separate bug, and Wrap("中a", 1, "") still keeps its blank line, since an over-wide cluster that begins a longer word goes through the word buffer rather than the branch changed here. --- ansi/widelimit_test.go | 156 +++++++++++++++++++++++++++++++++++++++++ ansi/wrap.go | 17 ++++- 2 files changed, 171 insertions(+), 2 deletions(-) create mode 100644 ansi/widelimit_test.go diff --git a/ansi/widelimit_test.go b/ansi/widelimit_test.go new file mode 100644 index 00000000..528818a2 --- /dev/null +++ b/ansi/widelimit_test.go @@ -0,0 +1,156 @@ +package ansi_test + +import ( + "strings" + "testing" + + "github.com/charmbracelet/x/ansi" +) + +// TestHardwrapClusterWiderThanLimit covers text whose grapheme cluster is wider +// than the limit — a double-width character wrapped to a single column, say. +// The cluster cannot be split and cannot fit, so it takes a line of its own; +// what it must not do is break a line that is still empty, which puts a blank +// line ahead of it. +func TestHardwrapClusterWiderThanLimit(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + in string + limit int + want string + }{ + {name: "double width, one column", in: "中", limit: 1, want: "中"}, + {name: "two double widths, one column", in: "中文", limit: 1, want: "中\n文"}, + {name: "emoji, one column", in: "\U0001F44D", limit: 1, want: "\U0001F44D"}, + {name: "flag, one column", in: "\U0001F1FA\U0001F1F8", limit: 1, want: "\U0001F1FA\U0001F1F8"}, + {name: "double width then narrow", in: "中a", limit: 1, want: "中\na"}, + // A narrow character first means the line is not empty, so the break + // before the wide one is a real one. + {name: "narrow then double width", in: "a中", limit: 1, want: "a\n中"}, + {name: "spaces then a flag", in: " \U0001F1FA\U0001F1F8", limit: 2, want: " \n\U0001F1FA\U0001F1F8"}, + {name: "fits exactly", in: "中", limit: 2, want: "中"}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + if got := ansi.Hardwrap(tc.in, tc.limit, false); got != tc.want { + t.Errorf("Hardwrap(%q, %d) = %q, want %q", tc.in, tc.limit, got, tc.want) + } + }) + } +} + +// TestWrapClusterWiderThanLimit is the same for word wrapping, over the cases +// it handles. A cluster wider than the limit that begins a longer word is not +// covered: Wrap("中a", 1) and Wrap(" \U0001F1FA\U0001F1F8", 2) still come out +// as "\n中a" and an unbroken " \U0001F1FA\U0001F1F8". Those run through the +// word and space buffers rather than the branch below, and reworking that is +// its own change. +func TestWrapClusterWiderThanLimit(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + in string + limit int + want string + }{ + {name: "double width, one column", in: "中", limit: 1, want: "中"}, + {name: "two double widths, one column", in: "中文", limit: 1, want: "中\n文"}, + {name: "emoji, one column", in: "\U0001F44D", limit: 1, want: "\U0001F44D"}, + {name: "narrow then double width", in: "a中", limit: 1, want: "a\n中"}, + {name: "separated by a space", in: "中 文", limit: 1, want: "中\n文"}, + // Suppressing the break must still drop the whitespace waiting to be + // written, which is what the break itself would have done with it. + // Left pending, it is flushed onto the next line and pushes it over. + {name: "leading space before a wide word", in: " éé", limit: 2, want: "éé"}, + {name: "indent before a wide word", in: " 中", limit: 2, want: "中"}, + {name: "indent after a newline", in: "ab\n 中文", limit: 4, want: "ab\n中文"}, + {name: "fits exactly", in: "中", limit: 2, want: "中"}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + if got := ansi.Wrap(tc.in, tc.limit, ""); got != tc.want { + t.Errorf("Wrap(%q, %d) = %q, want %q", tc.in, tc.limit, got, tc.want) + } + }) + } +} + +// TestHardwrapKeepsExplicitBlankLines guards the fix from swallowing blank lines +// the input actually asked for: the suppression is about breaks the wrapper +// adds, not newlines it was given. +func TestHardwrapKeepsExplicitBlankLines(t *testing.T) { + t.Parallel() + + for _, in := range []string{"a\n\nb", "\n\n", "a\n\n\nb"} { + if got := ansi.Hardwrap(in, 4, false); got != in { + t.Errorf("Hardwrap(%q, 4) = %q, want it unchanged", in, got) + } + } +} + +// FuzzHardwrapWidth asserts the property the fix is really about: a wrapped +// line is never wider than the limit unless it holds one cluster that is +// itself wider, which nothing can help. +// +// Hardwrap only. Wrap does not hold this invariant today and did not before +// this change either: Wrap(" bc", 2, "") returns " bc\n", a three-column line, +// with no wide character involved. Leading whitespace escaping the limit that +// way is its own bug and its own fix. +func FuzzHardwrapWidth(f *testing.F) { + // Built from whole tokens rather than raw bytes: random bytes are mostly + // unterminated escape sequences, where a "line" is a fragment of a + // sequence payload and its width in isolation means nothing. + tokens := []string{ + // No tab: it contributes no width here but is a column stop to a + // terminal, which is a separate question from this one. + "a", "bc", "def ", " ", " ", "中", "文字", + "\U0001F44D", "\U0001F44D\U0001F3FB", "\U0001F1FA\U0001F1F8", "é", + "\x1b[31m", "\x1b[0m", "\x1b]8;;http://x\x07", + } + build := func(idx []byte) string { + var b strings.Builder + for _, i := range idx { + b.WriteString(tokens[int(i)%len(tokens)]) + } + return b.String() + } + + f.Add([]byte{0, 1, 2, 6, 8}, 4) + f.Add([]byte{8, 8, 8, 8}, 3) + f.Add([]byte{10, 10, 10}, 1) + f.Add([]byte{12, 0, 1, 13}, 5) + f.Add([]byte{3, 3, 6, 7}, 2) + + f.Fuzz(func(t *testing.T, idx []byte, limit int) { + if len(idx) == 0 || len(idx) > 24 || limit < 1 || limit > 20 { + t.Skip() + } + + s := build(idx) + out := ansi.Hardwrap(s, limit, false) + + for i, line := range strings.Split(out, "\n") { + if ansi.StringWidth(line) <= limit { + continue + } + // Allowed only when the line holds a single cluster that is itself + // too wide; the escape sequences around it carry no width, so they + // have to come off before asking. + bare := ansi.Strip(line) + if cl, _ := ansi.FirstGraphemeCluster(bare, ansi.GraphemeWidth); cl == bare && ansi.StringWidth(cl) > limit { + continue + } + t.Fatalf("line %d of %q is %d wide, over the limit of %d:\n%q", + i, s, ansi.StringWidth(line), limit, line) + } + }) +} diff --git a/ansi/wrap.go b/ansi/wrap.go index 530be6ca..44e54d47 100644 --- a/ansi/wrap.go +++ b/ansi/wrap.go @@ -59,7 +59,11 @@ func hardwrap(m Method, s string, limit int, preserveSpace bool) string { cluster, width = FirstGraphemeCluster(b[i:], m) i += len(cluster) - if curWidth+width > limit { + // Only break a line that has something on it. A cluster wider + // than the limit cannot be split and cannot fit either, so it goes + // on a line of its own; breaking first would put an empty line + // ahead of it. + if curWidth > 0 && curWidth+width > limit { addNewline() } if !preserveSpace && curWidth == 0 && len(cluster) <= 4 { @@ -369,7 +373,16 @@ func wrap(m Method, s string, limit int, breakpoints string) string { wordLen += width if curWidth+wordLen+spaceWidth > limit { - addNewline() + if curWidth == 0 { + // Nothing on the line to break, so there is no break to + // make; the word is simply wider than the limit. Drop + // the pending indent all the same, which is what the + // break would have done with it. + space.Reset() + spaceWidth = 0 + } else { + addNewline() + } } if wordLen == limit {