fix(terminal): wrap long wide-char words by visible width, not code-point count#96689
Closed
ly-wang19 wants to merge 1 commit into
Closed
fix(terminal): wrap long wide-char words by visible width, not code-point count#96689ly-wang19 wants to merge 1 commit into
ly-wang19 wants to merge 1 commit into
Conversation
…oint count wrapNoteMessage measures every fit decision in visible columns, but its splitLongWord fallback (for a single word longer than the line budget) sliced the word into groups of maxLen code points. maxLen is a visible-column budget, so a run of wide characters (CJK / fullwidth / emoji, 2 columns each) produced lines up to twice the budget — e.g. a 24-char CJK word at maxWidth 20 emitted a 40-column line. Accumulate grapheme visible width (the same unit visibleWidth uses) and start a new segment when the next grapheme would exceed maxLen, so every wrapped segment fits the budget; a single grapheme wider than the budget still emits, preserving progress. ASCII wrapping is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
|
Closing this PR because the author has more than 20 active PRs in this repo. Please reduce the active PR queue and reopen or resubmit once it is back under the limit. You can close your own PRs to get back under the limit. |
Contributor
Author
|
Reopening — I've trimmed my active-PR queue back under the repo limit. This fix is self-contained and verified (reproduce → fix → fail-before-test → green CI), with full proof in the PR body; no live-channel environment needed. |
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.
What Problem This Solves
wrapNoteMessage(packages/terminal-core/src/note.ts) wraps note/diagnostic text to a visible-column budget — every fit decision inwrapLineis made withvisibleWidth(). But itssplitLongWordfallback (used when a single word is longer than the line budget, e.g. an unbroken CJK run) sliced the word into groups ofmaxLencode points viaArray.from(word). SincemaxLenis a visible-column budget and wide characters (CJK / fullwidth / emoji) are 2 columns each, a wide-char run produced lines up to twice the budget — e.g. a 24-character CJK word atmaxWidth: 20emitted a single 40-column line.Why This Change Was Made
splitLongWordwas the lone place that measured in code points while the rest ofwrapLinemeasures visible columns. The fix accumulates grapheme visible width (the samesplitGraphemes/visibleWidthdecomposition the module already uses) and starts a new segment when the next grapheme would exceedmaxLen, so every wrapped segment fits the budget. A single grapheme wider than the budget still emits on its own (preserving loop progress). ASCII wrapping is byte-identical.User Impact
Note bodies containing long wide-character runs (CJK text, emoji) without separators now wrap within the requested column width instead of overflowing to ~2× and breaking terminal alignment / re-wrapping. No API change.
Evidence
oxfmt,oxlint,tsgo:corepass; the fullpackages/terminal-core/src/table.test.tssuite passes with a new CJK regression test, and reverting the fix makes that test fail (fail-before).Real behavior proof
Behavior addressed:
wrapNoteMessageemitted wrapped lines up to twice the visible-column budget for long wide-character words, becausesplitLongWordsplit by code-point count rather than visible width.Real environment tested: Ran the exported
wrapNoteMessageandvisibleWidthfrompackages/terminal-core/src/{note,ansi}.tsviatsxon Node v22.22.1 (no mocks), before and after the patch, plus the colocated Vitest suite vianode scripts/run-vitest.mjs.Exact steps or command run after this patch:
wrapNoteMessage("東京特許許可局長今日休暇許可局長今日休暇東京特許", { maxWidth: 20, columns: 80 }).split("\n"), assertingvisibleWidth(line) <= 20for each line; thennode scripts/run-vitest.mjs packages/terminal-core/src/table.test.ts,node scripts/run-oxlint.mjs,pnpm tsgo:core.Evidence after fix:
Observed result after fix: every wrapped line of the CJK input is ≤ 20 visible columns (the long word breaks into 3 lines instead of one 40-column line), content is preserved (
join("")equals the input), and ASCII wrapping is unchanged.What was not tested: the actual @clack/prompts terminal render of a note (no interactive terminal in this environment); the fix is a pure wrapping-math change proven at the function and unit-test level using the module's own
visibleWidthmeasurement.