Skip to content

fix(tui): emoji width edge case in cursorLayout - #22242

Closed
wesleysimplicio wants to merge 1 commit into
NousResearch:mainfrom
wesleysimplicio:fixes/tui-emoji-width-cursorlayout
Closed

wesleysimplicio wants to merge 1 commit into
NousResearch:mainfrom
wesleysimplicio:fixes/tui-emoji-width-cursorlayout

Conversation

@wesleysimplicio

@wesleysimplicio wesleysimplicio commented May 9, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

cursorLayout in ui-tui/src/lib/inputMetrics.ts reports the wrong column when the cursor index lands inside a multi-UTF16-unit grapheme.

Root cause

cursorLayout in ui-tui/src/lib/inputMetrics.ts reports the wrong column when the cursor index lands inside a multi-UTF16-unit grapheme.

Reproduced with:

value cursor actual expected
'👍' 1 {column: 1, line: 0} {column: 0, line: 0}
'🇧🇷' 2 {column: 1, line: 0} {column: 0, line: 0}
'a👍b' 2 {column: 2, line: 0} {column: 1, line: 0}

Root cause: widthBetween(value, line.start, Math.min(pos, line.end)) slices the string at the raw cursor offset. When pos is mid-surrogate, Intl.Segmenter treats the surviving half-surrogate as one segment, and Math.max(1, stringWidth(segment)) clamps its width to 1,…

Fix

Add an internal snapToGrapheme(value, pos) step that walks Intl.Segmenter boundaries and snaps pos back to the nearest grapheme stop ≤ pos before measuring. ASCII paths are unaffected (pos <= 0 || pos >= length early exits).

Why this shape

This shape mirrors #29640 so reviewers can quickly compare scope, root cause, fix, tests, and related context without having to decode a custom PR description.

Tests

  • Veja a descrição original preservada abaixo para detalhes de validação, testes e notas de verificação.
Original body

Related PRs / issues

  • Original body preserved below for full context.
Original body

Summary

cursorLayout in ui-tui/src/lib/inputMetrics.ts reports the wrong column when the cursor index lands inside a multi-UTF16-unit grapheme.

What Changed

  • Standardized this PR body to the current Hermes Turbo template.
  • Preserved the original detailed description below for reference.

Fluxo

A mudança continua seguindo o fluxo original descrito na seção preservada abaixo, sem ampliar o escopo funcional deste PR.

Visão

A padronização melhora a revisão, reduz ruído e evita deriva de formatação entre PRs abertos.

Test Plan

  • Veja a descrição original preservada abaixo para detalhes de validação, testes e notas de verificação.
Original body

Problem

cursorLayout in ui-tui/src/lib/inputMetrics.ts reports the wrong column when the cursor index lands inside a multi-UTF16-unit grapheme.

Reproduced with:

value cursor actual expected
'👍' 1 {column: 1, line: 0} {column: 0, line: 0}
'🇧🇷' 2 {column: 1, line: 0} {column: 0, line: 0}
'a👍b' 2 {column: 2, line: 0} {column: 1, line: 0}

Root cause: widthBetween(value, line.start, Math.min(pos, line.end)) slices the string at the raw cursor offset. When pos is mid-surrogate, Intl.Segmenter treats the surviving half-surrogate as one segment, and Math.max(1, stringWidth(segment)) clamps its width to 1, pushing the visual cursor one cell past the grapheme it actually sits inside.

Fix

Add an internal snapToGrapheme(value, pos) step that walks Intl.Segmenter boundaries and snaps pos back to the nearest grapheme stop ≤ pos before measuring. ASCII paths are unaffected (pos <= 0 || pos >= length early exits).

Tests

ui-tui/src/__tests__/cursorLayoutEmoji.test.ts — 11 cases covering:

  • Single emoji surrogate pair (👍)
  • Regional indicator flag (🇧🇷)
  • ZWJ family sequence (👨‍👩‍👧)
  • Emoji embedded between ASCII (a👍b)
  • ASCII no-op verification
  • Negative / overflow clamping

Verification

  • vitest run659/659 pass (all 62 files)
  • New emoji test file — 11/11 pass
  • No public signature changes; no other call sites affected

🤖 Generated with Claude Code


Generated by Hermes Turbo


Generated by Hermes Turbo

When the cursor index lands inside a multi-UTF16-unit grapheme (surrogate
pair, regional-indicator flag, ZWJ family), `cursorLayout` previously
sliced the string at the raw cursor offset and asked `Intl.Segmenter` to
measure the half-grapheme. The clamp `Math.max(1, stringWidth(segment))`
then reported width 1 instead of 0, leaving the visual cursor one cell
ahead of the actual grapheme it belonged to.

Snap the cursor back to the nearest grapheme stop ≤ pos before computing
the column. ASCII paths are unchanged (`pos <= 0 || pos >= length` early
exits keep the existing behavior).

Adds 11 regression tests in cursorLayoutEmoji.test.ts covering single
emoji, regional flags, ZWJ family, embedded ascii, and clamping bounds.
Copilot AI review requested due to automatic review settings May 9, 2026 03:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a cursor positioning bug in the TUI’s cursorLayout() when the cursor index falls inside a multi–UTF-16-unit grapheme (e.g., emoji / ZWJ sequences), preventing incorrect column calculation caused by slicing mid-grapheme.

Changes:

  • Add internal grapheme-boundary snapping (snapToGrapheme) before measuring cursor width.
  • Update cursorLayout() to use the snapped cursor position for width measurement.
  • Add a dedicated vitest suite covering emoji / grapheme edge cases and clamping behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
ui-tui/src/lib/inputMetrics.ts Adds grapheme snapping and applies it in cursorLayout() to avoid mid-grapheme width mismeasurement.
ui-tui/src/tests/cursorLayoutEmoji.test.ts Adds regression tests for surrogate pairs, flags, ZWJ sequences, and boundary clamping.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +108 to +121
function snapToGrapheme(value: string, pos: number): number {
if (pos <= 0 || pos >= value.length) {
return pos
}

let snapped = 0

for (const { index, segment } of seg().segment(value)) {
const end = index + segment.length

if (end > pos) {
return snapped
}

@alt-glitch alt-glitch added type/bug Something isn't working comp/tui Terminal UI (ui-tui/ + tui_gateway/) P3 Low — cosmetic, nice to have labels May 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tui Terminal UI (ui-tui/ + tui_gateway/) P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants