Skip to content

fix(diff): use stringWidth for CJK-aware truncation in SplitDiff#1683

Closed
Bernardxu123 wants to merge 1 commit into
esengine:mainfrom
Bernardxu123:fix/split-diff-cjk-width
Closed

fix(diff): use stringWidth for CJK-aware truncation in SplitDiff#1683
Bernardxu123 wants to merge 1 commit into
esengine:mainfrom
Bernardxu123:fix/split-diff-cjk-width

Conversation

@Bernardxu123

Copy link
Copy Markdown
Collaborator

Problem

In review mode, the diff panel calculates wrong character width when CJK (Chinese/Japanese/Korean) characters are present. This causes incorrect border positions and visual misalignment.

Root Cause

In src/cli/ui/SplitDiff.tsx, the Cell function uses raw.length to calculate visual width for truncation:

const truncated = raw.length > inner ? `${raw.slice(0, inner - 1)}…` : raw;

The problem is that raw.length counts JavaScript characters, not visual terminal cells. For CJK characters:

  • raw.length counts each character as 1
  • Terminal renders each CJK character as 2 cells wide

This mismatch causes the truncation calculation to be wrong, resulting in incorrect border positions when diff lines contain Chinese characters.

Example

A diff line with Chinese characters like 函数返回值 has:

  • raw.length = 5 (5 JavaScript characters)
  • Actual visual width = 10 cells (each CJK char takes 2 cells)

When truncating to fit a column of width 8, the code thinks 5 < 8 so no truncation is needed, but the actual visual width is 10, which overflows.

Solution

Replace raw.length with stringWidth(raw) from the existing text-width.js utility:

import { stringWidth } from "./text-width.js";

// Before
const truncated = raw.length > inner ? `${raw.slice(0, inner - 1)}…` : raw;

// After
const truncated = stringWidth(raw) > inner ? `${raw.slice(0, inner - 1)}…` : raw;

The stringWidth function (from the string-width library) correctly handles:

  • CJK characters (2 cells wide)
  • Emoji (2 cells wide)
  • Zero-width characters (0 cells)
  • Regular ASCII (1 cell wide)

Files Changed

  • src/cli/ui/SplitDiff.tsx — Import stringWidth and use it for truncation calculation

Testing

  • ✅ Lint passed
  • ✅ Typecheck passed
  • ✅ All 3596 tests passed

Impact

This fix ensures the diff panel renders correctly regardless of the characters in the diff content. Users working with CJK characters (Chinese, Japanese, Korean) will now see correct border positions and proper text truncation.

Fixes #1671

The diff panel was using raw.length to calculate visual width for
truncation, which undercounts CJK characters (each takes 2 terminal
cells but raw.length counts it as 1). This caused incorrect border
positions when diff lines contained Chinese characters.

Fix by using stringWidth() from text-width.js which correctly handles
CJK, emoji, and other wide characters.

Fixes esengine#1671
esengine added a commit that referenced this pull request May 24, 2026
#1686)

Two bugs in the same `Cell` body (#1671):

- truncation used `raw.length > inner`, which undercounts wide chars —
  a CJK row that was visually too wide for the column slipped past the
  check and overflowed into the gutter
- padding used `padEnd(inner)`, which counts string length not cells —
  short CJK content got padded as if each wide char were 1 cell, so the
  background-color stripe ran past the column edge

Repo already has a cell-aware clipper (`clipToCells`) for the cut and
a private `padToCells` in `markdown.tsx` for the pad. Promote
`padToCells` to `text-width.ts` so SplitDiff can use both — collapses
the original four lines (slice + ternary + padEnd + ascii comment) to
a single `padToCells(clipToCells(raw, inner), inner)` and drops the
duplicate definition from `markdown.tsx`.

Supersedes #1683, which fixed the detection half but left the slice +
padEnd in place — for CJK content the truncated string ended up wider
than `inner` and short rows still over-padded.

Co-authored-by: reasonix <reasonix@deepseek.com>
Co-authored-by: Bernardxu123 <Bernardxu123@users.noreply.github.com>
@esengine

Copy link
Copy Markdown
Owner

Thanks for spotting the bug. Shipped the full fix as #1686 (merged) — used clipToCells for the truncation and a promoted padToCells for the right-pad, both already living in src/cli/ui/text-width.ts. Co-authored credit preserved.

Closing in favor of #1686 because the stringWidth check alone left two halves of the same bug in place: raw.slice(0, inner - 1) still cuts by JS chars (so the truncated CJK string ends up wider than inner cells), and truncated.padEnd(inner) still pads by string length (so short CJK rows get over-padded). Both knock the column border out of alignment in the same way the original bug did.

@esengine esengine closed this May 24, 2026
@Bernardxu123 Bernardxu123 deleted the fix/split-diff-cjk-width branch May 25, 2026 06:36
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.

Wrong character witdh in diff panel

2 participants