diff --git a/apps/web/src/components/diffs/DiffFileTree.test.tsx b/apps/web/src/components/diffs/DiffFileTree.test.tsx index d7767ab94258..cd8f4d5b3222 100644 --- a/apps/web/src/components/diffs/DiffFileTree.test.tsx +++ b/apps/web/src/components/diffs/DiffFileTree.test.tsx @@ -5,7 +5,10 @@ import { act, type MouseEvent, type ReactNode } from "react"; import { create, type ReactTestRenderer } from "react-test-renderer"; import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test"; +import type { FileDiffMetadata } from "@pierre/diffs"; + import { DiffFileTree, type DiffFileTreeEntry } from "./DiffFileTree"; +import { diffFileTreeEntries } from "./diffFileTree.logic"; import { useCodeViewFileReveal } from "./useCodeViewFileReveal"; vi.mock("../../hooks/useTheme", () => ({ useTheme: () => ({ resolvedTheme: "dark" }) })); @@ -112,6 +115,22 @@ describe("diff tree file activation", () => { vi.unstubAllGlobals(); }); + it("mounts a file-to-symlink type change as one tree row", async () => { + // Git carries a type change as a deletion and an addition of the same path. + const typeChange = diffFileTreeEntries([ + { type: "deleted", name: "AGENTS.md", prevName: "AGENTS.md" } as FileDiffMetadata, + { type: "new", name: "AGENTS.md", prevName: "AGENTS.md" } as FileDiffMetadata, + { type: "change", name: "CLAUDE.md", prevName: "CLAUDE.md" } as FileDiffMetadata, + ]); + await mount({ files: [...typeChange] }); + + const tree = model(); + expect(tree.getItem("AGENTS.md")?.isDirectory()).toBe(false); + expect(tree.getItem("CLAUDE.md")?.isDirectory()).toBe(false); + await activate("AGENTS.md"); + expect(targets).toEqual([{ type: "item", id: "AGENTS.md\u0000AGENTS.md", align: "start" }]); + }); + it("reissues the reveal when the sole selected file is activated again", async () => { await mount(); await activate("02-short.ts"); diff --git a/apps/web/src/components/diffs/diffFileTree.logic.test.ts b/apps/web/src/components/diffs/diffFileTree.logic.test.ts index 4aa1fd7208b1..170e84308069 100644 --- a/apps/web/src/components/diffs/diffFileTree.logic.test.ts +++ b/apps/web/src/components/diffs/diffFileTree.logic.test.ts @@ -34,6 +34,23 @@ describe("diffFileTreeEntries", () => { }); }); +describe("diffFileTreeEntries", () => { + it("folds a file-to-symlink type change into one modified entry", () => { + expect( + diffFileTreeEntries([ + file("change", "CLAUDE.md"), + file("deleted", "AGENTS.md"), + file("new", "AGENTS.md"), + file("new", "docs/new.md"), + ]), + ).toEqual([ + { path: "CLAUDE.md", status: "modified" }, + { path: "AGENTS.md", status: "modified" }, + { path: "docs/new.md", status: "added" }, + ]); + }); +}); + describe("collectDirectoryPaths", () => { it("lists every ancestor once, parents first, with Pierre's trailing slash", () => { expect(collectDirectoryPaths(["apps/web/src/a.ts", "apps/web/b.ts", "README.md"])).toEqual([ diff --git a/apps/web/src/components/diffs/diffFileTree.logic.ts b/apps/web/src/components/diffs/diffFileTree.logic.ts index 7331fb150e05..b727c3ee3ffb 100644 --- a/apps/web/src/components/diffs/diffFileTree.logic.ts +++ b/apps/web/src/components/diffs/diffFileTree.logic.ts @@ -23,11 +23,22 @@ function toGitStatus(file: FileDiffMetadata): GitStatus { } } -/** Maps parsed diff files to tree entries, keeping the diff's own order. */ +/** + * Maps parsed diff files to tree entries, keeping the diff's own order. A path + * appears once: a type change (regular file to symlink) is a deletion plus an + * addition of the same path, and the tree shows the surviving file as modified. + */ export function diffFileTreeEntries( files: ReadonlyArray, ): ReadonlyArray { - return files.map((file) => ({ path: resolveFileDiffPath(file), status: toGitStatus(file) })); + const statusByPath = new Map(); + for (const file of files) { + const path = resolveFileDiffPath(file); + const status = toGitStatus(file); + const previous = statusByPath.get(path); + statusByPath.set(path, previous === undefined || previous === status ? status : "modified"); + } + return [...statusByPath].map(([path, status]) => ({ path, status })); } /** diff --git a/apps/web/src/lib/diffRendering.test.ts b/apps/web/src/lib/diffRendering.test.ts index 4f6746942af2..2f98aac50635 100644 --- a/apps/web/src/lib/diffRendering.test.ts +++ b/apps/web/src/lib/diffRendering.test.ts @@ -35,10 +35,10 @@ describe("buildPatchCacheKey", () => { describe("getRenderablePatch", () => { it.each([ - ["a/example.ts", "a/example.ts"], - ["b/example.ts", "b/example.ts"], - ["a/before.ts", "b/after.ts"], - ])("preserves repository paths from %s to %s", (previousPath, path) => { + ["a/example.ts", "a/example.ts", "change"], + ["b/example.ts", "b/example.ts", "change"], + ["a/before.ts", "b/after.ts", "rename-changed"], + ])("preserves repository paths from %s to %s", (previousPath, path, type) => { const parsed = getRenderablePatch( [ `diff --git a/${previousPath} b/${path}`, @@ -59,7 +59,7 @@ describe("getRenderablePatch", () => { if (!file) return; expect(resolveFileDiffPath(file)).toBe(path); expect(resolveFileDiffPreviousPath(file)).toBe(previousPath); - expect(buildFileDiffIdentityKey(file)).toBe(`${previousPath}\0${path}`); + expect(buildFileDiffIdentityKey(file)).toBe(`${previousPath}\0${path}\0${type}`); }); it("compacts partial hunk render offsets for virtualized review diffs", () => { @@ -137,6 +137,33 @@ describe("diff file reconciliation", () => { expect(buildFileDiffRenderKey(file)).toBe(key); }); + it("gives a type change its own identity per block", () => { + const patch = [ + "diff --git a/AGENTS.md b/AGENTS.md", + "deleted file mode 100644", + "--- a/AGENTS.md", + "+++ /dev/null", + "@@ -1 +0,0 @@", + "-duplicated instructions", + "diff --git a/AGENTS.md b/AGENTS.md", + "new file mode 120000", + "--- /dev/null", + "+++ b/AGENTS.md", + "@@ -0,0 +1 @@", + "+CLAUDE.md", + ].join("\n"); + const parsed = getRenderablePatch(patch, "type-change"); + expect(parsed?.kind).toBe("files"); + if (parsed?.kind !== "files") return; + const [deleted, added] = parsed.files; + expect(deleted?.type).toBe("deleted"); + expect(added?.type).toBe("new"); + if (!deleted || !added) return; + + expect(buildFileDiffIdentityKey(deleted)).not.toBe(buildFileDiffIdentityKey(added)); + expect(new Set(parsed.files.map(buildFileDiffIdentityKey)).size).toBe(parsed.files.length); + }); + it("keeps identities stable and versions local to the changed file", () => { const patch = (secondLine: string) => [ diff --git a/apps/web/src/lib/diffRendering.ts b/apps/web/src/lib/diffRendering.ts index 257b4e7f8d6d..3a6c6e20969a 100644 --- a/apps/web/src/lib/diffRendering.ts +++ b/apps/web/src/lib/diffRendering.ts @@ -166,8 +166,13 @@ export function resolveFileDiffPreviousPath(fileDiff: FileDiffMetadata): string return fileDiffPath(fileDiff.prevName ?? fileDiff.name ?? ""); } +/** + * Stable across re-renders of the same file, distinct for every block in a + * patch. A type change (regular file to symlink) arrives as a deletion and an + * addition of the same path, so the change type is part of the identity. + */ export function buildFileDiffIdentityKey(fileDiff: FileDiffMetadata): string { - return `${resolveFileDiffPreviousPath(fileDiff)}\u0000${resolveFileDiffPath(fileDiff)}`; + return `${resolveFileDiffPreviousPath(fileDiff)}\u0000${resolveFileDiffPath(fileDiff)}\u0000${fileDiff.type}`; } export function buildFileDiffRenderKey(fileDiff: FileDiffMetadata): string {