Skip to content
Merged
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
5 changes: 5 additions & 0 deletions .changeset/wide-char-copy-selection.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"hunkdiff": patch
---

Fix mouse-selection copy misalignment on lines with wide (CJK, emoji) characters: drag, double-click, and triple-click selections now convert terminal cell columns into string indices before slicing, so the copied text matches the selected cells exactly. File-header rows with wide-character filenames now copy with the same cell alignment, and invisible zero-width characters at a selection boundary round-trip through the clipboard.
56 changes: 56 additions & 0 deletions src/ui/AppHost.selection.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { act } from "react";
import type { AppBootstrap } from "../core/types";
import { createTestVcsAppBootstrap } from "../../test/helpers/app-bootstrap";
import { createTestDiffFile, lines } from "../../test/helpers/diff-helpers";
import { measureTextWidth } from "./lib/text";

// These tests drive the DiffPane mouse-drag text-selection path end to end: begin/update/end
// copy-selection, double/triple-click word and line expansion, and the OSC 52 clipboard copy.
Expand Down Expand Up @@ -42,6 +43,24 @@ function createSplitSelectionBootstrap(): AppBootstrap {
return { ...bootstrap, initialMode: "split", input: { ...bootstrap.input } };
}

/** Build a diff whose changed line mixes full-width (CJK) and ASCII characters. */
function createWideCharSelectionBootstrap(): AppBootstrap {
return createTestVcsAppBootstrap({
changesetId: "changeset:copy-selection-cjk",
files: [
createTestDiffFile({
id: "i18n",
path: "i18n.ts",
before: lines("export const message = 'hello'; // greeting"),
after: lines("export const message = 'こんにちは'; // greeting"),
context: 1,
}),
],
initialMode: "stack",
initialCopyDecorations: true,
});
}

type Harness = Awaited<ReturnType<typeof testRender>>;

/** Settle pending renders so a frame reflects the latest interaction. */
Expand Down Expand Up @@ -158,6 +177,43 @@ describe("DiffPane copy selection", () => {
}
});

test("dragging across wide (CJK) characters copies exactly the selected cells", async () => {
const { setup, copied } = await renderSelectionApp(createWideCharSelectionBootstrap());

try {
const frame = setup.captureCharFrame();
const wide = locateText(frame, "こんにちは");
expect(wide).not.toBeNull();

// Everything left of the code is single-cell glyphs, so the string index of the code
// start is also its screen column; the drag end is then measured in terminal cells.
const row = frame.split("\n")[wide!.y]!;
const startX = row.indexOf("export const message");
expect(startX).toBeGreaterThanOrEqual(0);
const throughWide = "export const message = 'こんにちは";

await act(async () => {
await setup.mockMouse.drag(
startX,
wide!.y,
startX + measureTextWidth(throughWide) - 1,
wide!.y,
MouseButtons.LEFT,
);
});
await flush(setup);

// Cell-aware slicing keeps the copy aligned with the drag: without it, each full-width
// character shifted the endpoint and the clipboard over-included "'; //".
expect(copied.length).toBeGreaterThan(0);
expect(copied[copied.length - 1]).toBe(throughWide);
} finally {
await act(async () => {
setup.renderer.destroy();
});
}
});

test("double-clicking a token selects and copies the word under the pointer", async () => {
const { setup, copied } = await renderSelectionApp(createSelectionBootstrap());

Expand Down
269 changes: 269 additions & 0 deletions src/ui/components/panes/copySelection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
resolveStackCellGeometry,
resolveSplitPaneWidths,
} from "../../diff/codeColumns";
import { measureTextWidth } from "../../lib/text";

const OSC52_CLIPBOARD = "\x1b]52;c;SGVsbG8=\x07";
const CSI_CLEAR_SCREEN = "\x1b[2J";
Expand Down Expand Up @@ -100,6 +101,36 @@ function createMaliciousDiffFile(): DiffFile {
};
}

function createCjkDiffFile(): DiffFile {
const metadata = parseDiffFromFile(
{
name: "i18n.ts",
contents: "export const message = 'hello'; // greeting\n",
cacheKey: "cjk-before",
},
{
name: "i18n.ts",
contents: "export const message = 'こんにちは'; // greeting\n",
cacheKey: "cjk-after",
},
{ context: 3 },
true,
);

return {
id: "i18n",
path: "i18n.ts",
patch: "",
language: "typescript",
stats: {
additions: 1,
deletions: 1,
},
metadata,
agent: null,
};
}

function buildContext(
layout: "stack" | "split" = "stack",
width = 120,
Expand Down Expand Up @@ -713,3 +744,241 @@ describe("renderCopySelectionText in split with side", () => {
expect(text).toContain("export const answer = 42;");
});
});

describe("copy selection with wide (CJK) characters", () => {
// "export const message = 'こんにちは" is 29 code units but 34 terminal cells: each full-width
// character covers two cells, so cell columns and code-unit indices drift apart on this line.
const cjkLine = "export const message = 'こんにちは'; // greeting";
const throughWide = "export const message = 'こんにちは";

function buildCjkContext() {
const { context, fileSectionLayouts, sectionGeometry } = buildContext(
"stack",
120,
createCjkDiffFile(),
);
const section = fileSectionLayouts[0]!;
const geometry = sectionGeometry[0]!;
const { gutterWidth } = resolveStackCellGeometry(
context.width,
geometry.lineNumberDigits,
context.showLineNumbers,
DIFF_RAIL_PREFIX_WIDTH,
);
const codeStart = DIFF_RAIL_PREFIX_WIDTH + gutterWidth;
const codeOnlyContext: CopySelectionContext = { ...context, copyDecorations: false };

// Locate the CJK addition row through full-width row copies so the tests do not
// hard-code the planned-row layout.
let visualRow = -1;
for (let row = section.bodyTop; row < section.bodyTop + section.bodyHeight; row += 1) {
const text = renderCopySelectionText({
context: codeOnlyContext,
start: { kind: "review-row", column: 0, visualRow: row },
end: { kind: "review-row", column: context.width - 1, visualRow: row },
});
if (text.includes("こんにちは")) {
visualRow = row;
break;
}
}
expect(visualRow).toBeGreaterThanOrEqual(0);

return { context, codeOnlyContext, codeStart, visualRow };
}

test("code-only selection ending after the wide run copies exactly the selected cells", () => {
const { codeOnlyContext, codeStart, visualRow } = buildCjkContext();

const text = renderCopySelectionText({
context: codeOnlyContext,
start: { kind: "review-row", column: codeStart, visualRow },
end: {
kind: "review-row",
column: codeStart + measureTextWidth(throughWide) - 1,
visualRow,
},
});

expect(text).toBe(throughWide);
});

test("code-only selection starting after the wide run copies exactly the selected cells", () => {
const { codeOnlyContext, codeStart, visualRow } = buildCjkContext();

const text = renderCopySelectionText({
context: codeOnlyContext,
start: {
kind: "review-row",
column: codeStart + measureTextWidth(throughWide),
visualRow,
},
end: { kind: "review-row", column: codeOnlyContext.width - 1, visualRow },
});

expect(text).toBe("'; // greeting");
});

test("decorated selection ending after the wide run copies exactly the selected cells", () => {
const { context, codeStart, visualRow } = buildCjkContext();

const text = renderCopySelectionText({
context,
start: { kind: "review-row", column: codeStart, visualRow },
end: {
kind: "review-row",
column: codeStart + measureTextWidth(throughWide) - 1,
visualRow,
},
});

expect(text).toBe(throughWide);
});

test("double-click on a word after the wide run selects the word measured in cells", () => {
const { context, codeStart, visualRow } = buildCjkContext();
const wordStartCell = measureTextWidth("export const message = 'こんにちは'; // ");
const point: CopySelectionPoint = {
kind: "review-row",
// Click inside "greeting".
column: codeStart + wordStartCell + 2,
visualRow,
};

const result = expandSelectionPoint(point, 2, context);

expect(result).toEqual({
startCol: codeStart + wordStartCell,
endCol: codeStart + wordStartCell + "greeting".length - 1,
});
});

test("double-click on a wide character selects both of its terminal cells", () => {
const { context, codeStart, visualRow } = buildCjkContext();
// "ん" covers two cells; clicking the second cell must still select the whole character.
const wideCharStartCell = measureTextWidth("export const message = 'こ");
const point: CopySelectionPoint = {
kind: "review-row",
column: codeStart + wideCharStartCell + 1,
visualRow,
};

const result = expandSelectionPoint(point, 2, context);

expect(result).toEqual({
startCol: codeStart + wideCharStartCell,
endCol: codeStart + wideCharStartCell + 1,
});
});

test("code-only triple-click covers the full cell width of the line", () => {
const { codeOnlyContext, codeStart, visualRow } = buildCjkContext();
const point: CopySelectionPoint = {
kind: "review-row",
column: codeStart + 2,
visualRow,
};

const result = expandSelectionPoint(point, 3, codeOnlyContext);

expect(result).toEqual({
startCol: codeStart,
endCol: codeStart + measureTextWidth(cjkLine) - 1,
});
});

test("pinned-header selection stays cell-aligned for wide-character filenames", () => {
const metadata = parseDiffFromFile(
{ name: "日本語.ts", contents: "export const a = 1;\n", cacheKey: "cjk-path-before" },
{ name: "日本語.ts", contents: "export const a = 2;\n", cacheKey: "cjk-path-after" },
{ context: 3 },
true,
);
const file: DiffFile = {
id: "cjk-path",
path: "日本語.ts",
patch: "",
language: "typescript",
stats: { additions: 1, deletions: 1 },
metadata,
agent: null,
};
const { context, fileSectionLayouts } = buildContext("stack", 120, file);
const nextVisualRow = fileSectionLayouts[0]!.bodyTop;

const headerCells = (startColumn: number, endColumn: number) =>
renderCopySelectionText({
context,
start: { kind: "pinned-header", column: startColumn, fileId: "cjk-path", nextVisualRow },
end: { kind: "pinned-header", column: endColumn, fileId: "cjk-path", nextVisualRow },
});

// The label starts after one padding cell: "日本語.ts" spans cells 1..9.
expect(headerCells(1, 9)).toBe("日本語.ts");

// DiffFileHeaderRow right-aligns the stats, so "-1" ends at cell width - 3 (one trailing
// stats space plus one padding cell). A code-unit gap would shift these cells onto "+1".
expect(headerCells(context.width - 4, context.width - 3)).toBe("-1");
});
});

describe("copy selection with zero-width characters", () => {
test("selection starting at a zero-width boundary keeps the invisible character", () => {
// A mid-line U+200B survives rendering (only leading zero-width clusters are dropped by
// sliceTextByWidth), so copying must round-trip it for the invisible bug to stay visible
// in the pasted text.
const metadata = parseDiffFromFile(
{ name: "zero.ts", contents: "const x = 1;\n", cacheKey: "zero-before" },
{
name: "zero.ts",
contents: "const x = 1;\nconst zw = 'a\u200bb';\n",
cacheKey: "zero-after",
},
{ context: 3 },
true,
);
const file: DiffFile = {
id: "zero",
path: "zero.ts",
patch: "",
language: "typescript",
stats: { additions: 1, deletions: 0 },
metadata,
agent: null,
};
const { context, fileSectionLayouts, sectionGeometry } = buildContext("stack", 120, file);
const section = fileSectionLayouts[0]!;
const geometry = sectionGeometry[0]!;
const { gutterWidth } = resolveStackCellGeometry(
context.width,
geometry.lineNumberDigits,
context.showLineNumbers,
DIFF_RAIL_PREFIX_WIDTH,
);
const codeStart = DIFF_RAIL_PREFIX_WIDTH + gutterWidth;
const codeOnlyContext: CopySelectionContext = { ...context, copyDecorations: false };

let visualRow = -1;
for (let row = section.bodyTop; row < section.bodyTop + section.bodyHeight; row += 1) {
const text = renderCopySelectionText({
context: codeOnlyContext,
start: { kind: "review-row", column: 0, visualRow: row },
end: { kind: "review-row", column: context.width - 1, visualRow: row },
});
if (text.includes("zw")) {
visualRow = row;
break;
}
}
expect(visualRow).toBeGreaterThanOrEqual(0);

// "const zw = 'a" is 13 cells; the zero-width character sits at that boundary before "b".
const text = renderCopySelectionText({
context: codeOnlyContext,
start: { kind: "review-row", column: codeStart + 13, visualRow },
end: { kind: "review-row", column: codeOnlyContext.width - 1, visualRow },
});

expect(text).toBe("\u200bb';");
});
});
Loading