Skip to content

fix: measure Unicode grapheme clusters as terminal glyphs - #128

Open
tecfu wants to merge 11 commits into
masterfrom
fix/grapheme-aware-display-width
Open

tecfu wants to merge 11 commits into
masterfrom
fix/grapheme-aware-display-width

Conversation

@tecfu

@tecfu tecfu commented Oct 6, 2026

Copy link
Copy Markdown
Owner

The width implementation currently sums Unicode code points. That overcounts extended grapheme clusters such as ZWJ emoji sequences, causing wrapping, truncation, and column sizing to drift from what terminals actually display. Use Intl.Segmenter when available, with the existing code-point path as a browser fallback, and add regression coverage.

@tecfu

tecfu commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

Review: do not merge (crash)

This turns working output into an exception. Rendering a cell containing a ZWJ family emoji:

master: ┌──────────┬───┐   (renders fine)
branch: TypeError: width() expects exactly one Unicode code point

breakword.width() is contractually single-code-point — there is even an existing test/core.test.ts case pinning tty-table's sum-of-code-points semantics ("measures emoji and combining marks using breakword width semantics", added in the smartwrap-4 work and passing on master). Feeding it a whole grapheme segment throws.

Also broken on this branch:

  • typecheck fails: the new test/style.test.ts block uses displayWidth without importing it (2× TS2304). CI Typecheck step is red.
  • 3 vitest failures including the core characterization above.

Design concerns to resolve before a retry, not just the crash:

  1. Measuring by grapheme while smartwrap/breakword still wrap by code point re-introduces the measure-vs-wrap split that the 7.0.0 notes explicitly call out as fixed ("Wrapping and truncation use the same primitive, so behavior is internally consistent"). Either gate this behind the same primitive change upstream, or accept and document the divergence — the 7.0.0 notes currently promise the opposite ("ZWJ sequences are still measured as the sum of their code points").
  2. new Intl.Segmenter(...) per codePointWidth call — that's once per non-ASCII cell (and cells can repeat). Hoist one segmenter, or cache grapheme widths in a Map.
  3. The ZWJ test expecting 2 and the combining-mark test expecting 1 were never executed successfully (see typecheck), so neither target semantic is actually demonstrated to work.

A correct version probably needs width() to sum code points within a segment but special-case Emoji_Presentation/ZWJ sequences to 2 — i.e. a small grapheme-width table, not breakword-per-segment.

Sincerely qwen3.8-flash-next

tecfu commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

Review: request changes — grapheme segmentation is an improvement, but the width model is not yet terminal-correct

Using Intl.Segmenter is a good direction for ZWJ sequences and combining marks, and the tests cover two important cases.

The implementation still assumes that every grapheme containing a ZWJ has display width equal to the maximum width of its component code points. That is a heuristic, not a general terminal-width rule. Grapheme segmentation and terminal cell width are related but not equivalent; the fallback also reverts to code-point summation, so behavior changes substantially depending on runtime support.

Please expand the regression matrix beyond the family emoji to include flags, skin-tone/modifier sequences, keycap sequences, variation-selector emoji, and representative combining/emoji ZWJ cases, and document the intended fallback semantics. Verify the results against the project's actual terminal-width expectations rather than treating one grapheme as universally sufficient.

The API-level change is promising, but I would not merge until supported-runtime behavior is characterized more completely.

— GPT-5.6 Luna

tecfu commented Oct 7, 2026

Copy link
Copy Markdown
Owner Author

Re-review: changes addressed

Coverage now includes ZWJ sequences, flags, keycaps, emoji modifiers, and variation-selector emoji. CI passes on the updated branch.

The grapheme-width rules remain intentionally heuristic rather than a full Unicode terminal-width implementation; no remaining merge blocker found.

— GPT-5.6 Luna

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