From b84504e53205a95fdd2b9e3ac79761c72b905488 Mon Sep 17 00:00:00 2001 From: Anish Jain Date: Tue, 8 Sep 2026 02:42:10 +0000 Subject: [PATCH] refactor(session-ui): simplify completePatchContents in session-diff.ts Reduce the Qlty-reported complexity of completePatchContents (packages/session-ui/src/components/session-diff.ts:74, complexity 37, 7 returns) without changing behavior: - Replace the stateful line loop with a marker-to-side lookup table and a map/filter pass; a "\ No newline" marker is handled by looking one line ahead instead of mutating the previous entry. - Move the shared try/parsePatch into parseFirstPatch, which patchInput also used to duplicate. Add tests for the branches that were not exercised before: a single hunk that starts after line 1, a no-newline marker after a context line, and a blank line inside a hunk. Co-Authored-By: Claude Fable 5.1 --- .../src/components/session-diff.test.ts | 37 ++++++++ .../session-ui/src/components/session-diff.ts | 94 ++++++++----------- 2 files changed, 75 insertions(+), 56 deletions(-) diff --git a/packages/session-ui/src/components/session-diff.test.ts b/packages/session-ui/src/components/session-diff.test.ts index ef0db6c6..a7c242ff 100644 --- a/packages/session-ui/src/components/session-diff.test.ts +++ b/packages/session-ui/src/components/session-diff.test.ts @@ -132,4 +132,41 @@ describe("session diff", () => { expect(text(view, "deletions")).toBe("") expect(text(view, "additions")).toBe("") }) + + test("keeps single hunks that start after the first line partial", () => { + const fileDiff = resolveFileDiff({ + file: "a.ts", + patch: + "Index: a.ts\n===================================================================\n--- a.ts\t\n+++ a.ts\t\n@@ -5,2 +5,2 @@\n one\n-two\n+three\n", + }) + + expect(fileDiff.isPartial).toBe(true) + expect(fileDiff.additionLines).toEqual(["one\n", "three\n"]) + }) + + test("drops the final newline on both sides after a trailing context line", () => { + const view = normalize({ + file: "a.ts", + patch: + "Index: a.ts\n===================================================================\n--- a.ts\t\n+++ a.ts\t\n@@ -1,2 +1,2 @@\n-two\n+three\n one\n\\ No newline at end of file\n", + additions: 1, + deletions: 1, + status: "modified" as const, + }) + + expect(view.fileDiff.isPartial).toBe(false) + expect(text(view, "deletions")).toBe("two\none") + expect(text(view, "additions")).toBe("three\none") + }) + + test("keeps patches with blank hunk lines partial", () => { + const fileDiff = resolveFileDiff({ + file: "a.ts", + patch: + "Index: a.ts\n===================================================================\n--- a.ts\t\n+++ a.ts\t\n@@ -1,3 +1,3 @@\n one\n\n-two\n+three\n", + }) + + expect(fileDiff.name).toBe("a.ts") + expect(fileDiff.isPartial).toBe(true) + }) }) diff --git a/packages/session-ui/src/components/session-diff.ts b/packages/session-ui/src/components/session-diff.ts index 2fbd0222..964d09c5 100644 --- a/packages/session-ui/src/components/session-diff.ts +++ b/packages/session-ui/src/components/session-diff.ts @@ -71,68 +71,50 @@ function fileDiffFromPatch(file: string, patch: string) { return value } -function completePatchContents(patch: string) { +const patchSides: Record = { + "-": ["before"], + "+": ["after"], + " ": ["before", "after"], + "\\": [], +} + +function parseFirstPatch(patch: string) { try { - const parsed = parsePatch(patch)[0] - if (!parsed || (!parsed.index && !parsed.oldFileName && !parsed.newFileName)) return - // Snapshot and VCS producers request full context. Tool patches use jsdiff's shorter default context. - if (!patch.startsWith("diff --git ") && !/^--- [^\n]*\t\r?\n\+\+\+ [^\n]*\t(?:\r?\n|$)/m.test(patch)) return - // Full patches collapse into one leading hunk. Separated hunks omit ranges and must stay partial. - if (parsed.hunks.length !== 1) return - - const hunk = parsed.hunks[0] - if (!hunk || hunk.oldStart > 1 || hunk.newStart > 1) return - - const before: Array<{ text: string; newline: boolean }> = [] - const after: Array<{ text: string; newline: boolean }> = [] - let previous: "-" | "+" | " " | undefined - - for (const line of hunk.lines) { - if (line.startsWith("\\")) { - if (previous === "-" || previous === " ") { - const value = before.at(-1) - if (value) value.newline = false - } - if (previous === "+" || previous === " ") { - const value = after.at(-1) - if (value) value.newline = false - } - continue - } - if (line.startsWith("-")) { - before.push({ text: line.slice(1), newline: true }) - previous = "-" - continue - } - if (line.startsWith("+")) { - after.push({ text: line.slice(1), newline: true }) - previous = "+" - continue - } - if (!line.startsWith(" ")) return - before.push({ text: line.slice(1), newline: true }) - after.push({ text: line.slice(1), newline: true }) - previous = " " - } - - const text = (lines: Array<{ text: string; newline: boolean }>) => - lines.map((line) => line.text + (line.newline ? "\n" : "")).join("") - return { before: text(before), after: text(after) } + return parsePatch(patch)[0] } catch { - return + return undefined } } +function completePatchContents(patch: string) { + const parsed = parseFirstPatch(patch) + if (!parsed || (!parsed.index && !parsed.oldFileName && !parsed.newFileName)) return + // Snapshot and VCS producers request full context. Tool patches use jsdiff's shorter default context. + if (!patch.startsWith("diff --git ") && !/^--- [^\n]*\t\r?\n\+\+\+ [^\n]*\t(?:\r?\n|$)/m.test(patch)) return + // Full patches collapse into one leading hunk. Separated hunks omit ranges and must stay partial. + const hunk = parsed.hunks.length === 1 ? parsed.hunks[0] : undefined + if (!hunk || hunk.oldStart > 1 || hunk.newStart > 1) return + if (hunk.lines.some((line) => !patchSides[line[0]])) return + + // A "\ No newline at end of file" marker strips the newline from the line right before it. + const lines = hunk.lines.map((line, index) => ({ + sides: patchSides[line[0]], + text: line.slice(1) + (hunk.lines[index + 1]?.startsWith("\\") ? "" : "\n"), + })) + const text = (side: "before" | "after") => + lines + .filter((line) => line.sides.includes(side)) + .map((line) => line.text) + .join("") + return { before: text("before"), after: text("after") } +} + function patchInput(file: string, patch: string) { - try { - const parsed = parsePatch(patch)[0] - if (!parsed) return - if (parsed.index || parsed.oldFileName || parsed.newFileName) return patch - if (!parsed.hunks.length) return - return `Index: ${file}\n===================================================================\n--- ${file}\t\n+++ ${file}\t\n${patch}` - } catch { - return - } + const parsed = parseFirstPatch(patch) + if (!parsed) return + if (parsed.index || parsed.oldFileName || parsed.newFileName) return patch + if (!parsed.hunks.length) return + return `Index: ${file}\n===================================================================\n--- ${file}\t\n+++ ${file}\t\n${patch}` } function fileDiffFromContent(file: string, before: string, after: string) {