fix(maestro): render accurate diffs
This commit is contained in:
@@ -42,6 +42,42 @@ describe("computeDiffLines", () => {
|
||||
]);
|
||||
});
|
||||
|
||||
it("handles an insertion without changing surrounding lines", () => {
|
||||
expect(texts(computeDiffLines("a\nc\n", "a\nb\nc\n"))).toEqual([
|
||||
["ctx", "a"],
|
||||
["add", "b"],
|
||||
["ctx", "c"],
|
||||
]);
|
||||
});
|
||||
|
||||
it("handles a deletion without changing surrounding lines", () => {
|
||||
expect(texts(computeDiffLines("a\nb\nc\n", "a\nc\n"))).toEqual([
|
||||
["ctx", "a"],
|
||||
["del", "b"],
|
||||
["ctx", "c"],
|
||||
]);
|
||||
});
|
||||
|
||||
it("renders two empty files without invented lines", () => {
|
||||
expect(computeDiffLines("", "")).toEqual([]);
|
||||
});
|
||||
|
||||
it("shows when a final newline is added", () => {
|
||||
expect(texts(computeDiffLines("a", "a\n"))).toEqual([
|
||||
["del", "a"],
|
||||
["eof", "\\ No newline at end of file"],
|
||||
["add", "a"],
|
||||
]);
|
||||
});
|
||||
|
||||
it("shows when a final newline is removed", () => {
|
||||
expect(texts(computeDiffLines("a\n", "a"))).toEqual([
|
||||
["del", "a"],
|
||||
["add", "a"],
|
||||
["eof", "\\ No newline at end of file"],
|
||||
]);
|
||||
});
|
||||
|
||||
it("collapses a long unchanged interior run, keeping 3 lines of context each side", () => {
|
||||
const common = Array.from({ length: 20 }, (_, i) => `line${i}`);
|
||||
const oldText = [...common.slice(0, 10), "OLD", ...common.slice(10)].join("\n");
|
||||
@@ -85,10 +121,22 @@ describe("computeDiffLines", () => {
|
||||
const lines = computeDiffLines("a\nb", "a\nb\nc");
|
||||
expect(texts(lines)).toEqual([
|
||||
["ctx", "a"],
|
||||
["ctx", "b"],
|
||||
["del", "b"],
|
||||
["eof", "\\ No newline at end of file"],
|
||||
["add", "b"],
|
||||
["add", "c"],
|
||||
["eof", "\\ No newline at end of file"],
|
||||
]);
|
||||
});
|
||||
|
||||
it("falls back to a bounded coarse diff for a pathological changed middle", () => {
|
||||
const oldLines = Array.from({ length: 501 }, (_, index) => `line${index}`);
|
||||
const newLines = [...oldLines.slice(1), oldLines[0]];
|
||||
const lines = computeDiffLines(`${oldLines.join("\n")}\n`, `${newLines.join("\n")}\n`);
|
||||
expect(lines).toHaveLength(1_002);
|
||||
expect(lines.slice(0, 501).every((line) => line.kind === "del")).toBe(true);
|
||||
expect(lines.slice(501).every((line) => line.kind === "add")).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("diffStats", () => {
|
||||
|
||||
+52
-30
@@ -2,7 +2,7 @@
|
||||
// hands over full `oldText`/`newText`, not a unified-diff patch, so the
|
||||
// coloured hunks the design calls for have to be computed here.
|
||||
|
||||
export type DiffLineKind = "add" | "del" | "ctx" | "hunk";
|
||||
export type DiffLineKind = "add" | "del" | "ctx" | "hunk" | "eof";
|
||||
|
||||
export interface DiffLine {
|
||||
kind: DiffLineKind;
|
||||
@@ -19,66 +19,82 @@ const CONTEXT = 3;
|
||||
* still correct, just not minimal. */
|
||||
const LCS_CELL_LIMIT = 250_000;
|
||||
|
||||
function splitLines(text: string): string[] {
|
||||
if (text === "") return [];
|
||||
const body = text.endsWith("\n") ? text.slice(0, -1) : text;
|
||||
return body.split("\n");
|
||||
interface SourceLine {
|
||||
text: string;
|
||||
terminated: boolean;
|
||||
}
|
||||
|
||||
type Op = { kind: "add" | "del" | "ctx"; text: string };
|
||||
function splitLines(text: string): SourceLine[] {
|
||||
if (text === "") return [];
|
||||
const parts = text.split("\n");
|
||||
const hasFinalNewline = text.endsWith("\n");
|
||||
if (hasFinalNewline) parts.pop();
|
||||
return parts.map((line, index) => ({
|
||||
text: line,
|
||||
terminated: index < parts.length - 1 || hasFinalNewline,
|
||||
}));
|
||||
}
|
||||
|
||||
type Op = { kind: "add" | "del" | "ctx"; line: SourceLine };
|
||||
|
||||
function sameLine(a: SourceLine, b: SourceLine): boolean {
|
||||
return a.text === b.text && a.terminated === b.terminated;
|
||||
}
|
||||
|
||||
/** Longest-common-subsequence diff of two line arrays, already trimmed of any
|
||||
* shared prefix/suffix by the caller. */
|
||||
function lcsDiff(a: string[], b: string[]): Op[] {
|
||||
function lcsDiff(a: SourceLine[], b: SourceLine[]): Op[] {
|
||||
const n = a.length;
|
||||
const m = b.length;
|
||||
if (n === 0) return b.map((text) => ({ kind: "add", text }));
|
||||
if (m === 0) return a.map((text) => ({ kind: "del", text }));
|
||||
if (n === 0) return b.map((line) => ({ kind: "add", line }));
|
||||
if (m === 0) return a.map((line) => ({ kind: "del", line }));
|
||||
if (n * m > LCS_CELL_LIMIT) {
|
||||
return [...a.map((text) => ({ kind: "del" as const, text })), ...b.map((text) => ({ kind: "add" as const, text }))];
|
||||
return [...a.map((line) => ({ kind: "del" as const, line })), ...b.map((line) => ({ kind: "add" as const, line }))];
|
||||
}
|
||||
// dp[i][j] = length of the LCS of a[i:] and b[j:].
|
||||
const dp: Uint32Array[] = Array.from({ length: n + 1 }, () => new Uint32Array(m + 1));
|
||||
const columns = m + 1;
|
||||
const dp = new Uint32Array((n + 1) * columns);
|
||||
for (let i = n - 1; i >= 0; i--) {
|
||||
for (let j = m - 1; j >= 0; j--) {
|
||||
dp[i][j] =
|
||||
a[i] === b[j] ? dp[i + 1][j + 1] + 1 : Math.max(dp[i + 1][j], dp[i][j + 1]);
|
||||
dp[i * columns + j] = sameLine(a[i], b[j])
|
||||
? dp[(i + 1) * columns + j + 1] + 1
|
||||
: Math.max(dp[(i + 1) * columns + j], dp[i * columns + j + 1]);
|
||||
}
|
||||
}
|
||||
const ops: Op[] = [];
|
||||
let i = 0;
|
||||
let j = 0;
|
||||
while (i < n && j < m) {
|
||||
if (a[i] === b[j]) {
|
||||
ops.push({ kind: "ctx", text: a[i] });
|
||||
if (sameLine(a[i], b[j])) {
|
||||
ops.push({ kind: "ctx", line: a[i] });
|
||||
i++;
|
||||
j++;
|
||||
} else if (dp[i + 1][j] >= dp[i][j + 1]) {
|
||||
ops.push({ kind: "del", text: a[i] });
|
||||
} else if (dp[(i + 1) * columns + j] >= dp[i * columns + j + 1]) {
|
||||
ops.push({ kind: "del", line: a[i] });
|
||||
i++;
|
||||
} else {
|
||||
ops.push({ kind: "add", text: b[j] });
|
||||
ops.push({ kind: "add", line: b[j] });
|
||||
j++;
|
||||
}
|
||||
}
|
||||
while (i < n) ops.push({ kind: "del", text: a[i++] });
|
||||
while (j < m) ops.push({ kind: "add", text: b[j++] });
|
||||
while (i < n) ops.push({ kind: "del", line: a[i++] });
|
||||
while (j < m) ops.push({ kind: "add", line: b[j++] });
|
||||
return ops;
|
||||
}
|
||||
|
||||
function diffLines(a: string[], b: string[]): Op[] {
|
||||
function diffLines(a: SourceLine[], b: SourceLine[]): Op[] {
|
||||
let pre = 0;
|
||||
while (pre < a.length && pre < b.length && a[pre] === b[pre]) pre++;
|
||||
while (pre < a.length && pre < b.length && sameLine(a[pre], b[pre])) pre++;
|
||||
let suf = 0;
|
||||
while (
|
||||
suf < a.length - pre &&
|
||||
suf < b.length - pre &&
|
||||
a[a.length - 1 - suf] === b[b.length - 1 - suf]
|
||||
sameLine(a[a.length - 1 - suf], b[b.length - 1 - suf])
|
||||
) {
|
||||
suf++;
|
||||
}
|
||||
const prefixCtx: Op[] = a.slice(0, pre).map((text) => ({ kind: "ctx", text }));
|
||||
const suffixCtx: Op[] = a.slice(a.length - suf).map((text) => ({ kind: "ctx", text }));
|
||||
const prefixCtx: Op[] = a.slice(0, pre).map((line) => ({ kind: "ctx", line }));
|
||||
const suffixCtx: Op[] = a.slice(a.length - suf).map((line) => ({ kind: "ctx", line }));
|
||||
const middle = lcsDiff(a.slice(pre, a.length - suf), b.slice(pre, b.length - suf));
|
||||
return [...prefixCtx, ...middle, ...suffixCtx];
|
||||
}
|
||||
@@ -91,10 +107,16 @@ function diffLines(a: string[], b: string[]): Op[] {
|
||||
* (unless it is the *only* run, i.e. nothing changed at all). */
|
||||
function hunk(ops: Op[]): DiffLine[] {
|
||||
const out: DiffLine[] = [];
|
||||
const push = (op: Op) => {
|
||||
out.push({ kind: op.kind, text: op.line.text });
|
||||
if (op.kind !== "ctx" && !op.line.terminated) {
|
||||
out.push({ kind: "eof", text: "\\ No newline at end of file" });
|
||||
}
|
||||
};
|
||||
let i = 0;
|
||||
while (i < ops.length) {
|
||||
if (ops[i].kind !== "ctx") {
|
||||
out.push(ops[i]);
|
||||
push(ops[i]);
|
||||
i++;
|
||||
continue;
|
||||
}
|
||||
@@ -105,14 +127,14 @@ function hunk(ops: Op[]): DiffLine[] {
|
||||
const keepHead = wholeDiff || i > 0 ? CONTEXT : 0;
|
||||
const keepTail = wholeDiff || j < ops.length ? CONTEXT : 0;
|
||||
if (run.length <= keepHead + keepTail) {
|
||||
out.push(...run);
|
||||
run.forEach(push);
|
||||
} else {
|
||||
if (keepHead) out.push(...run.slice(0, keepHead));
|
||||
if (keepHead) run.slice(0, keepHead).forEach(push);
|
||||
out.push({
|
||||
kind: "hunk",
|
||||
text: `⋯ ${run.length - keepHead - keepTail} unchanged lines ⋯`,
|
||||
});
|
||||
if (keepTail) out.push(...run.slice(run.length - keepTail));
|
||||
if (keepTail) run.slice(run.length - keepTail).forEach(push);
|
||||
}
|
||||
i = j;
|
||||
}
|
||||
@@ -123,7 +145,7 @@ function hunk(ops: Op[]): DiffLine[] {
|
||||
* (a new file) renders every line of `newText` as added. */
|
||||
export function computeDiffLines(oldText: string | null | undefined, newText: string): DiffLine[] {
|
||||
const newLines = splitLines(newText);
|
||||
if (oldText == null) return newLines.map((text) => ({ kind: "add", text }));
|
||||
if (oldText == null) return hunk(newLines.map((line) => ({ kind: "add", line })));
|
||||
const ops = diffLines(splitLines(oldText), newLines);
|
||||
return hunk(ops);
|
||||
}
|
||||
|
||||
@@ -157,6 +157,17 @@ describe("buildTranscriptRow: tool diffs", () => {
|
||||
// stylesheet keys `overflow-x: auto` off, not a computed style.
|
||||
expect(el.querySelector(".st-diff")!.className).toBe("st-diff");
|
||||
});
|
||||
|
||||
it("renders a visible marker for a removed final newline", () => {
|
||||
const el = buildTranscriptRow(
|
||||
row("tool_diff", {
|
||||
title: "Edit(x.rs)",
|
||||
content: [{ type: "diff", path: "/repo/x.rs", oldText: "same\n", newText: "same" }],
|
||||
}),
|
||||
);
|
||||
(el.querySelector(".st-tool-head") as HTMLButtonElement).click();
|
||||
expect(el.querySelector(".st-ln-eof")!.textContent).toBe("\\ No newline at end of file");
|
||||
});
|
||||
});
|
||||
|
||||
describe("buildTranscriptRow: approval", () => {
|
||||
|
||||
@@ -2509,6 +2509,11 @@ input.leaf-rename:focus {
|
||||
background: var(--surface-2);
|
||||
}
|
||||
|
||||
.st-ln-eof {
|
||||
color: var(--faint);
|
||||
font-style: italic;
|
||||
}
|
||||
|
||||
/* the one lifted, card-like element on the surface: the only thing the
|
||||
operator owes a decision on */
|
||||
.st-approval {
|
||||
|
||||
Reference in New Issue
Block a user