| null>(null);
useEffect(() => {
- filePathsRef.current = new Set(paths);
+ filePathsRef.current = new Set(modelPaths);
onSelectFileRef.current = onSelectFile;
- }, [onSelectFile, paths]);
+ toSelectionPathRef.current = presented.selectionPath;
+ }, [modelPaths, onSelectFile, presented.selectionPath]);
const { model } = useFileTree({
density: "compact",
@@ -90,8 +106,13 @@ export function DiffFileTree({
icons: T3_PIERRE_ICONS,
onSelectionChange: (selectedPaths) => {
if (syncingSelectionRef.current) return;
- const path = selectedPaths.at(-1)?.replace(/\/$/, "");
- if (path && filePathsRef.current.has(path)) onSelectFileRef.current(path);
+ const raw = selectedPaths.at(-1);
+ if (!raw) return;
+ // Directory ids end in `/`. A file that shares that directory's name is stored under a
+ // different model path, so stripping the slash must not select it.
+ const modelPath = filePathsRef.current.has(raw) ? raw : raw.replace(/\/$/, "");
+ if (!filePathsRef.current.has(modelPath)) return;
+ onSelectFileRef.current(toSelectionPathRef.current(modelPath));
},
paths: [],
search: false,
@@ -105,29 +126,30 @@ export function DiffFileTree({
useEffect(() => {
ordering.update(positions);
const mountedPaths = mountedPathsRef.current;
- if (mountedPaths === paths) return;
- mountedPathsRef.current = paths;
+ if (mountedPaths === modelPaths) return;
+ mountedPathsRef.current = modelPaths;
if (mountedPaths === null) {
- model.resetPaths(paths);
- } else if (mountedPaths.every((path, index) => paths[index] === path)) {
+ model.resetPaths(modelPaths);
+ } else if (mountedPaths.every((path, index) => modelPaths[index] === path)) {
// PR slices only append files, so keep the existing tree and its open folders.
- const updates = buildDiffFileTreeUpdates(mountedPaths, paths);
+ const updates = buildDiffFileTreeUpdates(mountedPaths, modelPaths);
if (updates.length > 0) model.batch(updates);
} else {
- // A refreshed diff can change the rank of existing siblings. Mutations do not reorder
- // those rows, so rebuild while carrying the reader's folder expansion forward.
+ // A refreshed diff can change the rank of existing siblings. A file that becomes a
+ // directory prefix also changes its model path, which Pierre cannot rename in place.
+ // Rebuild while carrying the reader's folder expansion forward.
const collapsedDirectories = directoryPaths.filter((path) => {
const directory = model.getItem(path);
return directory !== null && "isExpanded" in directory && !directory.isExpanded();
});
- model.resetPaths(paths);
+ model.resetPaths(modelPaths);
for (const path of collapsedDirectories) {
const directory = model.getItem(path);
if (directory !== null && "collapse" in directory) directory.collapse();
}
}
model.setGitStatus(gitStatus);
- }, [directoryPaths, gitStatus, model, ordering, paths, positions]);
+ }, [directoryPaths, gitStatus, model, modelPaths, ordering, positions]);
useEffect(() => {
if (selectedPath === null) {
@@ -136,32 +158,39 @@ export function DiffFileTree({
}
// A path list that changes under an already-revealed file (a refresh, a later slice) must
// not pull the tree back to it over whatever the reader has picked since.
- const item = model.getItem(selectedPath);
+ const modelPath = presented.modelPath(selectedPath);
+ const item = model.getItem(modelPath);
if (item === null || item.isDirectory()) {
// A file that left the diff has to be revealed again when it comes back.
handledRevealRef.current = null;
return;
}
const handled = handledRevealRef.current;
- if (handled?.path === selectedPath && handled.revealRequestId === revealRequestId) return;
- handledRevealRef.current = { path: selectedPath, revealRequestId };
+ if (
+ handled?.path === selectedPath &&
+ handled.revealRequestId === revealRequestId &&
+ handled.modelPath === modelPath
+ ) {
+ return;
+ }
+ handledRevealRef.current = { path: selectedPath, revealRequestId, modelPath };
syncingSelectionRef.current = true;
for (const path of model.getSelectedPaths()) {
- if (path !== selectedPath) model.getItem(path)?.deselect();
+ if (path !== modelPath) model.getItem(path)?.deselect();
}
let ancestor = "";
- for (const segment of selectedPath.split("/").slice(0, -1)) {
+ for (const segment of modelPath.split("/").slice(0, -1)) {
ancestor += `${segment}/`;
const directory = model.getItem(ancestor);
if (directory !== null && "expand" in directory) directory.expand();
}
item.select();
- model.scrollToPath(selectedPath, { offset: "nearest" });
+ model.scrollToPath(modelPath, { offset: "nearest" });
queueMicrotask(() => {
syncingSelectionRef.current = false;
});
- // `paths` is a dependency so a file that arrives after it was asked for is still revealed.
- }, [model, paths, revealRequestId, selectedPath]);
+ // `presented` is a dependency so a file that arrives after it was asked for is still revealed.
+ }, [model, presented, revealRequestId, selectedPath]);
return (
@@ -218,14 +247,16 @@ export function DiffFileTree({
// Pierre does not emit a selection change for its sole selected row.
// Read selection before the row handles the click so new selections reveal only once.
const selected = model.getSelectedPaths();
- const path = selected.length === 1 ? selected[0] : undefined;
- if (!path || !filePathsRef.current.has(path)) return;
+ const raw = selected.length === 1 ? selected[0] : undefined;
+ if (!raw) return;
+ const path = filePathsRef.current.has(raw) ? raw : raw.replace(/\/$/, "");
+ if (!filePathsRef.current.has(path)) return;
const clickedSelectedRow = event.nativeEvent
.composedPath()
.some(
(node) => node instanceof HTMLElement && node.getAttribute("data-item-path") === path,
);
- if (clickedSelectedRow) onSelectFileRef.current(path);
+ if (clickedSelectedRow) onSelectFileRef.current(toSelectionPathRef.current(path));
}}
className="min-h-0 flex-1 overflow-hidden"
style={pierreTreeStyle(resolvedTheme)}
diff --git a/apps/web/src/components/diffs/diffFileTree.logic.test.ts b/apps/web/src/components/diffs/diffFileTree.logic.test.ts
index 170e84308069..73c678bd8714 100644
--- a/apps/web/src/components/diffs/diffFileTree.logic.test.ts
+++ b/apps/web/src/components/diffs/diffFileTree.logic.test.ts
@@ -1,11 +1,12 @@
import type { FileDiffMetadata } from "@pierre/diffs";
-import { preloadFileTree } from "@pierre/trees";
+import { FileTree, preloadFileTree } from "@pierre/trees";
import { describe, expect, it } from "vite-plus/test";
import {
buildDiffFileTreeUpdates,
compareDiffFileTreeEntries,
collectDirectoryPaths,
+ diffFileTreeModel,
diffFileTreePositions,
diffFileTreeEntries,
} from "./diffFileTree.logic";
@@ -115,3 +116,109 @@ describe("buildDiffFileTreeUpdates", () => {
expect(buildDiffFileTreeUpdates(["src/a.ts"], ["src/a.ts"])).toEqual([]);
});
});
+
+describe("diffFileTreeModel", () => {
+ it("returns ordinary paths unchanged", () => {
+ const paths = ["src/a.ts", "README.md"];
+ const presented = diffFileTreeModel(paths);
+ expect(presented.paths).toBe(paths);
+ expect(presented.selectionPath("src/a.ts")).toBe("src/a.ts");
+ expect(presented.modelPath("src/a.ts")).toBe("src/a.ts");
+ });
+
+ it("does not treat a shared string prefix as a directory collision", () => {
+ const paths = ["office", "officer.ts"];
+ expect(diffFileTreeModel(paths).paths).toBe(paths);
+ });
+
+ it("does not rewrite a directory that only contains files", () => {
+ const paths = ["office/config.ts", "office/index.ts"];
+ expect(diffFileTreeModel(paths).paths).toBe(paths);
+ });
+
+ it("keeps a file that is also a directory prefix selectable in Pierre", () => {
+ const paths = ["office", "office/config.ts"];
+ const presented = diffFileTreeModel(paths);
+ expect(presented.paths).not.toBe(paths);
+ expect(presented.selectionPath(presented.modelPath("office"))).toBe("office");
+ expect(presented.selectionPath(presented.modelPath("office/config.ts"))).toBe(
+ "office/config.ts",
+ );
+ expect(presented.modelPath("office/config.ts")).toBe("office/config.ts");
+ expect(presented.modelPath("office")).not.toBe("office");
+
+ const tree = new FileTree({
+ paths: presented.paths,
+ initialExpansion: "open",
+ flattenEmptyDirectories: true,
+ });
+ expect(tree.getItem(presented.modelPath("office"))?.isDirectory()).toBe(false);
+ expect(tree.getItem("office/")?.isDirectory()).toBe(true);
+ expect(tree.getItem("office/config.ts")?.isDirectory()).toBe(false);
+ tree.cleanUp();
+ });
+
+ it("keeps the reverse directory-to-file transition selectable", () => {
+ const paths = ["office/config.ts", "office"];
+ const presented = diffFileTreeModel(paths);
+ expect(presented.paths[0]).toBe("office/config.ts");
+ expect(presented.selectionPath(presented.modelPath("office"))).toBe("office");
+ expect(() => {
+ const tree = new FileTree({ paths: presented.paths, initialExpansion: "open" });
+ tree.cleanUp();
+ }).not.toThrow();
+ });
+
+ it("rewrites a colliding file deeper than the first segment, and a chain of prefixes", () => {
+ const nested = diffFileTreeModel(["src/office", "src/office/config.ts"]);
+ expect(nested.modelPath("src/office")).not.toBe("src/office");
+ expect(nested.modelPath("src/office/config.ts")).toBe("src/office/config.ts");
+ expect(nested.selectionPath(nested.modelPath("src/office"))).toBe("src/office");
+
+ const chain = diffFileTreeModel(["a", "a/b", "a/b/c"]);
+ expect(chain.modelPath("a")).not.toBe("a");
+ expect(chain.modelPath("a/b")).not.toBe("a/b");
+ expect(chain.modelPath("a/b/c")).toBe("a/b/c");
+ const tree = new FileTree({ paths: chain.paths, initialExpansion: "open" });
+ expect(tree.getItem(chain.modelPath("a"))?.isDirectory()).toBe(false);
+ expect(tree.getItem(chain.modelPath("a/b"))?.isDirectory()).toBe(false);
+ expect(tree.getItem("a/b/c")?.isDirectory()).toBe(false);
+ tree.cleanUp();
+ });
+
+ it("does not reuse a diff path that already ends with the file mark", () => {
+ const marked = "office\u200b";
+ const paths = ["office", marked, "office/config.ts"];
+ const presented = diffFileTreeModel(paths);
+ expect(new Set(presented.paths).size).toBe(paths.length);
+ expect(presented.modelPath("office")).not.toBe(marked);
+ expect(presented.selectionPath(presented.modelPath("office"))).toBe("office");
+ expect(presented.selectionPath(presented.modelPath(marked))).toBe(marked);
+ const tree = new FileTree({ paths: presented.paths, initialExpansion: "open" });
+ expect(tree.getItem(presented.modelPath("office"))?.isDirectory()).toBe(false);
+ expect(tree.getItem(presented.modelPath(marked))?.isDirectory()).toBe(false);
+ expect(tree.getItem("office/config.ts")?.isDirectory()).toBe(false);
+ tree.cleanUp();
+ });
+
+ it("rejects the raw colliding list and accepts a later slice once paths are safe", () => {
+ const tree = new FileTree({ paths: ["office"], initialExpansion: "open" });
+ expect(() =>
+ tree.batch(buildDiffFileTreeUpdates(["office"], ["office", "office/config.ts"])),
+ ).toThrow(/collides with an existing file/);
+
+ const appended = diffFileTreeModel(["src/a.ts", "office", "office/config.ts"]);
+ const updates = buildDiffFileTreeUpdates(["src/a.ts"], appended.paths);
+ const appending = new FileTree({ paths: ["src/a.ts"], initialExpansion: "open" });
+ expect(() => appending.batch(updates)).not.toThrow();
+ expect(appending.getItem(appended.modelPath("office"))?.isDirectory()).toBe(false);
+ expect(appending.getItem("office/config.ts")?.isDirectory()).toBe(false);
+
+ const rewritten = diffFileTreeModel(["office", "office/config.ts"]);
+ expect(() => tree.resetPaths(rewritten.paths)).not.toThrow();
+ expect(tree.getItem(rewritten.modelPath("office"))?.isDirectory()).toBe(false);
+ expect(tree.getItem("office/config.ts")?.isDirectory()).toBe(false);
+ tree.cleanUp();
+ appending.cleanUp();
+ });
+});
diff --git a/apps/web/src/components/diffs/diffFileTree.logic.ts b/apps/web/src/components/diffs/diffFileTree.logic.ts
index b727c3ee3ffb..93d8d86b2a9a 100644
--- a/apps/web/src/components/diffs/diffFileTree.logic.ts
+++ b/apps/web/src/components/diffs/diffFileTree.logic.ts
@@ -41,6 +41,86 @@ export function diffFileTreeEntries(
return [...statusByPath].map(([path, status]) => ({ path, status }));
}
+/**
+ * Pierre stores one filesystem. A diff path that is also a directory prefix of another
+ * path (`office` and `office/config.ts`, or the reverse) cannot be inserted as-is.
+ * The file keeps a zero-width suffix so the row label stays the file name and the
+ * directory can still be created. Callers translate with `selectionPath` and `modelPath`.
+ */
+const DIFF_FILE_TREE_FILE_MARK = "\u200b";
+
+const identityPath = (path: string) => path;
+
+export interface DiffFileTreeModel {
+ /** Paths safe to pass to Pierre. Same array as the input when nothing collides. */
+ readonly paths: ReadonlyArray;
+ /** Pierre path to the diff path a click should open. */
+ readonly selectionPath: (modelPath: string) => string;
+ /** Diff path to the path stored in the tree. */
+ readonly modelPath: (path: string) => string;
+}
+
+function collidingFilePaths(paths: ReadonlyArray): ReadonlySet | null {
+ if (paths.length < 2) return null;
+ const unique = [...new Set(paths)].toSorted();
+ const colliding = new Set();
+ for (let index = 0; index < unique.length; index += 1) {
+ const path = unique[index]!;
+ const directoryPrefix = `${path}/`;
+ for (let next = index + 1; next < unique.length; next += 1) {
+ const other = unique[next]!;
+ if (!other.startsWith(path)) break;
+ if (other.startsWith(directoryPrefix)) {
+ colliding.add(path);
+ break;
+ }
+ }
+ }
+ return colliding.size === 0 ? null : colliding;
+}
+
+function modelPathForCollidingFile(path: string, occupied: Set): string {
+ let modelPath = path;
+ const collides = (candidate: string) => {
+ if (occupied.has(candidate)) return true;
+ const prefix = `${candidate}/`;
+ for (const other of occupied) {
+ if (other.startsWith(prefix)) return true;
+ }
+ return false;
+ };
+ do {
+ modelPath += DIFF_FILE_TREE_FILE_MARK;
+ } while (collides(modelPath));
+ return modelPath;
+}
+
+export function diffFileTreeModel(paths: ReadonlyArray): DiffFileTreeModel {
+ const colliding = collidingFilePaths(paths);
+ if (colliding === null) {
+ return { paths, selectionPath: identityPath, modelPath: identityPath };
+ }
+ const occupied = new Set(paths);
+ const modelPaths = paths.map((path) => {
+ if (!colliding.has(path)) return path;
+ const modelPath = modelPathForCollidingFile(path, occupied);
+ occupied.add(modelPath);
+ return modelPath;
+ });
+ const selectionByModelPath = new Map();
+ const modelBySelectionPath = new Map();
+ paths.forEach((path, index) => {
+ const modelPath = modelPaths[index]!;
+ selectionByModelPath.set(modelPath, path);
+ modelBySelectionPath.set(path, modelPath);
+ });
+ return {
+ paths: modelPaths,
+ selectionPath: (modelPath) => selectionByModelPath.get(modelPath) ?? modelPath,
+ modelPath: (path) => modelBySelectionPath.get(path) ?? path,
+ };
+}
+
/**
* Every directory on the way to each file, registered with the trailing slash Pierre uses for
* directory ids. Parents come before children so the tree can add them in order.