diff --git a/apps/web/src/components/DiffPanel.tsx b/apps/web/src/components/DiffPanel.tsx index 44439d7a8fb0..402712cc1b37 100644 --- a/apps/web/src/components/DiffPanel.tsx +++ b/apps/web/src/components/DiffPanel.tsx @@ -63,7 +63,11 @@ import { DiffPanelLoadingState, DiffPanelShell, type DiffPanelMode } from "./Dif import { DiffStatLabel } from "./chat/DiffStatLabel"; import { AnnotatableCodeView, type AnnotatableCodeViewHandle } from "./diffs/AnnotatableCodeView"; import { DiffFileTree } from "./diffs/DiffFileTree"; -import { diffFileTreeEntries } from "./diffs/diffFileTree.logic"; +import { + diffFileTreeEntries, + groupedDiffFileTreeEntries, + groupedDiffFileTreePath, +} from "./diffs/diffFileTree.logic"; import { Button } from "./ui/button"; import { ToggleGroup, Toggle } from "./ui/toggle-group"; import { Switch } from "./ui/switch"; @@ -643,7 +647,6 @@ export default function DiffPanel({ const diffFileKeys = useMemo(() => codeViewFiles.map((file) => file.fileKey), [codeViewFiles]); const allDiffFilesCollapsed = areAllDiffFilesCollapsed(diffFileKeys, collapsedDiffFileKeys); const diffLineStat = useMemo(() => getDiffLineStat(renderableFiles), [renderableFiles]); - const fileTreeEntries = useMemo(() => diffFileTreeEntries(renderableFiles), [renderableFiles]); const selectedDiffFileKey = selectedFilePath ? (codeViewFiles.find((candidate) => candidate.filePath === selectedFilePath)?.fileKey ?? null) : null; @@ -703,9 +706,34 @@ export default function DiffPanel({ const visibleDiffTargets = effectiveRepoFilter ? diffRepoTargets.filter((entry) => repoRootBaseName(entry.repoRoot) === effectiveRepoFilter) : diffRepoTargets; - const visibleGroups = effectiveRepoFilter - ? renderableGroups.filter((group) => repoRootBaseName(group.repoRoot) === effectiveRepoFilter) - : renderableGroups; + const visibleGroups = useMemo( + () => + effectiveRepoFilter + ? renderableGroups.filter( + (group) => repoRootBaseName(group.repoRoot) === effectiveRepoFilter, + ) + : renderableGroups, + [effectiveRepoFilter, renderableGroups], + ); + + // The tree mirrors what the diff draws. A grouped view gets one folder per + // repo section (named like the section header), so two roots that changed the + // same relative path are two rows instead of a duplicate-path crash. + const fileTreeGroups = useMemo( + () => visibleGroups.map((group) => ({ label: group.displayName, files: group.files })), + [visibleGroups], + ); + const fileTreeEntries = useMemo( + () => + isGroupedDiffView + ? groupedDiffFileTreeEntries(fileTreeGroups) + : diffFileTreeEntries(renderableFiles), + [fileTreeGroups, isGroupedDiffView, renderableFiles], + ); + const selectedFileTreePath = + selectedFilePath && isGroupedDiffView + ? groupedDiffFileTreePath(fileTreeGroups, selectedFilePath) + : selectedFilePath; useEffect(() => { if (!selectedDiffFileKey || !codeView?.getInstance()) return; @@ -1463,7 +1491,7 @@ export default function DiffPanel({ diff --git a/apps/web/src/components/diffs/diffFileTree.logic.test.ts b/apps/web/src/components/diffs/diffFileTree.logic.test.ts index d8e24968dcea..782c327e1016 100644 --- a/apps/web/src/components/diffs/diffFileTree.logic.test.ts +++ b/apps/web/src/components/diffs/diffFileTree.logic.test.ts @@ -5,6 +5,8 @@ import { buildDiffFileTreeUpdates, collectDirectoryPaths, diffFileTreeEntries, + groupedDiffFileTreeEntries, + groupedDiffFileTreePath, } from "./diffFileTree.logic"; function file(type: FileDiffMetadata["type"], name: string, prevName = name): FileDiffMetadata { @@ -29,6 +31,45 @@ describe("diffFileTreeEntries", () => { { path: "README.md", status: "modified" }, ]); }); + + it("keeps the first of two files at the same path so the tree never throws on a duplicate", () => { + expect(diffFileTreeEntries([file("change", "README.md"), file("new", "README.md")])).toEqual([ + { path: "README.md", status: "modified" }, + ]); + }); +}); + +describe("groupedDiffFileTreeEntries", () => { + const groups = [ + { label: "api", files: [file("change", "README.md"), file("new", "src/a.ts")] }, + { label: "web", files: [file("change", "README.md")] }, + ]; + + it("files each repo's changes under a folder named for the repo", () => { + expect(groupedDiffFileTreeEntries(groups)).toEqual([ + { path: "api/README.md", status: "modified" }, + { path: "api/src/a.ts", status: "added" }, + { path: "web/README.md", status: "modified" }, + ]); + }); + + it("drops a repeat when two roots share a folder name", () => { + expect( + groupedDiffFileTreeEntries([ + { label: "app", files: [file("change", "README.md")] }, + { label: "app", files: [file("change", "README.md"), file("new", "b.ts")] }, + ]), + ).toEqual([ + { path: "app/README.md", status: "modified" }, + { path: "app/b.ts", status: "added" }, + ]); + }); + + it("resolves a repo-relative path to the first group that changed it", () => { + expect(groupedDiffFileTreePath(groups, "README.md")).toBe("api/README.md"); + expect(groupedDiffFileTreePath(groups, "src/a.ts")).toBe("api/src/a.ts"); + expect(groupedDiffFileTreePath(groups, "missing.ts")).toBeNull(); + }); }); describe("collectDirectoryPaths", () => { diff --git a/apps/web/src/components/diffs/diffFileTree.logic.ts b/apps/web/src/components/diffs/diffFileTree.logic.ts index 4535ece8b143..cd13ca586fe6 100644 --- a/apps/web/src/components/diffs/diffFileTree.logic.ts +++ b/apps/web/src/components/diffs/diffFileTree.logic.ts @@ -23,11 +23,65 @@ 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 that repeats + * keeps its first entry: Pierre's path store throws on a duplicate, and a tree missing a row + * beats a diff panel that cannot render at all. + */ export function diffFileTreeEntries( files: ReadonlyArray, ): ReadonlyArray { - return files.map((file) => ({ path: resolveFileDiffPath(file), status: toGitStatus(file) })); + const entries: DiffFileTreeEntry[] = []; + appendDiffFileTreeEntries(entries, new Set(), files, ""); + return entries; +} + +function appendDiffFileTreeEntries( + entries: DiffFileTreeEntry[], + seen: Set, + files: ReadonlyArray, + pathPrefix: string, +): void { + for (const file of files) { + const path = `${pathPrefix}${resolveFileDiffPath(file)}`; + if (seen.has(path)) continue; + seen.add(path); + entries.push({ path, status: toGitStatus(file) }); + } +} + +/** A group of changed files under one repo root of a multi-repo diff. */ +export interface DiffFileTreeGroup { + /** Folder name the group's files sit under in the tree, matching the diff's section header. */ + readonly label: string; + readonly files: ReadonlyArray; +} + +/** + * Tree entries for a diff that spans several repo roots. Each root's files sit under a folder + * named for that root, so two roots that both changed `README.md` stay two rows, the same way + * the diff draws one section per root. + */ +export function groupedDiffFileTreeEntries( + groups: ReadonlyArray, +): ReadonlyArray { + const entries: DiffFileTreeEntry[] = []; + const seen = new Set(); + for (const group of groups) { + appendDiffFileTreeEntries(entries, seen, group.files, `${group.label}/`); + } + return entries; +} + +/** The tree path a repo-relative file takes inside a grouped tree, or null when no group has it. */ +export function groupedDiffFileTreePath( + groups: ReadonlyArray, + filePath: string, +): string | null { + const group = groups.find((candidate) => + candidate.files.some((file) => resolveFileDiffPath(file) === filePath), + ); + return group ? `${group.label}/${filePath}` : null; } /**