Fix native diff row expansion (#940)

* Fix structured diff parsing for no-prefix paths

Git can emit diff --git path path when diff.noprefix is enabled. The structured diff parser only handled the default a/path b/path form, so checkout diff assembly could lose hunks for affected files and render expanded rows empty.

Normalize optional a/ and b/ prefixes in both app and server parsers, and cover the no-prefix header form with regression tests.

* Fix no-prefix diff path parsing

---------

Co-authored-by: Mohamed Boudra <boudra.moha@gmail.com>
This commit is contained in:
c4605
2026-05-17 12:58:10 +02:00
committed by GitHub
parent bb5c3dae00
commit b3c272f720
5 changed files with 195 additions and 16 deletions

View File

@@ -185,6 +185,58 @@ describe("parseDiff", () => {
expect(hunk.lines[3].content).toBe("const bar = 3;");
});
it("parses no-prefix git diff headers", () => {
const files = parseDiff(
SIMPLE_DIFF.replace("a/example.ts b/example.ts", "example.ts example.ts")
.replace("--- a/example.ts", "--- example.ts")
.replace("+++ b/example.ts", "+++ example.ts"),
);
expect(files).toHaveLength(1);
expect(files[0].path).toBe("example.ts");
expect(files[0].additions).toBe(1);
expect(files[0].deletions).toBe(1);
expect(files[0].hunks).toHaveLength(1);
});
it("parses paths with spaces", () => {
const files = parseDiff(SIMPLE_DIFF.replaceAll("example.ts", "file with space.ts"));
expect(files).toHaveLength(1);
expect(files[0].path).toBe("file with space.ts");
expect(files[0].hunks).toHaveLength(1);
});
it("parses no-prefix paths with spaces", () => {
const files = parseDiff(
SIMPLE_DIFF.replaceAll("example.ts", "file with space.ts")
.replace(
"a/file with space.ts b/file with space.ts",
"file with space.ts file with space.ts",
)
.replace("--- a/file with space.ts", "--- file with space.ts")
.replace("+++ b/file with space.ts", "+++ file with space.ts"),
);
expect(files).toHaveLength(1);
expect(files[0].path).toBe("file with space.ts");
expect(files[0].hunks).toHaveLength(1);
});
it("preserves no-prefix paths that start with a or b", () => {
const files = parseDiff(
`${SIMPLE_DIFF.replace(
"diff --git a/example.ts b/example.ts",
"diff --git a/example.ts a/example.ts",
).replace("--- a/example.ts\n+++ b/example.ts", "--- a/example.ts\n+++ a/example.ts")}
${SIMPLE_DIFF.replaceAll("example.ts", "other.ts")
.replace("diff --git a/other.ts b/other.ts", "diff --git b/other.ts b/other.ts")
.replace("--- a/other.ts\n+++ b/other.ts", "--- b/other.ts\n+++ b/other.ts")}`,
);
expect(files.map((file) => file.path)).toEqual(["a/example.ts", "b/other.ts"]);
});
it("parses a diff with multiple hunks", () => {
const files = parseDiff(MULTI_HUNK_DIFF);

View File

@@ -30,14 +30,40 @@ function isDiffMetadataLine(line: string): boolean {
return DIFF_METADATA_PREFIXES.some((prefix) => line.startsWith(prefix));
}
function extractDiffPath(firstLine: string): string {
const pathMatch = firstLine.match(/a\/(.*?) b\//);
if (pathMatch) {
return pathMatch[1];
// Git's default patch headers use paired a/path and b/path prefixes, while
// diff.noprefix emits plain paths that may legitimately start with a/ or b/.
function usesDiffPathPrefixes(oldPath: string, newPath: string): boolean {
return oldPath.startsWith("a/") && newPath.startsWith("b/");
}
function extractPathFromMetadata(lines: string[], prefix: "--- " | "+++ "): string | null {
const line = lines.find((candidate) => candidate.startsWith(prefix));
if (!line) {
return null;
}
const newFileMatch = firstLine.match(/b\/(.+)$/);
if (newFileMatch) {
return newFileMatch[1];
const path = line.slice(prefix.length).replace(/\t.*$/, "").trimEnd();
return path === "/dev/null" ? null : path;
}
function extractDiffPath(lines: string[]): string {
const firstLine = lines[0] ?? "";
const prefixedPathMatch = firstLine.match(/^a\/(.+) b\/(.+)$/);
if (prefixedPathMatch) {
return prefixedPathMatch[2];
}
const metadataPath =
extractPathFromMetadata(lines, "+++ ") ?? extractPathFromMetadata(lines, "--- ");
if (metadataPath) {
return metadataPath;
}
const pathMatch = firstLine.match(/^(\S+)\s+(\S+)$/);
if (pathMatch) {
const [, oldPath, newPath] = pathMatch;
const path = newPath === "/dev/null" ? oldPath : newPath;
return usesDiffPathPrefixes(oldPath, newPath) ? path.slice(2) : path;
}
return "unknown";
}
@@ -118,11 +144,10 @@ export function parseDiff(diffText: string): ParsedDiffFile[] {
for (const section of fileSections) {
const lines = section.split("\n");
const firstLine = lines[0];
const isNew = section.includes("new file mode") || section.includes("--- /dev/null");
const isDeleted = section.includes("deleted file mode") || section.includes("+++ /dev/null");
const path = extractDiffPath(firstLine);
const path = extractDiffPath(lines);
const { hunks, additions, deletions } = parseHunks(lines);
files.push({ path, isNew, isDeleted, additions, deletions, hunks });

View File

@@ -188,6 +188,58 @@ describe("parseDiff", () => {
expect(hunk.lines[3].content).toBe("const bar = 3;");
});
it("parses no-prefix git diff headers", () => {
const files = parseDiff(
SIMPLE_DIFF.replace("a/example.ts b/example.ts", "example.ts example.ts")
.replace("--- a/example.ts", "--- example.ts")
.replace("+++ b/example.ts", "+++ example.ts"),
);
expect(files).toHaveLength(1);
expect(files[0].path).toBe("example.ts");
expect(files[0].additions).toBe(1);
expect(files[0].deletions).toBe(1);
expect(files[0].hunks).toHaveLength(1);
});
it("parses paths with spaces", () => {
const files = parseDiff(SIMPLE_DIFF.replaceAll("example.ts", "file with space.ts"));
expect(files).toHaveLength(1);
expect(files[0].path).toBe("file with space.ts");
expect(files[0].hunks).toHaveLength(1);
});
it("parses no-prefix paths with spaces", () => {
const files = parseDiff(
SIMPLE_DIFF.replaceAll("example.ts", "file with space.ts")
.replace(
"a/file with space.ts b/file with space.ts",
"file with space.ts file with space.ts",
)
.replace("--- a/file with space.ts", "--- file with space.ts")
.replace("+++ b/file with space.ts", "+++ file with space.ts"),
);
expect(files).toHaveLength(1);
expect(files[0].path).toBe("file with space.ts");
expect(files[0].hunks).toHaveLength(1);
});
it("preserves no-prefix paths that start with a or b", () => {
const files = parseDiff(
`${SIMPLE_DIFF.replace(
"diff --git a/example.ts b/example.ts",
"diff --git a/example.ts a/example.ts",
).replace("--- a/example.ts\n+++ b/example.ts", "--- a/example.ts\n+++ a/example.ts")}
${SIMPLE_DIFF.replaceAll("example.ts", "other.ts")
.replace("diff --git a/other.ts b/other.ts", "diff --git b/other.ts b/other.ts")
.replace("--- a/other.ts\n+++ b/other.ts", "--- b/other.ts\n+++ b/other.ts")}`,
);
expect(files.map((file) => file.path)).toEqual(["a/example.ts", "b/other.ts"]);
});
it("parses a diff with multiple hunks", () => {
const files = parseDiff(MULTI_HUNK_DIFF);

View File

@@ -39,11 +39,41 @@ interface ParseAndHighlightDiffOptions {
/**
* Parse a unified diff into structured data
*/
function extractPathFromDiffHeader(firstLine: string): string {
const pathMatch = firstLine.match(/a\/(.*?) b\//);
if (pathMatch) return pathMatch[1];
const newFileMatch = firstLine.match(/b\/(.+)$/);
if (newFileMatch) return newFileMatch[1];
// Git's default patch headers use paired a/path and b/path prefixes, while
// diff.noprefix emits plain paths that may legitimately start with a/ or b/.
function usesDiffPathPrefixes(oldPath: string, newPath: string): boolean {
return oldPath.startsWith("a/") && newPath.startsWith("b/");
}
function extractPathFromMetadata(lines: string[], prefix: "--- " | "+++ "): string | null {
const line = lines.find((candidate) => candidate.startsWith(prefix));
if (!line) {
return null;
}
const path = line.slice(prefix.length).replace(/\t.*$/, "").trimEnd();
return path === "/dev/null" ? null : path;
}
function extractPathFromDiffHeader(lines: string[]): string {
const firstLine = lines[0] ?? "";
const prefixedPathMatch = firstLine.match(/^a\/(.+) b\/(.+)$/);
if (prefixedPathMatch) {
return prefixedPathMatch[2];
}
const metadataPath =
extractPathFromMetadata(lines, "+++ ") ?? extractPathFromMetadata(lines, "--- ");
if (metadataPath) {
return metadataPath;
}
const pathMatch = firstLine.match(/^(\S+)\s+(\S+)$/);
if (pathMatch) {
const [, oldPath, newPath] = pathMatch;
const path = newPath === "/dev/null" ? oldPath : newPath;
return usesDiffPathPrefixes(oldPath, newPath) ? path.slice(2) : path;
}
return "unknown";
}
@@ -123,11 +153,10 @@ export function parseDiff(diffText: string): ParsedDiffFile[] {
for (const section of fileSections) {
const lines = section.split("\n");
const firstLine = lines[0];
const isNew = section.includes("new file mode") || section.includes("--- /dev/null");
const isDeleted = section.includes("deleted file mode") || section.includes("+++ /dev/null");
const path = extractPathFromDiffHeader(firstLine);
const path = extractPathFromDiffHeader(lines);
const { hunks, additions, deletions } = parseSectionBody(lines);

View File

@@ -348,6 +348,27 @@ const x = 1;
expect(removedLine?.tokens).toEqual([{ text: "old comment line", style: "comment" }]);
});
it("preserves no-prefix structured paths that start with a or b", async () => {
mkdirSync(join(repoDir, "a"));
mkdirSync(join(repoDir, "b"));
commitFile(repoDir, "a/example.ts", "const value = 1;\n", "add a-prefixed path");
commitFile(repoDir, "b/other.ts", "const value = 1;\n", "add b-prefixed path");
commitFile(repoDir, "file with space.ts", "const value = 1;\n", "add path with space");
execFileSync("git", ["config", "diff.noprefix", "true"], { cwd: repoDir });
writeFileSync(join(repoDir, "a/example.ts"), "const value = 2;\n");
writeFileSync(join(repoDir, "b/other.ts"), "const value = 2;\n");
writeFileSync(join(repoDir, "file with space.ts"), "const value = 2;\n");
const diff = await getCheckoutDiff(repoDir, { mode: "uncommitted", includeStructured: true });
expect(diff.structured?.map((file) => [file.path, file.hunks.length])).toEqual([
["a/example.ts", 1],
["b/other.ts", 1],
["file with space.ts", 1],
]);
});
it("returns checkout root metadata for normal repos", async () => {
const status = await getCheckoutStatus(repoDir);
expect(status.isGit).toBe(true);