Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions src/ui/AppHost.scroll-regression.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,27 @@ function createScrollBootstrap(): AppBootstrap {
});
}

function createCjkUntrackedScrollBootstrap(): AppBootstrap {
const cjkPhrase = "這是一段很長的中文內容用來驗證分割視圖滑鼠捲動";
const after = Array.from(
{ length: 40 },
(_, index) => `第${String(index + 1).padStart(2, "0")}行${cjkPhrase.repeat(5)}\n`,
).join("");

return createTestVcsAppBootstrap({
changesetId: "scroll-regression-cjk",
files: [
createTestDiffFile({
after,
before: "",
context: 3,
id: "cjk-new",
path: "notes.md",
}),
],
});
}

describe("UI scroll regression", () => {
test("keeps split diff lines intact after a wheel scroll repaint", async () => {
const setup = await testRender(<AppHost bootstrap={createScrollBootstrap()} />, {
Expand Down Expand Up @@ -69,4 +90,33 @@ describe("UI scroll regression", () => {
});
}
});

test("clips CJK split additions after a wheel scroll repaint", async () => {
const setup = await testRender(<AppHost bootstrap={createCjkUntrackedScrollBootstrap()} />, {
width: 80,
height: 14,
});

try {
await act(async () => {
await setup.renderOnce();
await Bun.sleep(100);
await setup.renderOnce();
});

await act(async () => {
await setup.mockMouse.scroll(50, 8, "down");
await Bun.sleep(0);
await setup.renderOnce();
});

const scrolledFrame = setup.captureCharFrame();
expect(scrolledFrame).toContain("▌ 4 + 第04行這是一段很長的中文");
expect(scrolledFrame).not.toMatch(/\n[^\S\r\n]*滑鼠捲動\s*\n/);
} finally {
await act(async () => {
setup.renderer.destroy();
});
}
});
});
11 changes: 7 additions & 4 deletions src/ui/diff/codeColumns.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { describe, expect, test } from "bun:test";
import type { DiffFile } from "../../core/types";
import { findMaxLineNumberInRows, maxFileCodeLineWidth } from "./codeColumns";
import { findMaxLineNumberInRows, maxFileCodeLineWidth, measureRenderedCodeLineWidth } from "./codeColumns";
import type { DiffRow } from "./pierre";

/** Generate a large diff metadata fixture without checking a huge file into the repo. */
Expand Down Expand Up @@ -30,10 +30,13 @@ describe("code column measurement", () => {
expect(maxFileCodeLineWidth(file)).toBe("the widest generated line".length);
});

test("counts wide CJK characters by terminal cells", () => {
const file = createLargeLineFixture(2, "日本語");
test("measures CJK lines in terminal cells", () => {
const file = createLargeLineFixture(3, "かなカナ漢字 mixed");

expect(maxFileCodeLineWidth(file)).toBe(6);
expect(measureRenderedCodeLineWidth("中文 mixed")).toBe(10);
expect(measureRenderedCodeLineWidth("かなカナ漢字 mixed")).toBe(18);
expect(measureRenderedCodeLineWidth("한글 테스트 mixed")).toBe(17);
expect(maxFileCodeLineWidth(file)).toBe(18);
});
});

Expand Down
4 changes: 2 additions & 2 deletions src/ui/diff/codeColumns.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { DiffFile, LayoutMode } from "../../core/types";
import { measureTextWidth } from "../lib/text";
import { terminalCellWidth } from "../lib/text";
import type { DiffRow } from "./pierre";

export const DIFF_CODE_TAB_WIDTH = 2;
Expand All @@ -15,7 +15,7 @@ export function expandDiffTabs(text: string) {

/** Measure one rendered code line after tab expansion and newline trimming. */
export function measureRenderedCodeLineWidth(line: string | undefined) {
return measureTextWidth(expandDiffTabs((line ?? "").replace(/\n$/, "")));
return terminalCellWidth(expandDiffTabs((line ?? "").replace(/\n$/, "")));
}

/** Track the widest rendered code line for one file. */
Expand Down
170 changes: 127 additions & 43 deletions src/ui/lib/text.ts
Original file line number Diff line number Diff line change
@@ -1,69 +1,153 @@
import stringWidth from "string-width";
import { sanitizeTerminalLine } from "../../lib/terminalText";

const printableAsciiRegex = /^[\u0020-\u007E]*$/;
const graphemeSegmenter =
typeof Intl !== "undefined" && "Segmenter" in Intl
? new Intl.Segmenter(undefined, { granularity: "grapheme" })
: null;
/** Return whether a Unicode code point has zero visible terminal width. */
function isZeroWidthCodePoint(codePoint: number) {
return (
codePoint === 0 ||
codePoint === 0x200b ||
codePoint === 0x200c ||
codePoint === 0x200d ||
codePoint === 0xfeff ||
(codePoint >= 0x0001 && codePoint <= 0x001f) ||
(codePoint >= 0x007f && codePoint <= 0x009f) ||
(codePoint >= 0x0300 && codePoint <= 0x036f) ||
(codePoint >= 0x1ab0 && codePoint <= 0x1aff) ||
(codePoint >= 0x1dc0 && codePoint <= 0x1dff) ||
(codePoint >= 0x20d0 && codePoint <= 0x20ff) ||
(codePoint >= 0xfe00 && codePoint <= 0xfe0f) ||
(codePoint >= 0xfe20 && codePoint <= 0xfe2f) ||
(codePoint >= 0xe0100 && codePoint <= 0xe01ef)
);
}

/** Return whether a Unicode code point normally occupies two terminal cells. */
function isWideCodePoint(codePoint: number) {
return (
codePoint >= 0x1100 &&
(codePoint <= 0x115f ||
codePoint === 0x2329 ||
codePoint === 0x232a ||
(codePoint >= 0x2e80 && codePoint <= 0xa4cf && codePoint !== 0x303f) ||
(codePoint >= 0xac00 && codePoint <= 0xd7a3) ||
(codePoint >= 0xf900 && codePoint <= 0xfaff) ||
(codePoint >= 0xfe10 && codePoint <= 0xfe19) ||
(codePoint >= 0xfe30 && codePoint <= 0xfe6f) ||
(codePoint >= 0xff00 && codePoint <= 0xff60) ||
(codePoint >= 0xffe0 && codePoint <= 0xffe6) ||
(codePoint >= 0x1f300 && codePoint <= 0x1f64f) ||
(codePoint >= 0x1f900 && codePoint <= 0x1f9ff) ||
(codePoint >= 0x20000 && codePoint <= 0x3fffd))
);
}

/** Iterate user-visible text clusters so wide and combining characters stay together. */
function textClusters(text: string) {
if (!graphemeSegmenter) {
return Array.from(text);
/** Measure one Unicode code point in terminal cells. */
function codePointCellWidth(codePoint: number) {
if (isZeroWidthCodePoint(codePoint)) {
return 0;
}

return Array.from(graphemeSegmenter.segment(text), (segment) => segment.segment);
return isWideCodePoint(codePoint) ? 2 : 1;
}

function measureSanitizedTextWidth(text: string) {
return printableAsciiRegex.test(text) ? text.length : stringWidth(text);
/** Measure rendered text in terminal cells, counting CJK/fullwidth characters as two cells. */
export function terminalCellWidth(text: string) {
let width = 0;

for (let index = 0; index < text.length; ) {
const codePoint = text.codePointAt(index);
if (codePoint === undefined) {
break;
}

width += codePointCellWidth(codePoint);
index += codePoint > 0xffff ? 2 : 1;
}

return width;
}

/** Measure text in terminal cells, treating CJK and emoji clusters as wide. */
/** Measure sanitized text in terminal cells, treating CJK and emoji as wide. */
export function measureTextWidth(text: string) {
return measureSanitizedTextWidth(sanitizeTerminalLine(text));
return terminalCellWidth(sanitizeTerminalLine(text));
}

/** Slice text by terminal cells without splitting wide or combining clusters. */
export function sliceTextByWidth(text: string, offset: number, width: number) {
const safeText = sanitizeTerminalLine(text);
const startOffset = Math.max(0, offset);
const maxWidth = Math.max(0, width);
if (maxWidth === 0) {
return { text: "", width: 0 };
}

if (printableAsciiRegex.test(safeText)) {
const sliced = safeText.slice(startOffset, startOffset + maxWidth);
return { text: sliced, width: sliced.length };
/** Slice text to a visible terminal-cell window without splitting fullwidth characters. */
export function sliceTextByTerminalCells(text: string, offset: number, width: number) {
if (width <= 0) {
return { clipped: terminalCellWidth(text) > Math.max(0, offset), text: "", width: 0 };
}

let cursor = 0;
const windowStart = Math.max(0, offset);
const windowEnd = windowStart + width;
let cellCursor = 0;
let output = "";
let usedWidth = 0;
let visibleText = "";
let clipped = false;
let includedPreviousVisibleCodePoint = false;

for (const cluster of textClusters(safeText)) {
const clusterWidth = measureSanitizedTextWidth(cluster);
const clusterStart = cursor;
const clusterEnd = cursor + clusterWidth;
cursor = clusterEnd;
for (let index = 0; index < text.length; ) {
const codePoint = text.codePointAt(index);
if (codePoint === undefined) {
break;
}

if (clusterEnd <= startOffset) {
const char = String.fromCodePoint(codePoint);
const charWidth = codePointCellWidth(codePoint);
const nextCellCursor = cellCursor + charWidth;
index += codePoint > 0xffff ? 2 : 1;

if (charWidth === 0) {
if (
includedPreviousVisibleCodePoint ||
(output.length > 0 && cellCursor >= windowStart && cellCursor <= windowEnd)
) {
output += char;
}
continue;
}
if (clusterStart < startOffset) {

if (nextCellCursor <= windowStart) {
cellCursor = nextCellCursor;
includedPreviousVisibleCodePoint = false;
continue;
}
if (usedWidth + clusterWidth > maxWidth) {

// If the requested window starts in the middle of a fullwidth glyph, omit that glyph entirely.
if (cellCursor < windowStart) {
const hiddenCellWidth = Math.min(nextCellCursor, windowEnd) - windowStart;
if (hiddenCellWidth > 0) {
output += " ".repeat(hiddenCellWidth);
usedWidth += hiddenCellWidth;
}

cellCursor = nextCellCursor;
includedPreviousVisibleCodePoint = false;
continue;
}
Comment on lines +115 to +126
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.


if (cellCursor >= windowEnd || nextCellCursor > windowEnd) {
clipped = true;
break;
}

visibleText += cluster;
usedWidth += clusterWidth;
output += char;
usedWidth += charWidth;
cellCursor = nextCellCursor;
includedPreviousVisibleCodePoint = true;
}

return { text: visibleText, width: usedWidth };
return { clipped, text: output, width: usedWidth };
}

/** Slice sanitized text by terminal cells without splitting fullwidth characters. */
export function sliceTextByWidth(text: string, offset: number, width: number) {
const { text: sliced, width: usedWidth } = sliceTextByTerminalCells(
sanitizeTerminalLine(text),
offset,
width,
);

return { text: sliced, width: usedWidth };
}

/** Clamp text to a fixed width using a plain-dot terminal fallback marker. */
Expand All @@ -73,19 +157,19 @@ export function fitText(text: string, width: number) {
return "";
}

if (measureTextWidth(safeText) <= width) {
if (terminalCellWidth(safeText) <= width) {
return safeText;
}

if (width === 1) {
return ".";
}

return `${sliceTextByWidth(safeText, 0, width - 1).text}.`;
return `${sliceTextByTerminalCells(safeText, 0, width - 1).text}.`;
}

/** Clamp and then right-pad text to an exact width. */
export function padText(text: string, width: number) {
const trimmed = fitText(text, width);
return `${trimmed}${" ".repeat(Math.max(0, width - measureTextWidth(trimmed)))}`;
return `${trimmed}${" ".repeat(Math.max(0, width - terminalCellWidth(trimmed)))}`;
}
41 changes: 39 additions & 2 deletions src/ui/lib/ui-lib.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,14 @@ import {
isStepDownKey,
isStepUpKey,
} from "./keyboard";
import { fitText, measureTextWidth, padText, sliceTextByWidth } from "./text";
import {
fitText,
measureTextWidth,
padText,
sliceTextByTerminalCells,
sliceTextByWidth,
terminalCellWidth,
} from "./text";
import { computeHunkRevealScrollTop } from "./hunkScroll";
import {
estimateDiffSectionBodyRows,
Expand Down Expand Up @@ -301,9 +308,39 @@ describe("ui helpers", () => {
test("text helpers measure and slice wide characters by terminal cells", () => {
expect(measureTextWidth("日本語")).toBe(6);
expect(sliceTextByWidth("a日本b", 1, 4)).toEqual({ text: "日本", width: 4 });
expect(sliceTextByWidth("a日本b", 2, 4)).toEqual({ text: "本b", width: 3 });
expect(sliceTextByWidth("a日本b", 2, 4)).toEqual({ text: " 本b", width: 4 });
expect(fitText("日本語", 5)).toBe("日本.");
expect(measureTextWidth(padText("日本", 6))).toBe(6);
expect(terminalCellWidth("中文 mixed")).toBe(10);
expect(terminalCellWidth("かなカナ漢字 mixed")).toBe(18);
expect(terminalCellWidth("한글 테스트 mixed")).toBe(17);
expect(sliceTextByTerminalCells("中文 mixed", 0, 5)).toEqual({
clipped: true,
text: "中文 ",
width: 5,
});
expect(sliceTextByTerminalCells("かなmixed", 0, 5)).toEqual({
clipped: true,
text: "かなm",
width: 5,
});
expect(sliceTextByTerminalCells("한글 mixed", 0, 5)).toEqual({
clipped: true,
text: "한글 ",
width: 5,
});
expect(sliceTextByTerminalCells("中", 1, 3)).toEqual({
clipped: false,
text: " ",
width: 1,
});
expect(sliceTextByTerminalCells("中文", 1, 3)).toEqual({
clipped: false,
text: " 文",
width: 3,
});
expect(fitText("中文 mixed", 6)).toBe("中文 .");
expect(padText("中文", 5)).toBe("中文 ");
});

test("agent popover helpers wrap text and right-align the card within the viewport", () => {
Expand Down
Loading
Loading