From 8211497f521454a762dbf473037d7b6af83511a9 Mon Sep 17 00:00:00 2001 From: 0xferrous <0xferrous@proton.me> Date: Sat, 12 Sep 2026 04:26:52 +0000 Subject: [PATCH] fix(pager): render conflicted Git diffs --- .changeset/bright-diffs-conflict.md | 5 + .../hunk/src/core/patch/gitFormat.test.ts | 142 +++++++++++++++ packages/hunk/src/core/patch/gitFormat.ts | 167 +++++++++++++++++- packages/hunk/src/ui/staticDiffPager.test.ts | 62 +++++++ 4 files changed, 373 insertions(+), 3 deletions(-) create mode 100644 .changeset/bright-diffs-conflict.md diff --git a/.changeset/bright-diffs-conflict.md b/.changeset/bright-diffs-conflict.md new file mode 100644 index 000000000..ad225f2f3 --- /dev/null +++ b/.changeset/bright-diffs-conflict.md @@ -0,0 +1,5 @@ +--- +"hunkdiff": patch +--- + +Render Git combined conflict diffs from `git diff` during paused merges and rebases. diff --git a/packages/hunk/src/core/patch/gitFormat.test.ts b/packages/hunk/src/core/patch/gitFormat.test.ts index 14bdf1a46..5b1faf3d2 100644 --- a/packages/hunk/src/core/patch/gitFormat.test.ts +++ b/packages/hunk/src/core/patch/gitFormat.test.ts @@ -301,3 +301,145 @@ describe("sanitizeGitPatchText", () => { ); }); }); + +describe("combined Git patch normalization", () => { + test("converts a two-parent conflict diff into a first-parent unified diff", () => { + const combinedPatch = [ + "diff --cc notes.txt", + "index 0459513,a7453f0..0000000", + "--- a/notes.txt", + "+++ b/notes.txt", + "@@@ -1,1 -1,1 +1,5 @@@", + "++<<<<<<< HEAD", + " +upstream", + "++=======", + "+ feature", + "++>>>>>>> topic", + "", + ].join("\n"); + + expect(sanitizeGitPatchText(combinedPatch)).toBe( + [ + "diff --git a/notes.txt b/notes.txt", + "index 0459513,a7453f0..0000000", + "--- a/notes.txt", + "+++ b/notes.txt", + "@@ -1,1 +1,5 @@", + "+<<<<<<< HEAD", + " upstream", + "+=======", + "+feature", + "+>>>>>>> topic", + "", + ].join("\n"), + ); + }); + + test("omits lines deleted only from another parent", () => { + const combinedPatch = [ + "diff --cc notes.txt", + "index 1111111,2222222..0000000", + "--- a/notes.txt", + "+++ b/notes.txt", + "@@@ -1,3 -1,3 +1,3 @@@", + "- main", + " -topic", + "++resolved", + " common", + " tail", + "", + ].join("\n"); + + expect(sanitizeGitPatchText(combinedPatch)).toBe( + [ + "diff --git a/notes.txt b/notes.txt", + "index 1111111,2222222..0000000", + "--- a/notes.txt", + "+++ b/notes.txt", + "@@ -1,3 +1,3 @@", + "-main", + "+resolved", + " common", + " tail", + "", + ].join("\n"), + ); + }); + + test("rewrites binary combined entries without unified file headers", () => { + const combinedPatch = [ + "diff --cc data.bin", + "index ff69e82,fdd5296..0000000", + "Binary files differ", + "", + ].join("\n"); + + expect(sanitizeGitPatchText(combinedPatch)).toBe( + [ + "diff --git a/data.bin b/data.bin", + "index ff69e82,fdd5296..0000000", + "Binary files differ", + "", + ].join("\n"), + ); + }); + + test("keeps only the first parent path in combined-all-paths headers", () => { + const combinedPatch = [ + "diff --cc renamed.txt", + "index 1111111,2222222..0000000", + "--- a/first-name.txt", + "--- a/second-name.txt", + "+++ b/renamed.txt", + "@@@ -1,1 -1,1 +1,1 @@@", + "-old", + "++new", + "", + ].join("\n"); + + expect(sanitizeGitPatchText(combinedPatch)).toBe( + [ + "diff --git a/first-name.txt b/renamed.txt", + "index 1111111,2222222..0000000", + "--- a/first-name.txt", + "+++ b/renamed.txt", + "@@ -1,1 +1,1 @@", + "-old", + "+new", + "", + ].join("\n"), + ); + }); + + test("normalizes combined headers with quoted paths and multiple parents", () => { + const combinedPatch = [ + 'diff --combined "notes copy.txt"', + "index 1111111,2222222,3333333..4444444", + '--- "a/notes copy.txt"', + '+++ "b/notes copy.txt"', + "@@@@ -1,1 -1,1 -1,1 +1,2 @@@@", + " old", + "+++new", + "", + ].join("\n"); + + expect(sanitizeGitPatchText(combinedPatch)).toBe( + [ + "diff --git a/notes copy.txt b/notes copy.txt", + "index 1111111,2222222,3333333..4444444", + "--- a/notes copy.txt", + "+++ b/notes copy.txt", + "@@ -1,1 +1,2 @@", + " old", + "+new", + "", + ].join("\n"), + ); + }); + + test("leaves ordinary unified patches unchanged", () => { + const patch = "diff --git a/a.txt b/a.txt\n@@ -1 +1 @@\n-old\n+new\n"; + + expect(sanitizeGitPatchText(patch)).toBe(patch); + }); +}); diff --git a/packages/hunk/src/core/patch/gitFormat.ts b/packages/hunk/src/core/patch/gitFormat.ts index 2c608ae86..eae4fe189 100644 --- a/packages/hunk/src/core/patch/gitFormat.ts +++ b/packages/hunk/src/core/patch/gitFormat.ts @@ -25,6 +25,166 @@ export interface SanitizedGitPatch { filePaths: Array; } +/** Return the pathname token from a unified diff file header. */ +function unifiedFilePath(line: string, marker: "--- " | "+++ ") { + const value = line.slice(marker.length).trimEnd(); + if (value.startsWith('"')) { + const quoted = value.match(/^"(?:\\.|[^"\\])*"/)?.[0]; + if (quoted) { + return quoted; + } + } + + return value.split("\t", 1)[0] ?? ""; +} + +/** Move a unified-diff pathname onto the requested Git side prefix. */ +function gitSidePath(path: string, side: "a" | "b") { + if (path === "/dev/null") { + return path; + } + + const quoted = path.match(/^"((?:\\.|[^"\\])*)"(.*)$/); + const pathText = quoted?.[1] ?? path; + const suffix = quoted?.[2] ?? ""; + const unprefixed = pathText.replace(/^[ab]\//, ""); + const prefixed = `${side}/${unprefixed}`; + return quoted ? `"${prefixed}"${suffix}` : prefixed; +} + +/** Rewrite one combined-diff file header into a conventional two-sided Git header. */ +function rewriteCombinedFileHeader( + line: string, + fallbackPath: string, + oldPath: string | undefined, + newPath: string | undefined, +) { + const fallback = line.slice(line.indexOf(" ") + 1).trimEnd(); + const left = (oldPath ?? fallbackPath) || fallback; + const right = (newPath ?? fallbackPath) || fallback; + const leftPath = left === "/dev/null" ? right : left; + const rightPath = right === "/dev/null" ? left : right; + + return `diff --git ${gitSidePath(leftPath, "a")} ${gitSidePath(rightPath, "b")}`; +} + +/** Convert a combined hunk header to the first-parent unified hunk it contains. */ +function rewriteCombinedHunkHeader(line: string) { + const match = line.match(/^(@@@+)\s+(.+?)\s+(@@@+)(.*)$/); + if (!match || match[1] !== match[3]) { + return null; + } + + const openingMarker = match[1]!; + const parentCount = openingMarker.length - 1; + const ranges = match[2]!.trim().split(/\s+/); + if ( + parentCount < 2 || + ranges.length !== parentCount + 1 || + ranges.slice(0, parentCount).some((range) => !range.startsWith("-")) || + !ranges[parentCount]?.startsWith("+") + ) { + return null; + } + + return { + line: `@@ ${ranges[0]} ${ranges[parentCount]} @@${match[4]}`, + parentCount, + }; +} + +/** Convert Git combined conflict patches into first-parent unified patches Pierre can render. */ +export function normalizeCombinedGitPatch(patchText: string) { + const lines = patchText.split("\n"); + const normalizedLines: string[] = []; + let combinedHeaderIndex: number | undefined; + let combinedFallbackPath = ""; + let combinedOldPath: string | undefined; + let combinedNewPath: string | undefined; + let combinedParentCount: number | undefined; + + const resetCombinedState = () => { + combinedHeaderIndex = undefined; + combinedFallbackPath = ""; + combinedOldPath = undefined; + combinedNewPath = undefined; + combinedParentCount = undefined; + }; + + for (const line of lines) { + const combinedHeader = line.match(/^diff --(cc|combined) (.*)$/); + if (combinedHeader) { + resetCombinedState(); + combinedHeaderIndex = normalizedLines.length; + combinedFallbackPath = combinedHeader[2] ?? ""; + // Rewrite immediately so binary combined entries, which have no unified file headers, + // still reach the normal Git patch parser. + normalizedLines.push( + rewriteCombinedFileHeader(line, combinedFallbackPath, undefined, undefined), + ); + continue; + } + + if (line.startsWith("diff --git ")) { + resetCombinedState(); + normalizedLines.push(line); + continue; + } + + if (combinedHeaderIndex === undefined) { + normalizedLines.push(line); + continue; + } + + if (combinedParentCount === undefined && line.startsWith("--- ")) { + // `--combined-all-paths` emits one old-file header per parent. The first parent is the + // projection used below; discard the remaining parent headers before parsing. + if (combinedOldPath === undefined) { + combinedOldPath = unifiedFilePath(line, "--- "); + normalizedLines.push(line); + } + continue; + } + + if (combinedParentCount === undefined && line.startsWith("+++ ")) { + combinedNewPath = unifiedFilePath(line, "+++ "); + normalizedLines[combinedHeaderIndex] = rewriteCombinedFileHeader( + normalizedLines[combinedHeaderIndex]!, + combinedFallbackPath, + combinedOldPath, + combinedNewPath, + ); + normalizedLines.push(line); + continue; + } + + const combinedHunk = rewriteCombinedHunkHeader(line); + if (combinedHunk) { + combinedParentCount = combinedHunk.parentCount; + normalizedLines.push(combinedHunk.line); + continue; + } + + if (combinedParentCount !== undefined && line.length >= combinedParentCount) { + const prefix = line.slice(0, combinedParentCount); + const firstParentMarker = prefix[0]; + if (firstParentMarker && /^[ +-]+$/.test(prefix)) { + // A parent-only deletion has no representation in the first-parent/result diff. In a + // combined row, that shape is a context marker for the first parent plus `-` elsewhere. + if (firstParentMarker === " " && prefix.slice(1).includes("-")) { + continue; + } + normalizedLines.push(`${firstParentMarker}${line.slice(combinedParentCount)}`); + continue; + } + } + + normalizedLines.push(line); + } + + return normalizedLines.join("\n"); +} + const gitQuotedUtf8Decoder = new TextDecoder("utf-8", { fatal: true }); const gitQuotedUtf8Encoder = new TextEncoder(); const gitUnsafeDecodedHeaderCharacter = /[\x00-\x1f\x7f-\x9f]/; @@ -139,11 +299,12 @@ function decodeGitQuotedPath(path: string) { /** Normalize Git patch syntax and retain exact decoded paths separately from parser-safe text. */ export function sanitizeGitPatch(patchText: string): SanitizedGitPatch { - if (!patchText.includes("diff --git ")) { - return { text: patchText, filePaths: [] }; + const normalizedPatch = normalizeCombinedGitPatch(patchText); + if (!normalizedPatch.includes("diff --git ")) { + return { text: normalizedPatch, filePaths: [] }; } - const lines = patchText.split("\n"); + const lines = normalizedPatch.split("\n"); const normalizedLines: string[] = []; const filePaths: Array = []; let blockLines: string[] = []; diff --git a/packages/hunk/src/ui/staticDiffPager.test.ts b/packages/hunk/src/ui/staticDiffPager.test.ts index cce346b4c..1eb5e9072 100644 --- a/packages/hunk/src/ui/staticDiffPager.test.ts +++ b/packages/hunk/src/ui/staticDiffPager.test.ts @@ -47,6 +47,68 @@ describe("static diff pager", () => { expect(output).not.toContain("\x1b[?1049h"); }); + test("renders Git combined conflict diffs instead of reporting an empty review", async () => { + const patchText = [ + "diff --cc notes.txt", + "index 0459513,a7453f0..0000000", + "--- a/notes.txt", + "+++ b/notes.txt", + "@@@ -1,1 -1,1 +1,5 @@@", + "++<<<<<<< HEAD", + " +upstream", + "++=======", + "+ feature", + "++>>>>>>> topic", + "", + ].join("\n"); + + const plain = stripAnsi(await renderStaticDiffPager(patchText)); + + expect(plain).toContain("notes.txt modified +4 -0"); + expect(plain).toContain("<<<<<<< HEAD"); + expect(plain).toContain("upstream"); + expect(plain).toContain("feature"); + expect(plain).not.toContain("No files match the current filter."); + }); + + test("renders partially resolved combined conflicts without parent-only deletions", async () => { + const patchText = [ + "diff --cc notes.txt", + "index 1111111,2222222..0000000", + "--- a/notes.txt", + "+++ b/notes.txt", + "@@@ -1,3 -1,3 +1,3 @@@", + "- main", + " -topic", + "++resolved", + " common", + " tail", + "", + ].join("\n"); + + const plain = stripAnsi(await renderStaticDiffPager(patchText)); + + expect(plain).toContain("notes.txt modified +1 -1"); + expect(plain).toContain("resolved"); + expect(plain).not.toContain("topic"); + expect(plain).not.toContain("No files match the current filter."); + }); + + test("renders binary combined conflicts as binary file metadata", async () => { + const patchText = [ + "diff --cc data.bin", + "index ff69e82,fdd5296..0000000", + "Binary files differ", + "", + ].join("\n"); + + const plain = stripAnsi(await renderStaticDiffPager(patchText)); + + expect(plain).toContain("data.bin modified +0 -0"); + expect(plain).toContain("No textual changes."); + expect(plain).not.toContain("No files match the current filter."); + }); + test("honors configured hidden line numbers and hunk headers", async () => { const patchText = "diff --git a/a.ts b/a.ts\n--- a/a.ts\n+++ b/a.ts\n@@ -1 +1 @@\n-const value = 1;\n+const value = 2;\n";