mirror of
https://github.com/getpaseo/paseo.git
synced 2026-10-30 21:23:04 +00:00
Preserve renames in committed file diffs (#6895)
* fix(git): preserve renames in committed file diffs Follow the requested destination through rename detection so the patch agrees with commit history instead of showing the whole file as newly added. * test(git): keep rename fixtures valid across platforms * fix(git): keep added copies out of committed rename diffs Detect rename sources across the first-parent commit before narrowing the patch. Follow mode also detected copies, causing added files to open as partial diffs against unchanged files. * perf(git): share committed rename discovery across file requests Opening a commit requests all of its file diffs concurrently. Share pending and completed rename scans for immutable commit IDs in a bounded cache, while symbolic refs always read fresh metadata. --------- Co-authored-by: paseo-bot[bot] <266920839+paseo-bot[bot]@users.noreply.github.com>
This commit is contained in:
@@ -5,6 +5,44 @@ import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { createTestPaseoDaemon, DaemonClient } from "./test-utils/index.js";
|
||||
|
||||
test("a rename-only commit returns zero added or deleted lines through the client", async () => {
|
||||
const cwd = mkdtempSync(join(tmpdir(), "commit-rename-diff-"));
|
||||
const git = (...args: string[]) => execFileSync("git", args, { cwd, encoding: "utf8" }).trim();
|
||||
const daemon = await createTestPaseoDaemon({ agentClients: {} });
|
||||
const client = new DaemonClient({ url: `ws://127.0.0.1:${daemon.port}/ws` });
|
||||
try {
|
||||
git("init", "-b", "main");
|
||||
git("config", "user.name", "Test");
|
||||
git("config", "user.email", "test@example.com");
|
||||
writeFileSync(join(cwd, "old.txt"), "same\n");
|
||||
git("add", ".");
|
||||
git("-c", "commit.gpgsign=false", "commit", "-m", "initial");
|
||||
git("mv", "old.txt", "new.txt");
|
||||
git("-c", "commit.gpgsign=false", "commit", "-m", "rename");
|
||||
const sha = git("rev-parse", "HEAD");
|
||||
|
||||
await client.connect();
|
||||
const history = await client.listCheckoutCommits(cwd);
|
||||
expect(history.commits.find((commit) => commit.sha === sha)?.files).toEqual([
|
||||
{ path: "new.txt", additions: 0, deletions: 0, status: "renamed" },
|
||||
]);
|
||||
expect(await client.getCommitFileDiff(cwd, sha, "new.txt")).toEqual({
|
||||
file: {
|
||||
path: "new.txt",
|
||||
isNew: false,
|
||||
isDeleted: false,
|
||||
additions: 0,
|
||||
deletions: 0,
|
||||
hunks: [],
|
||||
},
|
||||
});
|
||||
} finally {
|
||||
await client.close();
|
||||
await daemon.close();
|
||||
rmSync(cwd, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 });
|
||||
}
|
||||
});
|
||||
|
||||
test.skipIf(process.platform === "win32")(
|
||||
"checkout and commit diffs preserve filenames that Git quotes",
|
||||
async () => {
|
||||
|
||||
@@ -3,7 +3,8 @@ import { mkdirSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from "fs"
|
||||
import { tmpdir } from "os";
|
||||
import { join } from "path";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { getCommitFileDiff } from "./checkout-git.js";
|
||||
import { getCommitFileDiff, listCheckoutCommits } from "./checkout-git.js";
|
||||
import { startGitCommandMetrics, stopGitCommandMetrics } from "./run-git-command.js";
|
||||
|
||||
const tempDirs: string[] = [];
|
||||
|
||||
@@ -44,6 +45,75 @@ function headSha(repoDir: string): string {
|
||||
}
|
||||
|
||||
describe("getCommitFileDiff", () => {
|
||||
it.each(["new.txt", "with spaces.txt"])("preserves a rename-only commit to %s", async (name) => {
|
||||
const repoDir = initRepo();
|
||||
commitFile(repoDir, "old.txt", "same\n", "initial");
|
||||
git(["mv", "old.txt", name], repoDir);
|
||||
git(["-c", "commit.gpgsign=false", "commit", "-m", "rename"], repoDir);
|
||||
const sha = headSha(repoDir);
|
||||
|
||||
const history = await listCheckoutCommits({ cwd: repoDir });
|
||||
expect(history.commits.find((commit) => commit.sha === sha)?.files).toEqual([
|
||||
{ path: name, status: "renamed", additions: 0, deletions: 0 },
|
||||
]);
|
||||
expect(await getCommitFileDiff({ cwd: repoDir, sha, path: name })).toEqual({
|
||||
path: name,
|
||||
isNew: false,
|
||||
isDeleted: false,
|
||||
additions: 0,
|
||||
deletions: 0,
|
||||
hunks: [],
|
||||
});
|
||||
});
|
||||
|
||||
it.each(["new.txt", "目录.txt"])(
|
||||
"preserves the edits in a committed rename to %s",
|
||||
async (name) => {
|
||||
const repoDir = initRepo();
|
||||
commitFile(repoDir, "old.txt", "same\nkeep\nlast\n", "initial");
|
||||
git(["mv", "old.txt", name], repoDir);
|
||||
writeFileSync(join(repoDir, name), "same\nKEEP\nlast\n");
|
||||
git(["add", "."], repoDir);
|
||||
git(["-c", "commit.gpgsign=false", "commit", "-m", "rename and edit"], repoDir);
|
||||
const sha = headSha(repoDir);
|
||||
|
||||
const history = await listCheckoutCommits({ cwd: repoDir });
|
||||
expect(history.commits.find((commit) => commit.sha === sha)?.files).toEqual([
|
||||
{ path: name, status: "renamed", additions: 1, deletions: 1 },
|
||||
]);
|
||||
const file = await getCommitFileDiff({ cwd: repoDir, sha, path: name });
|
||||
expect(file).toMatchObject({
|
||||
path: name,
|
||||
isNew: false,
|
||||
isDeleted: false,
|
||||
additions: 1,
|
||||
deletions: 1,
|
||||
});
|
||||
expect(file?.hunks.flatMap((hunk) => hunk.lines)).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({ type: "remove", content: "keep" }),
|
||||
expect.objectContaining({ type: "add", content: "KEEP" }),
|
||||
]),
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
it.skipIf(process.platform === "win32")("preserves a rename to a quoted filename", async () => {
|
||||
const repoDir = initRepo();
|
||||
const name = 'new "draft".txt';
|
||||
commitFile(repoDir, "old.txt", "same\n", "initial");
|
||||
git(["mv", "old.txt", name], repoDir);
|
||||
git(["-c", "commit.gpgsign=false", "commit", "-m", "rename"], repoDir);
|
||||
expect(await getCommitFileDiff({ cwd: repoDir, sha: headSha(repoDir), path: name })).toEqual({
|
||||
path: name,
|
||||
isNew: false,
|
||||
isDeleted: false,
|
||||
additions: 0,
|
||||
deletions: 0,
|
||||
hunks: [],
|
||||
});
|
||||
});
|
||||
|
||||
it("returns the parsed diff for a modified file in a commit", async () => {
|
||||
const repoDir = initRepo();
|
||||
commitFile(repoDir, "foo.txt", "a\nb\nc\n", "initial");
|
||||
@@ -78,6 +148,78 @@ describe("getCommitFileDiff", () => {
|
||||
expect(file?.deletions).toBe(0);
|
||||
});
|
||||
|
||||
it.each([
|
||||
["exact", "line1\nline2\nline3\nline4\nline5\nline6\n"],
|
||||
["edited", "line1\nline2\nline3\nline4\nline5\nCHANGED\n"],
|
||||
])("shows an added %s copy as a new file", async (_kind, addedContent) => {
|
||||
const repoDir = initRepo();
|
||||
commitFile(repoDir, "source.txt", "line1\nline2\nline3\nline4\nline5\nline6\n", "initial");
|
||||
commitFile(repoDir, "added.txt", addedContent, "add similar file");
|
||||
const sha = headSha(repoDir);
|
||||
|
||||
const history = await listCheckoutCommits({ cwd: repoDir });
|
||||
expect(history.commits.find((commit) => commit.sha === sha)?.files).toEqual([
|
||||
{ path: "added.txt", status: "added", additions: 6, deletions: 0 },
|
||||
]);
|
||||
const file = await getCommitFileDiff({ cwd: repoDir, sha, path: "added.txt" });
|
||||
expect(file).toMatchObject({
|
||||
path: "added.txt",
|
||||
isNew: true,
|
||||
isDeleted: false,
|
||||
additions: 6,
|
||||
deletions: 0,
|
||||
});
|
||||
expect(
|
||||
file?.hunks
|
||||
.flatMap((hunk) => hunk.lines)
|
||||
.filter((line) => line.type !== "header")
|
||||
.map(({ type, content }) => ({ type, content })),
|
||||
).toEqual(
|
||||
addedContent
|
||||
.trimEnd()
|
||||
.split("\n")
|
||||
.map((content) => ({ type: "add", content })),
|
||||
);
|
||||
});
|
||||
|
||||
it("shares commit rename discovery across concurrent and later file requests", async () => {
|
||||
const repoDir = initRepo();
|
||||
commitFile(repoDir, "old.txt", "same\n", "initial");
|
||||
git(["mv", "old.txt", "new.txt"], repoDir);
|
||||
commitFile(repoDir, "added.txt", "added\n", "rename and add");
|
||||
const sha = headSha(repoDir);
|
||||
|
||||
startGitCommandMetrics();
|
||||
try {
|
||||
const [renamed, added] = await Promise.all([
|
||||
getCommitFileDiff({ cwd: repoDir, sha, path: "new.txt" }),
|
||||
getCommitFileDiff({ cwd: repoDir, sha, path: "added.txt" }),
|
||||
]);
|
||||
expect(renamed).toMatchObject({ isNew: false, additions: 0, deletions: 0 });
|
||||
expect(added).toMatchObject({ isNew: true, additions: 1, deletions: 0 });
|
||||
expect(await getCommitFileDiff({ cwd: repoDir, sha, path: "new.txt" })).toEqual(renamed);
|
||||
} finally {
|
||||
const metrics = stopGitCommandMetrics();
|
||||
expect(metrics.commands.filter(({ args }) => args.includes("--name-status"))).toHaveLength(1);
|
||||
}
|
||||
});
|
||||
|
||||
it("rediscovers renames when a symbolic commit ref moves", async () => {
|
||||
const repoDir = initRepo();
|
||||
commitFile(repoDir, "old.txt", "same\n", "initial");
|
||||
expect(await getCommitFileDiff({ cwd: repoDir, sha: "HEAD", path: "old.txt" })).toMatchObject({
|
||||
isNew: true,
|
||||
additions: 1,
|
||||
});
|
||||
git(["mv", "old.txt", "new.txt"], repoDir);
|
||||
git(["-c", "commit.gpgsign=false", "commit", "-m", "rename"], repoDir);
|
||||
expect(await getCommitFileDiff({ cwd: repoDir, sha: "HEAD", path: "new.txt" })).toMatchObject({
|
||||
isNew: false,
|
||||
additions: 0,
|
||||
deletions: 0,
|
||||
});
|
||||
});
|
||||
|
||||
it("returns null for a path not changed in the commit", async () => {
|
||||
const repoDir = initRepo();
|
||||
commitFile(repoDir, "foo.txt", "a\n", "initial");
|
||||
@@ -89,6 +231,59 @@ describe("getCommitFileDiff", () => {
|
||||
expect(file).toBeNull();
|
||||
});
|
||||
|
||||
it("does not follow an unchanged file back to an earlier commit", async () => {
|
||||
const repoDir = initRepo();
|
||||
commitFile(repoDir, "foo.txt", "same\n", "initial");
|
||||
commitFile(repoDir, "other.txt", "other\n", "unrelated change");
|
||||
expect(
|
||||
await getCommitFileDiff({ cwd: repoDir, sha: headSha(repoDir), path: "foo.txt" }),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it("returns the added file in a root commit and the deleted file in its deletion commit", async () => {
|
||||
const repoDir = initRepo();
|
||||
commitFile(repoDir, "foo.txt", "same\n", "initial");
|
||||
expect(
|
||||
await getCommitFileDiff({ cwd: repoDir, sha: headSha(repoDir), path: "foo.txt" }),
|
||||
).toMatchObject({
|
||||
isNew: true,
|
||||
isDeleted: false,
|
||||
additions: 1,
|
||||
deletions: 0,
|
||||
});
|
||||
git(["rm", "foo.txt"], repoDir);
|
||||
git(["-c", "commit.gpgsign=false", "commit", "-m", "delete"], repoDir);
|
||||
expect(
|
||||
await getCommitFileDiff({ cwd: repoDir, sha: headSha(repoDir), path: "foo.txt" }),
|
||||
).toMatchObject({
|
||||
isNew: false,
|
||||
isDeleted: true,
|
||||
additions: 0,
|
||||
deletions: 1,
|
||||
});
|
||||
});
|
||||
|
||||
it("preserves a rename introduced by a merge against its first parent", async () => {
|
||||
const repoDir = initRepo();
|
||||
commitFile(repoDir, "old.txt", "same\n", "initial");
|
||||
git(["checkout", "-b", "feature"], repoDir);
|
||||
git(["mv", "old.txt", "new.txt"], repoDir);
|
||||
git(["-c", "commit.gpgsign=false", "commit", "-m", "rename"], repoDir);
|
||||
git(["checkout", "main"], repoDir);
|
||||
commitFile(repoDir, "main.txt", "main\n", "advance main");
|
||||
git(["merge", "--no-ff", "feature", "-m", "merge feature"], repoDir);
|
||||
expect(
|
||||
await getCommitFileDiff({ cwd: repoDir, sha: headSha(repoDir), path: "new.txt" }),
|
||||
).toMatchObject({
|
||||
path: "new.txt",
|
||||
isNew: false,
|
||||
isDeleted: false,
|
||||
additions: 0,
|
||||
deletions: 0,
|
||||
hunks: [],
|
||||
});
|
||||
});
|
||||
|
||||
it("returns a merge commit diff against its first parent", async () => {
|
||||
const repoDir = initRepo();
|
||||
commitFile(repoDir, "README.md", "base\n", "initial");
|
||||
|
||||
@@ -2643,6 +2643,60 @@ export async function listCheckoutCommits({
|
||||
return { baseRef: comparisonBaseRef, commits };
|
||||
}
|
||||
|
||||
const commitRenamesCache = new TTLCache<string, Promise<ReadonlyMap<string, string>>>({
|
||||
max: 100,
|
||||
ttl: 60_000,
|
||||
checkAgeOnGet: true,
|
||||
});
|
||||
|
||||
function getCommitRenames(cwd: string, sha: string): Promise<ReadonlyMap<string, string>> {
|
||||
// Symbolic refs and abbreviated IDs can resolve to a different commit later.
|
||||
if (!/^(?:[0-9a-f]{40}|[0-9a-f]{64})$/i.test(sha)) {
|
||||
return readCommitRenames(cwd, sha);
|
||||
}
|
||||
const key = JSON.stringify([cwd, sha]);
|
||||
const cached = commitRenamesCache.get(key);
|
||||
if (cached) {
|
||||
return cached;
|
||||
}
|
||||
const lookup = readCommitRenames(cwd, sha).catch((error: unknown) => {
|
||||
if (commitRenamesCache.get(key) === lookup) {
|
||||
commitRenamesCache.delete(key);
|
||||
}
|
||||
throw error;
|
||||
});
|
||||
// Store the pending lookup too, so concurrent file requests share the scan.
|
||||
commitRenamesCache.set(key, lookup);
|
||||
return lookup;
|
||||
}
|
||||
|
||||
// Discover renames across the commit before narrowing the patch to a file.
|
||||
// --follow also detects copies, which would disagree with history's -M-only list.
|
||||
async function readCommitRenames(cwd: string, sha: string): Promise<ReadonlyMap<string, string>> {
|
||||
const result = await runGitCommand(
|
||||
[
|
||||
"show",
|
||||
sha,
|
||||
"--format=",
|
||||
"--diff-merges=first-parent",
|
||||
"--name-status",
|
||||
"-z",
|
||||
"-M",
|
||||
"--diff-filter=R",
|
||||
],
|
||||
{ cwd, envOverlay: READ_ONLY_GIT_ENV },
|
||||
);
|
||||
if (result.truncated) {
|
||||
throw new Error("Commit renames exceeded the git output limit");
|
||||
}
|
||||
const renames = new Map<string, string>();
|
||||
const fields = result.stdout.split("\0");
|
||||
for (let index = 0; index + 2 < fields.length; index += 3) {
|
||||
renames.set(fields[index + 2]!, fields[index + 1]!);
|
||||
}
|
||||
return renames;
|
||||
}
|
||||
|
||||
/**
|
||||
* Fetches the unified diff of a single file as introduced by one commit and
|
||||
* parses it into the same {@link ParsedDiffFile} shape the diff subscription
|
||||
@@ -2665,8 +2719,20 @@ export async function getCommitFileDiff({
|
||||
path: string;
|
||||
}): Promise<ParsedDiffFile | null> {
|
||||
const worktreeRoot = await requireGitWorktreeRoot(cwd);
|
||||
const renames = await getCommitRenames(worktreeRoot, sha);
|
||||
const oldPath = renames.get(path);
|
||||
const paths = oldPath === undefined ? [path] : [oldPath, path];
|
||||
const { stdout } = await runGitCommand(
|
||||
["show", sha, "--format=", "--diff-merges=first-parent", ...GIT_PATCH_FORMAT_ARGS, "--", path],
|
||||
[
|
||||
"show",
|
||||
sha,
|
||||
"--format=",
|
||||
"--diff-merges=first-parent",
|
||||
"-M",
|
||||
...GIT_PATCH_FORMAT_ARGS,
|
||||
"--",
|
||||
...paths.map((filePath) => `:(literal)${filePath}`),
|
||||
],
|
||||
{
|
||||
cwd: worktreeRoot,
|
||||
envOverlay: READ_ONLY_GIT_ENV,
|
||||
@@ -2682,8 +2748,8 @@ export async function getCommitFileDiff({
|
||||
getNewFileContent: (file) => readGitFileContentAtRef(worktreeRoot, sha, file.path),
|
||||
});
|
||||
|
||||
// `--` scopes the diff to a single pathspec, so there is at most one real
|
||||
// entry. Pick by path to drop any stray header-only section the parser emits.
|
||||
// Including the rename source lets Git pair it with the destination. Select
|
||||
// the requested entry if another change also matches the source path.
|
||||
const file = parsedFiles.find((candidate) => candidate.path === path) ?? null;
|
||||
if (!file) {
|
||||
return null;
|
||||
|
||||
Reference in New Issue
Block a user