mirror of
https://github.com/getpaseo/paseo.git
synced 2026-10-14 13:22:37 +00:00
Show actual edits when a file is renamed in Changes (#6559)
* fix(server): show actual edits when a file is renamed Path-limited patch requests discarded the detected rename source, so Git reported the destination as a new file. Include both literal paths and select the destination patch to preserve the rename and avoid duplicating changed copy sources. * fix(server): preserve small copy patches beside oversized sources Filter rename and copy output before enforcing the bounded request budget, so a modified source cannot hide its small copy patch. --------- Co-authored-by: paseo-bot[bot] <266920839+paseo-bot[bot]@users.noreply.github.com>
This commit is contained in:
paseo-bot[bot]
@@ -539,10 +539,129 @@ describe("checkout git utilities", () => {
|
||||
});
|
||||
|
||||
expect(diff.structured).toContainEqual(
|
||||
expect.objectContaining({ path: "renamed.txt", oldPath: "file.txt" }),
|
||||
expect.objectContaining({
|
||||
path: "renamed.txt",
|
||||
oldPath: "file.txt",
|
||||
additions: 0,
|
||||
deletions: 0,
|
||||
hunks: [],
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ mode: "uncommitted", oldPath: "to-rename.txt", path: "renamed.txt" },
|
||||
{ mode: "base", oldPath: "to-rename.txt", path: "renamed.txt" },
|
||||
{ mode: "uncommitted", oldPath: "old [file].txt", path: "new [file].txt" },
|
||||
] as const)(
|
||||
"shows only the edited line of a rename ($mode, $path)",
|
||||
async ({ mode, oldPath, path }) => {
|
||||
const original = Array.from({ length: 12 }, (_, index) => `line ${index + 1}`);
|
||||
commitFile(repoDir, oldPath, original.join("\n") + "\n", "file to rename");
|
||||
if (mode === "base") {
|
||||
execFileSync("git", ["checkout", "-b", "feature/rename"], { cwd: repoDir });
|
||||
}
|
||||
execFileSync("git", ["mv", oldPath, path], { cwd: repoDir });
|
||||
const edited = [...original];
|
||||
edited[5] = "edited line 6";
|
||||
writeFileSync(join(repoDir, path), edited.join("\n") + "\n");
|
||||
execFileSync("git", ["add", path], { cwd: repoDir });
|
||||
|
||||
expect(
|
||||
execFileSync("git", ["diff", "--name-status", "HEAD"], { cwd: repoDir }).toString(),
|
||||
).toMatch(/R\d+\t/);
|
||||
if (mode === "base") {
|
||||
execFileSync("git", ["-c", "commit.gpgsign=false", "commit", "-m", "rename and edit"], {
|
||||
cwd: repoDir,
|
||||
});
|
||||
}
|
||||
|
||||
const diff = await getCheckoutDiff(repoDir, {
|
||||
mode,
|
||||
...(mode === "base" ? { baseRef: "main" } : {}),
|
||||
includeStructured: true,
|
||||
});
|
||||
|
||||
expect(diff.structured).toEqual([
|
||||
expect.objectContaining({
|
||||
path,
|
||||
oldPath,
|
||||
isNew: false,
|
||||
isDeleted: false,
|
||||
additions: 1,
|
||||
deletions: 1,
|
||||
status: "ok",
|
||||
}),
|
||||
]);
|
||||
expect(diff.diff).toContain(`rename from ${oldPath}`);
|
||||
expect(diff.diff).toContain(`rename to ${path}`);
|
||||
expect(diff.diff).toContain("-line 6\n+edited line 6");
|
||||
expect(diff.diff).not.toContain("new file mode");
|
||||
},
|
||||
);
|
||||
|
||||
it("includes a changed copy source only once in the diff", async () => {
|
||||
const original =
|
||||
Array.from({ length: 12 }, (_, index) => `line ${index + 1}`).join("\n") + "\n";
|
||||
commitFile(repoDir, "source.txt", original, "copy source");
|
||||
execFileSync("git", ["config", "diff.renames", "copies"], { cwd: repoDir });
|
||||
writeFileSync(join(repoDir, "copy.txt"), original);
|
||||
writeFileSync(join(repoDir, "source.txt"), original.replace("line 6", "edited line 6"));
|
||||
execFileSync("git", ["add", "copy.txt", "source.txt"], { cwd: repoDir });
|
||||
expect(
|
||||
execFileSync("git", ["diff", "--name-status", "HEAD"], { cwd: repoDir }).toString(),
|
||||
).toContain("C100\tsource.txt\tcopy.txt");
|
||||
|
||||
const diff = await getCheckoutDiff(repoDir, {
|
||||
mode: "uncommitted",
|
||||
includeStructured: true,
|
||||
});
|
||||
expect(diff.structured).toEqual([
|
||||
expect.objectContaining({
|
||||
path: "copy.txt",
|
||||
oldPath: "source.txt",
|
||||
additions: 0,
|
||||
deletions: 0,
|
||||
}),
|
||||
expect.objectContaining({ path: "source.txt", additions: 1, deletions: 1 }),
|
||||
]);
|
||||
expect(diff.diff.match(/^diff --git a\/source.txt b\/source.txt$/gm)).toHaveLength(1);
|
||||
expect(diff.diff).toContain("copy from source.txt");
|
||||
expect(diff.diff).toContain("copy to copy.txt");
|
||||
});
|
||||
|
||||
it("keeps a small copy patch when its edited source exceeds the patch limit", async () => {
|
||||
const original =
|
||||
Array.from({ length: 12 }, (_, index) => `line ${index + 1}`).join("\n") + "\n";
|
||||
commitFile(repoDir, "source.txt", original, "copy source");
|
||||
execFileSync("git", ["config", "diff.renames", "copies"], { cwd: repoDir });
|
||||
writeFileSync(join(repoDir, "copy.txt"), original);
|
||||
writeFileSync(join(repoDir, "source.txt"), original + "added source line\n".repeat(80_000));
|
||||
execFileSync("git", ["add", "copy.txt", "source.txt"], { cwd: repoDir });
|
||||
expect(
|
||||
execFileSync("git", ["diff", "--name-status", "HEAD"], { cwd: repoDir }).toString(),
|
||||
).toContain("C100\tsource.txt\tcopy.txt");
|
||||
|
||||
const diff = await getCheckoutDiff(repoDir, {
|
||||
mode: "uncommitted",
|
||||
includeStructured: true,
|
||||
});
|
||||
expect(diff.structured).toEqual([
|
||||
expect.objectContaining({
|
||||
path: "copy.txt",
|
||||
oldPath: "source.txt",
|
||||
additions: 0,
|
||||
deletions: 0,
|
||||
hunks: [],
|
||||
status: "ok",
|
||||
}),
|
||||
expect.objectContaining({ path: "source.txt", status: "too_large", hunks: [] }),
|
||||
]);
|
||||
expect(diff.diff).toContain("copy from source.txt");
|
||||
expect(diff.diff).toContain("copy to copy.txt");
|
||||
expect(diff.diff).not.toContain("+added source line");
|
||||
});
|
||||
|
||||
it("reads the origin URL once when collecting facts for an origin-tracking branch", async () => {
|
||||
setupRemoteTrackingMain(repoDir, tempDir);
|
||||
|
||||
|
||||
@@ -721,12 +721,19 @@ async function getTrackedDiffTextForPath(input: {
|
||||
cwd: string;
|
||||
refsForDiff: CheckoutDiffRefs;
|
||||
path: string;
|
||||
oldPath?: string;
|
||||
ignoreWhitespace: boolean;
|
||||
}): Promise<{ path: string; text: string; truncated: boolean }> {
|
||||
const paths = input.oldPath ? [input.oldPath, input.path] : [input.path];
|
||||
const result = await runGitCommand(
|
||||
buildGitDiffArgs({
|
||||
ignoreWhitespace: input.ignoreWhitespace,
|
||||
extra: [...getCheckoutDiffRefArgs(input.refsForDiff), "--", `:(literal)${input.path}`],
|
||||
extra: [
|
||||
...(input.oldPath ? ["--diff-filter=RC"] : []),
|
||||
...getCheckoutDiffRefArgs(input.refsForDiff),
|
||||
"--",
|
||||
...paths.map((path) => `:(literal)${path}`),
|
||||
],
|
||||
}),
|
||||
{
|
||||
cwd: input.cwd,
|
||||
@@ -735,9 +742,22 @@ async function getTrackedDiffTextForPath(input: {
|
||||
},
|
||||
);
|
||||
|
||||
// A source path can participate in another rename/copy in the same request.
|
||||
// Return only the requested destination; the filter keeps source edits out of its budget.
|
||||
let text = result.stdout;
|
||||
if (input.oldPath && !result.truncated) {
|
||||
text = result.stdout
|
||||
.split(/(?=^diff --git )/m)
|
||||
.filter((section) => {
|
||||
const hunkStart = section.indexOf("\n@@ ");
|
||||
const header = hunkStart < 0 ? section : section.slice(0, hunkStart);
|
||||
return parseDiff(header)[0]?.path === input.path;
|
||||
})
|
||||
.join("");
|
||||
}
|
||||
return {
|
||||
path: input.path,
|
||||
text: result.stdout,
|
||||
text,
|
||||
truncated: result.truncated,
|
||||
};
|
||||
}
|
||||
@@ -3314,26 +3334,29 @@ async function processTrackedChanges(
|
||||
|
||||
let trackedDiffText = "";
|
||||
let trackedDiffBytes = 0;
|
||||
const renamedPaths = new Set(
|
||||
trackedChanges.filter((change) => change.oldPath).map((change) => change.path),
|
||||
const oldPathByPath = new Map(
|
||||
trackedChanges
|
||||
.filter((change) => change.oldPath)
|
||||
.map((change) => [change.path, change.oldPath]),
|
||||
);
|
||||
for (let start = 0; start < trackedDiffPaths.length; start += TRACKED_DIFF_BATCH_SIZE) {
|
||||
const paths = trackedDiffPaths.slice(start, start + TRACKED_DIFF_BATCH_SIZE);
|
||||
// Keep rename/copy path filtering identical to the existing per-file command.
|
||||
// Rename/copy detection needs both paths.
|
||||
const ordinary = await getTrackedDiffTexts({
|
||||
cwd,
|
||||
refsForDiff,
|
||||
paths: paths.filter((path) => !renamedPaths.has(path)),
|
||||
paths: paths.filter((path) => !oldPathByPath.has(path)),
|
||||
ignoreWhitespace,
|
||||
});
|
||||
const renamed = await Promise.all(
|
||||
paths
|
||||
.filter((path) => renamedPaths.has(path))
|
||||
.filter((path) => oldPathByPath.has(path))
|
||||
.map((path) =>
|
||||
getTrackedDiffTextForPath({
|
||||
cwd,
|
||||
refsForDiff,
|
||||
path,
|
||||
oldPath: oldPathByPath.get(path),
|
||||
ignoreWhitespace,
|
||||
}),
|
||||
),
|
||||
|
||||
Reference in New Issue
Block a user