Skip to content

fix: wrap by display width so full-width text is not dropped - #365

Open
sujipar wants to merge 1 commit into
cli-table:masterfrom
sujipar:fix/wrap-by-display-width
Open

sujipar wants to merge 1 commit into
cli-table:masterfrom
sujipar:fix/wrap-by-display-width

Conversation

@sujipar

@sujipar sujipar commented Oct 27, 2026

Copy link
Copy Markdown

The problem — characters are silently dropped

textWrap() compares line.length against maxLength, which is a display width, and slices by
code units:

while (line.length > maxLength) {
  lines.push(line.slice(0, maxLength));
  line = line.slice(maxLength);
}

A line of full-width characters is twice as wide as its .length, so the loop exits while the line
is still wider than the cell. truncate() then cuts the overflow and replaces it with an ellipsis —
so the characters do not just wrap badly, they disappear from the output.

With {colWidths: [14], wordWrap: true, wrapOnWordBoundary: false}:

===== Korean, 14 chars =====
input : 가나다라마바사아자차카타파하   (14 chars, 28 columns)
┌──────────────┐
│ 가나다라마…  │
│ 파하         │
└──────────────┘
left in the cell : 가나다라마…파하
lost             : 바사아자차카타     <- 7 characters, gone

===== ASCII, 24 chars (control) =====
input : abcdefghijklmnopqrstuvwx
┌──────────────┐
│ abcdefghijkl │
│ mnopqrstuvwx │
└──────────────┘
lost             : none

===== Chinese, 11 chars =====
input : 这是一段没有空格的中文
│ 这是一段没…  │
lost             : 有空格的中文       <- 6 characters, gone

The same options preserve all 24 ASCII characters, so this is specific to wide text. Calling
wordWrap(12, input, false) directly confirms the intermediate step produces a 24-column line for a
12-column budget.

The fix

Measure with strlen() and slice with truncateWidth() — both already defined in this file, which
already imports string-width on line 1. No new dependency, no new helper.

===== Korean, 14 chars =====
┌──────────────┐
│ 가나다라마바 │
│ 사아자차카타 │
│ 파하         │
└──────────────┘
left in the cell : 가나다라마바사아자차카타파하  (14 chars)
lost             : none

Chinese is preserved in full as well.

ASCII output is unchanged. truncateWidth() takes a substr fast path when
str.length === strlen(str), so pure ASCII is structurally identical to the old code — and I
verified it by diffing the before/after output of the ASCII cases (0 lines differ).

About the new guard

let head = truncateWidth(line, maxLength);
if (!head.length) head = line.slice(0, 1);

truncateWidth() returns "" when the budget cannot fit even one character:

truncateWidth("가나다", 1) = ""
truncateWidth("가나다", 2) = "가"
truncateWidth("abc", 1)    = "a"

Without the guard, colWidths: [3] (an internal maxLength of 1) with full-width content would
never shorten line and the loop would not terminate — a hang the old code did not have. The guard
takes the character anyway so the loop progresses; the cell then renders …, which is unavoidable
when one character cannot fit in one column. Verified: colWidths: [3] renders │ … │,
colWidths: [4] renders each character, no hang, 10ms.

Happy to drop the guard if you'd rather handle narrow widths differently — it is the one line here
that is not purely a width-vs-length correction.

Tests

test/utils-test.js, inside the existing describe('textWrap'):

  • wraps long full-width words by display width — expected arrays for Korean and Chinese
  • never returns a line wider than maxLength — strlen(line) <= 12 for every line

test/table-test.js, next to the existing wordWrap with colored text:

  • wordWrap without word boundaries does not drop full-width chars — full table output
  • wordWrap without word boundaries keeps ascii behaviour — ASCII regression guard

Reverting src/utils.js fails 3 of the 4, so they test the behavior rather than restate it. The
ASCII guard passes either way, as it should.

One thing worth noting about how these tests were written: the first version of the full-width test
asserted wordWrap(12, input, false).join('') === input, and it passed on the reverted code —
textWrap does not drop anything by itself, it only emits over-wide lines, and the loss happens
later in truncate(). That test measured nothing, so it was replaced with the table-level one above.

Verification

before (stashed) : 1 failed, 1 skipped, 343 passed, 345 total
after            : 1 failed, 1 skipped, 347 passed, 349 total

No existing test broke; the +4 are mine. The one remaining failure
(@api original-cli-table index tests › test table colors) fails on master without my change —
it is an ansis color-output issue from the truecolor commit and is unrelated to textWrap.

prettier --write applied (jest runs the lint project).

textWrap() compared line.length against maxLength, which is a display width, and sliced
by code units. A line of full-width characters is twice as wide as its length, so the
function emitted lines wider than the cell; truncate() then cut the overflow and replaced
it with an ellipsis, so the characters were lost from the output entirely.

With colWidths: [14], wordWrap: true, wrapOnWordBoundary: false, a 14-character Korean
string came back as 7 characters. The same options preserve all 24 characters of an ASCII
string, so the loss was specific to wide text.

Measure with strlen() and slice with truncateWidth() — both already in this file, which
already imports string-width. ASCII takes truncateWidth's fast path and is unchanged.

The guard for an empty head is new: truncateWidth() returns  when even one character is
too wide for the budget (a full-width character in a single column), which would stop the
loop from making progress.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant