fix(ansi): do not break a line that is still empty - #958
Open
Rohilalala wants to merge 1 commit into
Open
Rohilalala wants to merge 1 commit into
Rohilalala wants to merge 1 commit into
Conversation
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.
Rohilalala
force-pushed
the
fix/ansi-empty-line-wrap
branch
from
August 24, 2026 16:52
d9053d3 to
e0de969
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A grapheme cluster wider than the limit cannot be split and cannot fit, so it takes a line of its own. Both wrappers broke the line before placing it — including when that line was still empty, which prepends a blank line to the output:
Wrapping CJK or emoji into a narrow column therefore starts with an empty line. The trigger is narrow — the limit has to be smaller than the first cluster's width, so in practice a limit of 1 with double-width text — but it is a real artifact and the fix is small.
There is nothing to break when the line is empty, so both wrapping call sites now check that first.
One subtlety
In
wrap(),addNewline()was also doing thespace/spaceWidthreset. Guarding the call without accounting for that flushes pending leading whitespace onto an already-full line —Wrap("ab\n 中文", 4, "")comes out with a six-column second line, and it needs no wide character at all (Wrap(" éé", 2, "")too). The empty-line branch drops the pending indent instead, which is what the break would have done with it.The suppression applies only to breaks the wrapper adds. Newlines in the input are untouched;
TestHardwrapKeepsExplicitBlankLinescovers that.Scope
Wrapis improved but not made whole. A cluster wider than the limit that begins a longer word still comes out with the leading blank line —Wrap("中a", 1, "")is"\n中a"— because it runs through the word and space buffers rather than the branch touched here. Reworking that machinery is its own change.Separately, and pre-existing:
Wrapdoes not hold the width invariant at all with leading whitespace.Wrap(" bc", 2, "")returns" bc\n", a three-column line, with no wide character involved — byte-identical onmain. That is why the fuzz target below coversHardwraponly; pointing it atWrapfails onmainfor reasons unrelated to this change.Tests
FuzzHardwrapWidthasserts the property the fix is about: no wrapped line is wider than the limit unless it holds a single cluster that is itself wider. It builds inputs from whole tokens — text of various widths plus complete escape sequences — rather than raw bytes, because random bytes are mostly unterminated sequences, where a "line" is a fragment of a sequence payload and its width in isolation means nothing. Escape sequences are stripped before the single-cluster check, since they carry no width. 45.8M executions clean.gofmt,go vet,golangci-lint --config ../.golangci.yml(the one finding is pre-existing inansi/sixel),go test ./... -race -count=2 -shuffle=on.