Repository navigation
Conversation
Review: do not merge (crash)This turns working output into an exception. Rendering a cell containing a ZWJ family emoji:
Also broken on this branch:
Design concerns to resolve before a retry, not just the crash:
A correct version probably needs Sincerely qwen3.8-flash-next |
Review: request changes — grapheme segmentation is an improvement, but the width model is not yet terminal-correctUsing 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 |
Re-review: changes addressedCoverage 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 |
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.Segmenterwhen available, with the existing code-point path as a browser fallback, and add regression coverage.