Repository navigation
Conversation
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>
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
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.
The problem — characters are silently dropped
textWrap()comparesline.lengthagainstmaxLength, which is a display width, and slices bycode units:
A line of full-width characters is twice as wide as its
.length, so the loop exits while the lineis 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}: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 a12-column budget.
The fix
Measure with
strlen()and slice withtruncateWidth()— both already defined in this file, whichalready imports
string-widthon line 1. No new dependency, no new helper.Chinese is preserved in full as well.
ASCII output is unchanged.
truncateWidth()takes asubstrfast path whenstr.length === strlen(str), so pure ASCII is structurally identical to the old code — and Iverified it by diffing the before/after output of the ASCII cases (0 lines differ).
About the new guard
truncateWidth()returns""when the budget cannot fit even one character:Without the guard,
colWidths: [3](an internalmaxLengthof 1) with full-width content wouldnever shorten
lineand the loop would not terminate — a hang the old code did not have. The guardtakes the character anyway so the loop progresses; the cell then renders
…, which is unavoidablewhen 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 existingdescribe('textWrap'):wraps long full-width words by display width— expected arrays for Korean and Chinesenever returns a line wider than maxLength—strlen(line) <= 12for every linetest/table-test.js, next to the existingwordWrap with colored text:wordWrap without word boundaries does not drop full-width chars— full table outputwordWrap without word boundaries keeps ascii behaviour— ASCII regression guardReverting
src/utils.jsfails 3 of the 4, so they test the behavior rather than restate it. TheASCII 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 —textWrapdoes not drop anything by itself, it only emits over-wide lines, and the loss happenslater in
truncate(). That test measured nothing, so it was replaced with the table-level one above.Verification
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 --writeapplied (jest runs the lint project).