From 2a8e5e791d5c2b53a6f22b23e5a6871512f92771 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=83=A1=E7=BF=80?= <1020327414@qq.com> Date: Tue, 27 Oct 2026 18:54:15 +0800 Subject: [PATCH] fix(diff): Keep distant edits as separate chunks and render continuous change blocks (#95) In long files several distant edits could collapse into one large replacement that highlighted unchanged lines, and multi-line changes showed per-line stripes. Lines are now matched before character-level changes. The line diff is anchored on lines that occur once on both sides (patience-style), with a bounded diff only between anchors, so 50k-line files with distant edits keep separate chunks. Whole inserted or deleted lines render as continuous blocks; paired modified lines keep the line background plus word-level highlights, also inside multi-line chunks. Line decorations are built for the visible range only. Co-authored-by: zzxzz12345 Claude-Session: https://claude.ai/code/session_01VUYsdXA2nkF5v6zsC93dpn --- src/features/vcs/DiffEditor.tsx | 2 +- .../vcs/__tests__/diffOptions.test.ts | 149 +++++++++++++++++- src/features/vcs/cm.ts | 82 +++++++++- src/features/vcs/diffOptions.ts | 78 +++++++-- 4 files changed, 292 insertions(+), 19 deletions(-) diff --git a/src/features/vcs/DiffEditor.tsx b/src/features/vcs/DiffEditor.tsx index 5cf33d48d..398bd016c 100644 --- a/src/features/vcs/DiffEditor.tsx +++ b/src/features/vcs/DiffEditor.tsx @@ -216,7 +216,7 @@ export function DiffEditor({ input }: { input: EditorInput }) { }); const collapseUnchanged = opts.collapse ? { margin: 3, minSize: 6 } : undefined; const override = whitespaceDiff(opts.ignore); - const diffConfig = override ? { override } : undefined; + const diffConfig = { override }; const highlightChanges = opts.highlight === "words"; const inclusion = includable ? inclusionGutter(path, textA) : []; const side = (att: DocumentAttachment | null, editable: boolean, notes?: "left" | "right"): Extension[] => [ diff --git a/src/features/vcs/__tests__/diffOptions.test.ts b/src/features/vcs/__tests__/diffOptions.test.ts index 54be8b972..40b337d50 100644 --- a/src/features/vcs/__tests__/diffOptions.test.ts +++ b/src/features/vcs/__tests__/diffOptions.test.ts @@ -2,15 +2,22 @@ import { describe, expect, it } from "vitest"; import { Chunk, MergeView } from "@codemirror/merge"; import { Text } from "@codemirror/state"; import { applyChunk, arrowControls } from "../diffControls"; -import { DEFAULT_DIFF_OPTIONS, parseDiffOptions, transferChunk, whitespaceDiff, type IgnorePolicy } from "../diffOptions"; +import { + DEFAULT_DIFF_OPTIONS, + parseDiffOptions, + transferChunk, + whitespaceDiff, + type IgnorePolicy, +} from "../diffOptions"; import { clipboardDiffData, compareFilesData, parseTextsArg } from "../compare"; import { diffInput, diffTitle, sideFile, sideKey } from "../diffInput"; +import { viewerBase } from "../cm"; const doc = (s: string) => Text.of(s.split("\n")); function chunks(a: string, b: string, ignore: IgnorePolicy) { const override = whitespaceDiff(ignore); - return Chunk.build(doc(a), doc(b), override ? { override } : undefined); + return Chunk.build(doc(a), doc(b), { override }); } describe("diff options", () => { @@ -45,6 +52,140 @@ describe("diff options", () => { expect(c!.changes.every((ch) => ch.toB - ch.fromB < 20)).toBe(true); }); + it("keeps distant insertions separate in a long file", () => { + const lines = Array.from({ length: 1200 }, (_, i) => `line_${i}: ${"content ".repeat(8)}${i}`); + const changed = [...lines]; + for (let i = 1000; i >= 100; i -= 100) changed.splice(i, 0, `insert_${i} ${"x".repeat(90)}`); + const a = lines.join("\n"); + const b = changed.join("\n"); + const result = chunks(a, b, "none"); + expect(result).toHaveLength(10); + expect(result.every((c) => c.fromA === c.toA && c.toB - c.fromB < 120)).toBe(true); + let restored = doc(b); + for (const c of [...result].reverse()) { + const edit = transferChunk(c, "a", doc(a), restored); + restored = restored.replace(edit.from, edit.to, Text.of(edit.insert.split("\n"))); + } + expect(restored.toString()).toBe(a); + }); + + it("keeps distant edits separate in a very long file", () => { + const lines = Array.from({ length: 50000 }, (_, i) => `const v${i} = ${i * 7};`); + const changed = [...lines]; + for (let i = 49000; i > 0; i -= 2500) changed.splice(i, 1, `inserted ${i}`, `${changed[i]} // edited`); + const result = chunks(lines.join("\n"), changed.join("\n"), "none"); + expect(result).toHaveLength(20); + expect(result.every((c) => c.toB - c.fromB < 60)).toBe(true); + }); + + it("matches unchanged lines between many small edits", () => { + const lines = Array.from({ length: 4000 }, (_, i) => `line ${i}`); + const a = lines.join("\n"); + const b = lines.map((l, i) => (i % 2 ? `${l} changed` : l)).join("\n"); + expect(chunks(a, b, "none")).toHaveLength(2000); + expect(chunks(a, b, "whitespace")).toHaveLength(2000); + }); + + it("handles empty documents", () => { + expect(chunks("", "", "none")).toHaveLength(0); + const [ins] = chunks("", "one\ntwo\n", "none"); + expect(ins).toMatchObject({ fromA: 0, toA: 0, fromB: 0 }); + const [del] = chunks("one\ntwo\n", "", "whitespace"); + expect(del).toMatchObject({ fromA: 0, fromB: 0, toB: 0 }); + }); + + it("detects a missing final newline", () => { + for (const [a, b] of [ + ["same\n", "same"], + ["same", "same\n"], + ]) { + const [chunk] = chunks(a!, b!, "none"); + expect(chunk).toBeDefined(); + const edit = transferChunk(chunk!, "a", doc(a!), doc(b!)); + expect( + doc(b!) + .replace(edit.from, edit.to, Text.of(edit.insert.split("\n"))) + .toString(), + ).toBe(a); + } + }); + + it("decorates consecutive inserted and deleted lines as whole lines", () => { + const parent = document.createElement("div"); + document.body.appendChild(parent); + const mv = new MergeView({ + a: { doc: "before\nremoved one\nremoved two\nafter\n", extensions: viewerBase({ readOnly: true }) }, + b: { doc: "before\nafter\n", extensions: viewerBase({ readOnly: true }) }, + parent, + diffConfig: { override: whitespaceDiff("none") }, + }); + expect(mv.a.dom.querySelectorAll(".cm-diffWholeLine")).toHaveLength(2); + expect(mv.b.dom.querySelectorAll(".cm-diffWholeLine")).toHaveLength(0); + mv.destroy(); + parent.remove(); + }); + + it("keeps word highlighting on paired lines of a multi-line replacement", () => { + const parent = document.createElement("div"); + document.body.appendChild(parent); + const mv = new MergeView({ + a: { + doc: "before\nfrom old.first import X\nfrom old.second import Y\nafter\n", + extensions: viewerBase({ readOnly: true }), + }, + b: { + doc: "before\nfrom new.first import A\nfrom new.second import B\nadded line\nafter\n", + extensions: viewerBase({ readOnly: true }), + }, + parent, + diffConfig: { override: whitespaceDiff("none") }, + }); + expect(mv.a.dom.querySelectorAll(".cm-changedLine")).toHaveLength(2); + expect(mv.a.dom.querySelector(".cm-diffWholeLine")).toBeNull(); + expect([...mv.a.dom.querySelectorAll(".cm-changedText")].map((e) => e.textContent)).toEqual([ + "old", + "X", + "old", + "Y", + ]); + expect(mv.b.dom.querySelectorAll(".cm-changedLine")).toHaveLength(3); + expect([...mv.b.dom.querySelectorAll(".cm-diffWholeLine")].map((e) => e.textContent)).toEqual(["added line"]); + expect(mv.b.dom.querySelectorAll(".cm-changedLine:not(.cm-diffWholeLine) .cm-changedText")).toHaveLength(4); + mv.destroy(); + parent.remove(); + }); + + it("marks a deleted last line as a whole line", () => { + const parent = document.createElement("div"); + document.body.appendChild(parent); + const mv = new MergeView({ + a: { doc: "keep\ngone", extensions: viewerBase({ readOnly: true }) }, + b: { doc: "keep", extensions: viewerBase({ readOnly: true }) }, + parent, + diffConfig: { override: whitespaceDiff("none") }, + }); + expect([...mv.a.dom.querySelectorAll(".cm-diffWholeLine")].map((e) => e.textContent)).toEqual(["gone"]); + mv.destroy(); + parent.remove(); + }); + + it("keeps word highlighting for a single-line replacement", () => { + const parent = document.createElement("div"); + document.body.appendChild(parent); + const mv = new MergeView({ + a: { doc: "before\nvalue = old\nafter\n", extensions: viewerBase({ readOnly: true }) }, + b: { doc: "before\nvalue = new\nafter\n", extensions: viewerBase({ readOnly: true }) }, + parent, + diffConfig: { override: whitespaceDiff("none") }, + }); + expect(mv.a.dom.querySelector(".cm-diffWholeLine")).toBeNull(); + expect(mv.b.dom.querySelector(".cm-diffWholeLine")).toBeNull(); + expect(mv.a.dom.querySelector(".cm-changedText")).not.toBeNull(); + expect(mv.b.dom.querySelector(".cm-changedText")).not.toBeNull(); + mv.destroy(); + parent.remove(); + }); + it("transfers a chunk between sides (>> / << / Revert)", () => { const a = doc("x\nold\ny\n"); const b = doc("x\nnew\nnewer\ny\n"); @@ -105,7 +246,9 @@ describe("compare entry points", () => { const k1 = sideKey({ type: "text", text: "a", label: "Original" }); const k2 = sideKey({ type: "text", text: "b", label: "Original" }); expect(k1).not.toBe(k2); - expect(parseTextsArg({ path: "/p/a", left: { text: "1", label: "L" }, right: { text: "2", label: "R" } })).toMatchObject({ + expect( + parseTextsArg({ path: "/p/a", left: { text: "1", label: "L" }, right: { text: "2", label: "R" } }), + ).toMatchObject({ path: "/p/a", }); expect(parseTextsArg({ path: "/p/a", left: { text: 1 } })).toBeNull(); diff --git a/src/features/vcs/cm.ts b/src/features/vcs/cm.ts index a8b83d722..8e4dd3272 100644 --- a/src/features/vcs/cm.ts +++ b/src/features/vcs/cm.ts @@ -3,14 +3,79 @@ * based theme + syntax colours and lazy language loading by file name. */ import { HighlightStyle, LanguageDescription, syntaxHighlighting, type LanguageSupport } from "@codemirror/language"; -import { EditorState, type Extension } from "@codemirror/state"; -import { EditorView, drawSelection, highlightActiveLine, keymap, lineNumbers } from "@codemirror/view"; +import { EditorState, RangeSetBuilder, type Extension, type Line } from "@codemirror/state"; +import { + Decoration, + EditorView, + ViewPlugin, + drawSelection, + highlightActiveLine, + keymap, + lineNumbers, + type DecorationSet, + type ViewUpdate, +} from "@codemirror/view"; +import { getChunks } from "@codemirror/merge"; import { defaultKeymap } from "@codemirror/commands"; import { search, searchKeymap } from "@codemirror/search"; import { tags as t } from "@lezer/highlight"; +const wholeLine = Decoration.line({ class: "cm-diffWholeLine" }); + +/** + * Marks the changed lines that have no counterpart on the other side (inserted + * or deleted as a whole) so they get a solid background. Lines paired with a + * line on the other side keep the line tint plus word-level highlights. + */ +function wholeLineDeco(view: EditorView): DecorationSet { + const info = getChunks(view.state); + if (!info || !info.side) return Decoration.none; + const isA = info.side === "a"; + const { doc } = view.state; + const { from: vpFrom, to: vpTo } = view.viewport; + const builder = new RangeSetBuilder(); + for (const chunk of info.chunks) { + const from = isA ? chunk.fromA : chunk.fromB; + const to = isA ? chunk.toA : chunk.toB; + if (from >= vpTo) break; + if (from === to || to < vpFrom) continue; + const otherEmpty = isA ? chunk.fromB === chunk.toB : chunk.fromA === chunk.toA; + const ranges = chunk.changes.map((c) => (isA ? [c.fromA, c.toA] : [c.fromB, c.toB]).map((p) => p + from)); + // Covered including the line break after it, or the one before it (a deleted last line). + const covered = (line: Line) => + ranges.some(([f = 0, t = 0]) => (f <= line.from && t > line.to) || (f < line.from && t >= line.to)); + for (let line = doc.lineAt(Math.max(from, vpFrom)); line.from < Math.min(to, vpTo);) { + if (otherEmpty || covered(line)) builder.add(line.from, line.from, wholeLine); + if (line.number === doc.lines) break; + line = doc.line(line.number + 1); + } + } + return builder.finish(); +} + +const wholeChangedLines = ViewPlugin.fromClass( + class { + decorations: DecorationSet; + constructor(view: EditorView) { + this.decorations = wholeLineDeco(view); + } + update(update: ViewUpdate) { + if ( + update.docChanged || + update.viewportChanged || + getChunks(update.startState)?.chunks !== getChunks(update.state)?.chunks + ) + this.decorations = wholeLineDeco(update.view); + } + }, + { decorations: (value) => value.decorations }, +); + export const viewerHighlight = HighlightStyle.define([ - { tag: [t.keyword, t.modifier, t.controlKeyword, t.operatorKeyword, t.definitionKeyword], color: "var(--syn-keyword)" }, + { + tag: [t.keyword, t.modifier, t.controlKeyword, t.operatorKeyword, t.definitionKeyword], + color: "var(--syn-keyword)", + }, { tag: [t.string, t.special(t.string), t.regexp, t.character], color: "var(--syn-string)" }, { tag: [t.typeName, t.className, t.namespace, t.tagName], color: "var(--syn-type)" }, { tag: [t.function(t.variableName), t.function(t.propertyName)], color: "var(--syn-function)" }, @@ -55,6 +120,15 @@ export const viewerTheme = EditorView.theme( "&.cm-merge-b .cm-changedLine, .cm-inlineChangedLine": { backgroundColor: "color-mix(in srgb, var(--gutter-added) 12%, transparent)", }, + "&.cm-merge-a .cm-changedLine.cm-diffWholeLine": { + backgroundColor: "color-mix(in srgb, var(--danger) 26%, transparent)", + }, + "&.cm-merge-b .cm-changedLine.cm-diffWholeLine": { + backgroundColor: "color-mix(in srgb, var(--success) 26%, transparent)", + }, + "&.cm-merge-a .cm-diffWholeLine .cm-changedText, &.cm-merge-b .cm-diffWholeLine .cm-changedText": { + background: "transparent", + }, "&.cm-merge-a .cm-changedText, .cm-deletedChunk .cm-deletedText": { background: "color-mix(in srgb, var(--danger) 26%, transparent)", }, @@ -64,7 +138,6 @@ export const viewerTheme = EditorView.theme( "&.cm-merge-b .cm-changedText, .cm-insertedLine .cm-changedText": { background: "color-mix(in srgb, var(--success) 26%, transparent)", }, - ".cm-insertedLine": { backgroundColor: "color-mix(in srgb, var(--gutter-added) 12%, transparent)" }, ".cm-changeGutter": { width: "3px", paddingLeft: "0" }, "&.cm-merge-a .cm-changedLineGutter, .cm-deletedLineGutter": { background: "var(--gutter-deleted)" }, "&.cm-merge-b .cm-changedLineGutter": { background: "var(--gutter-modified)" }, @@ -104,6 +177,7 @@ export function viewerBase(opts: { readOnly: boolean; lineNumbers?: boolean }): keymap.of([...searchKeymap, ...defaultKeymap]), syntaxHighlighting(viewerHighlight), viewerTheme, + wholeChangedLines, EditorState.readOnly.of(opts.readOnly), EditorView.editable.of(!opts.readOnly), ]; diff --git a/src/features/vcs/diffOptions.ts b/src/features/vcs/diffOptions.ts index e2db3d1a5..35860fe44 100644 --- a/src/features/vcs/diffOptions.ts +++ b/src/features/vcs/diffOptions.ts @@ -94,7 +94,7 @@ function splitLines(s: string, policy: IgnorePolicy): Line[] { for (;;) { const nl = s.indexOf("\n", from); const end = nl < 0 ? s.length : nl; - const key = lineKey(s.slice(from, end), policy); + const key = lineKey(s.slice(from, end), policy) + (policy === "none" && nl >= 0 ? "\n" : ""); if (!(policy === "whitespaceAndLines" && key === "")) out.push({ from, to: nl < 0 ? s.length : nl + 1, key }); if (nl < 0) return out; from = nl + 1; @@ -119,13 +119,67 @@ function encoder() { }; } +/** Bounds the expensive diff search; larger inputs fall back to a cheaper, coarser match. */ +const DIFF_CONFIG = { scanLimit: 500 }; + /** - * Diff that ignores whitespace per `policy`: lines are matched by their - * normalized text, then changed blocks get a precise character diff (for word - * highlighting). Returns null when `policy` is "none" (use the default diff). + * Diff of encoded lines (one char per line). Lines that occur exactly once on + * each side anchor the match (patience diff), and only the gaps between the + * anchors get a regular diff, so distant edits in long files stay separate + * without a quadratic search. */ -export function whitespaceDiff(policy: IgnorePolicy): ((a: string, b: string) => readonly Change[]) | null { - if (policy === "none") return null; +function diffLines(a: string, b: string): Change[] { + const seen = new Map(); + for (let i = 0; i < a.length; i++) { + const e = seen.get(a[i]!); + if (e) e.inA++; + else seen.set(a[i]!, { inA: 1, inB: 0, at: i }); + } + for (const c of b) { + const e = seen.get(c); + if (e) e.inB++; + } + // Unique common lines as [indexA, indexB] in A order, then their longest increasing run in B. + const pairs: [number, number][] = []; + for (let j = 0; j < b.length; j++) { + const e = seen.get(b[j]!); + if (e && e.inA === 1 && e.inB === 1) pairs.push([e.at, j]); + } + pairs.sort((x, y) => x[0] - y[0]); + const tails: number[] = []; + const prev = new Array(pairs.length); + for (let i = 0; i < pairs.length; i++) { + let lo = 0; + let hi = tails.length; + while (lo < hi) { + const mid = (lo + hi) >> 1; + if (pairs[tails[mid]!]![1] < pairs[i]![1]) lo = mid + 1; + else hi = mid; + } + prev[i] = lo > 0 ? tails[lo - 1]! : -1; + tails[lo] = i; + } + const anchors: [number, number][] = []; + for (let i = tails.length ? tails[tails.length - 1]! : -1; i >= 0; i = prev[i]!) anchors.push(pairs[i]!); + anchors.reverse().push([a.length, b.length]); + const out: Change[] = []; + let fromA = 0; + let fromB = 0; + for (const [toA, toB] of anchors) { + if (toA > fromA || toB > fromB) + for (const c of diff(a.slice(fromA, toA), b.slice(fromB, toB), DIFF_CONFIG)) + out.push(new Change(c.fromA + fromA, c.toA + fromA, c.fromB + fromB, c.toB + fromB)); + fromA = toA + 1; + fromB = toB + 1; + } + return out; +} + +/** + * Match lines first, then compare the contents of changed blocks for word + * highlighting. The line keys also implement the whitespace policy. + */ +export function whitespaceDiff(policy: IgnorePolicy): (a: string, b: string) => readonly Change[] { return (a, b) => { const la = splitLines(a, policy); const lb = splitLines(b, policy); @@ -134,17 +188,17 @@ export function whitespaceDiff(policy: IgnorePolicy): ((a: string, b: string) => const eb: string[] = []; for (const l of la) { const c = enc(l.key); - if (c === null) return diff(a, b); + if (c === null) return diff(a, b, DIFF_CONFIG); ea.push(c); } for (const l of lb) { const c = enc(l.key); - if (c === null) return diff(a, b); + if (c === null) return diff(a, b, DIFF_CONFIG); eb.push(c); } const out: Change[] = []; const at = (lines: Line[], i: number) => (i > 0 ? lines[i - 1]!.to : 0); - for (const c of diff(ea.join(""), eb.join(""))) { + for (const c of diffLines(ea.join(""), eb.join(""))) { const fromA = c.toA > c.fromA ? la[c.fromA]!.from : at(la, c.fromA); const toA = c.toA > c.fromA ? la[c.toA - 1]!.to : fromA; const fromB = c.toB > c.fromB ? lb[c.fromB]!.from : at(lb, c.fromB); @@ -153,7 +207,7 @@ export function whitespaceDiff(policy: IgnorePolicy): ((a: string, b: string) => out.push(new Change(fromA, toA, fromB, toB)); continue; } - for (const inner of diff(a.slice(fromA, toA), b.slice(fromB, toB))) + for (const inner of diff(a.slice(fromA, toA), b.slice(fromB, toB), DIFF_CONFIG)) out.push(new Change(inner.fromA + fromA, inner.toA + fromA, inner.fromB + fromB, inner.toB + fromB)); } return out; @@ -166,7 +220,9 @@ export function whitespaceDiff(policy: IgnorePolicy): ((a: string, b: string) => */ export function transferChunk(chunk: Chunk, source: "a" | "b", src: Text, dst: Text) { const [srcFrom, srcTo, dstFrom, dstTo] = - source === "a" ? [chunk.fromA, chunk.toA, chunk.fromB, chunk.toB] : [chunk.fromB, chunk.toB, chunk.fromA, chunk.toA]; + source === "a" + ? [chunk.fromA, chunk.toA, chunk.fromB, chunk.toB] + : [chunk.fromB, chunk.toB, chunk.fromA, chunk.toA]; let insert = src.sliceString(srcFrom, Math.max(srcFrom, srcTo - 1)); if (srcFrom !== srcTo && dstTo <= dst.length) insert += "\n"; return { from: dstFrom, to: Math.min(dst.length, dstTo), insert };