Skip to content

fix: clip CJK split rows by terminal cell width#287

Open
iamken1204 wants to merge 2 commits intomodem-dev:mainfrom
iamken1204:fix-diff-cjk-content
Open

fix: clip CJK split rows by terminal cell width#287
iamken1204 wants to merge 2 commits intomodem-dev:mainfrom
iamken1204:fix-diff-cjk-content

Conversation

@iamken1204
Copy link
Copy Markdown
Contributor

Fixes split-view rendering for long CJK lines, especially untracked files with Mandarin content, where mouse-wheel repainting could let wide characters spill past the right split pane into the left side.

Changes:

  • Measure diff code and UI text by terminal cell width instead of UTF-16 string length.
  • Slice, pad, wrap, and horizontally scroll styled diff spans without splitting fullwidth characters.
  • Add Mandarin-only long-line regression coverage for mouse-wheel split-view repainting.
  • Add Japanese and Korean unit coverage for CJK width and clipping behavior.

Before
CleanShot 2026-05-11 at 15 39 40@2x

After
CleanShot 2026-05-11 at 15 39 59@2x

Measure and slice diff text by terminal cell width so long CJK lines stay within split panes after mouse-wheel repaints.

Add unit coverage for Chinese, Japanese, and Korean width handling plus a PTY regression for untracked Mandarin-only content in split nowrap mode.
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 11, 2026

Greptile Summary

This PR fixes split-view rendering for long CJK lines by replacing UTF-16 .length measurements with a proper terminal-cell-width calculation that counts fullwidth characters as two cells. New terminalCellWidth and sliceTextByTerminalCells utilities in text.ts are wired into codeColumns.ts, renderRows.tsx (fitText, sliceSpansWindow, wrapSpans), and padText, eliminating the wide-character overflow that let Mandarin content spill from the right split pane into the left.

  • src/ui/lib/text.ts adds hand-rolled isWideCodePoint / isZeroWidthCodePoint helpers and a sliceTextByTerminalCells function that skips fullwidth glyphs that straddle a boundary rather than splitting them mid-character.
  • src/ui/diff/renderRows.tsx rewrites sliceSpansWindow and wrapSpans to use cell-width accounting; wrapSpans iterates codepoint-by-codepoint so no glyph is split across a visual wrap.
  • Regression coverage is added at both the unit level (Chinese / Japanese / Korean assertions) and the PTY integration level (live scroll repaint with a 40-line Mandarin fixture).

Confidence Score: 3/5

Safe to merge for the reported vertical-overflow bug, but horizontal scrolling of CJK lines can produce misaligned output due to an unaddressed edge case in sliceSpansWindow.

When a user horizontally scrolls a CJK line to an offset that falls inside a 2-cell wide character, sliceTextByTerminalCells correctly skips the glyph but returns clipped=false and width=0, giving sliceSpansWindow no signal to deduct the blank right-half cell from its remaining budget. The next span's content then fills that blank cell, shifting everything one cell to the left. This happens any time the horizontal scroll offset is odd and the character at that boundary is a fullwidth CJK glyph.

src/ui/lib/text.ts (sliceTextByTerminalCells, lines 108-113) and src/ui/diff/renderRows.tsx (sliceSpansWindow, the text.length===0 / continue branch) together own the edge case. All test and harness files look correct.

Important Files Changed

Filename Overview
src/ui/lib/text.ts New terminal-cell-width measurement and slicing utilities. Core logic is correct for the straight-line case, but sliceTextByTerminalCells does not communicate blank cells consumed by partially-visible wide glyphs, causing a misalignment in sliceSpansWindow when horizontal offset lands inside a wide character.
src/ui/diff/renderRows.tsx fitText, sliceSpansWindow, and wrapSpans updated to use terminal cell widths. wrapSpans correctly handles per-codepoint wrapping. sliceSpansWindow inherits the partial-wide-char edge case from text.ts.
src/ui/diff/codeColumns.ts measureRenderedCodeLineWidth now uses terminalCellWidth instead of .length — straightforward and correct.
src/ui/AppHost.scroll-regression.test.tsx Adds a Mandarin untracked-file scroll regression test that covers the main reported bug (vertical CJK overflow after mouse-wheel repaint).
test/pty/ui-integration.test.ts Adds a live PTY test that launches hunk in split mode and verifies CJK additions stay within their pane after a scroll — directly reproduces the reported bug.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[diff line spans] --> B[sliceSpansWindow\noffset, width]
    B --> C{remainingOffset\n>= spanWidth?}
    C -- yes --> D[skip span entirely\nsubtract spanWidth]
    D --> B
    C -- no --> E[sliceTextByTerminalCells\nspan.text, offset, remaining]
    E --> F{clipped?}
    F -- yes --> G[append slice\nbreak outer loop]
    F -- no --> H{text empty?}
    H -- yes, clipped+offset=0 --> I[break: wide char\ndoes not fit]
    H -- yes, otherwise --> J[continue to next span\n⚠️ blank right-half\nnot deducted from remaining]
    H -- no --> K[append slice\nsubtract textWidth from remaining]
    K --> B
    G --> L[renderInlineSpans\npad to full width]
    I --> L
    J --> B
    L --> M[React spans output]
Loading
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
src/ui/lib/text.ts:108-113
**Skipped wide-char right-half not deducted from remaining window**

When a wide character straddles `windowStart` (e.g., offset=1, first char is 2-cell "中"), the function skips the glyph and returns `{ clipped: false, text: "", width: 0 }`. The caller (`sliceSpansWindow`) sees `clipped=false` and moves on to the next span with `remaining` unchanged, which causes the next span's first character to be rendered at the blank position that the skipped glyph's right half should occupy. Concretely: with spans `["中", "文"]` and `offset=1, width=3`, the output will be `"文"` rather than a 1-cell blank followed by nothing — every character in a CJK-heavy line appears one cell too far to the left whenever the horizontal scroll offset lands inside a wide glyph.

The blank right-half cells consumed by the skipped glyph need to be deducted from `remaining` (or reported back to the caller) so that `sliceSpansWindow` does not pull in content from subsequent spans to fill them.

Reviews (1): Last reviewed commit: "fix(ui): clip CJK split rows by cell wid..." | Re-trigger Greptile

Comment thread src/ui/lib/text.ts
Comment on lines +108 to +113
// If the requested window starts in the middle of a fullwidth glyph, omit that glyph entirely.
if (cellCursor < windowStart) {
cellCursor = nextCellCursor;
includedPreviousVisibleCodePoint = false;
continue;
}
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.

P1 Skipped wide-char right-half not deducted from remaining window

When a wide character straddles windowStart (e.g., offset=1, first char is 2-cell "中"), the function skips the glyph and returns { clipped: false, text: "", width: 0 }. The caller (sliceSpansWindow) sees clipped=false and moves on to the next span with remaining unchanged, which causes the next span's first character to be rendered at the blank position that the skipped glyph's right half should occupy. Concretely: with spans ["中", "文"] and offset=1, width=3, the output will be "文" rather than a 1-cell blank followed by nothing — every character in a CJK-heavy line appears one cell too far to the left whenever the horizontal scroll offset lands inside a wide glyph.

The blank right-half cells consumed by the skipped glyph need to be deducted from remaining (or reported back to the caller) so that sliceSpansWindow does not pull in content from subsequent spans to fill them.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ui/lib/text.ts
Line: 108-113

Comment:
**Skipped wide-char right-half not deducted from remaining window**

When a wide character straddles `windowStart` (e.g., offset=1, first char is 2-cell "中"), the function skips the glyph and returns `{ clipped: false, text: "", width: 0 }`. The caller (`sliceSpansWindow`) sees `clipped=false` and moves on to the next span with `remaining` unchanged, which causes the next span's first character to be rendered at the blank position that the skipped glyph's right half should occupy. Concretely: with spans `["中", "文"]` and `offset=1, width=3`, the output will be `"文"` rather than a 1-cell blank followed by nothing — every character in a CJK-heavy line appears one cell too far to the left whenever the horizontal scroll offset lands inside a wide glyph.

The blank right-half cells consumed by the skipped glyph need to be deducted from `remaining` (or reported back to the caller) so that `sliceSpansWindow` does not pull in content from subsequent spans to fill them.

How can I resolve this? If you propose a fix, please make it concise.

Reserve the hidden half-cell when horizontal scrolling starts inside a fullwidth glyph so later spans do not shift left.

Add unit coverage for the mid-glyph CJK offset boundary.
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