Skip to content

Commit 34418e8

Browse files
committed
refactor: simplify diff route — use execFileAsync, merge ref helpers
- Replace manual spawn+buffer with execFileAsync for collecting git diff - Merge getOldRef/getNewRef into single getContentRefs returning both - Extract fetchContent helper to eliminate nested ternary - Extract MAX_FILE_BYTES constant
1 parent 33b98b0 commit 34418e8

1 file changed

Lines changed: 27 additions & 49 deletions

File tree

packages/cli/src/routes/diff.ts

Lines changed: 27 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { execFile, spawn } from "node:child_process";
1+
import { execFile } from "node:child_process";
22
import fs from "node:fs/promises";
33
import path from "node:path";
44
import { promisify } from "node:util";
@@ -14,6 +14,8 @@ import { writeJson } from "./json.js";
1414

1515
const execFileAsync = promisify(execFile);
1616

17+
const MAX_FILE_BYTES = 5 * 1024 * 1024;
18+
1719
export function diffRoutes(db: StageDb): Route[] {
1820
return [
1921
{
@@ -45,7 +47,11 @@ export function diffRoutes(db: StageDb): Route[] {
4547
run.scopeKind === SCOPE_KIND.COMMITTED ? "private, max-age=300" : "no-store";
4648

4749
try {
48-
const patch = await collectGitDiff(repoRoot, args);
50+
const { stdout: patch } = await execFileAsync("git", args, {
51+
cwd: repoRoot,
52+
encoding: "utf8",
53+
maxBuffer: 50 * 1024 * 1024,
54+
});
4955
const fileContents = await buildFileContents(run, repoRoot, patch);
5056
const body: DiffResponse = { patch, fileContents };
5157
res.writeHead(200, {
@@ -62,28 +68,6 @@ export function diffRoutes(db: StageDb): Route[] {
6268
];
6369
}
6470

65-
function collectGitDiff(cwd: string, args: string[]): Promise<string> {
66-
return new Promise((resolve, reject) => {
67-
const child = spawn("git", args, { cwd, stdio: ["ignore", "pipe", "pipe"] });
68-
const stdoutChunks: Buffer[] = [];
69-
let stderr = "";
70-
71-
child.stdout.on("data", (chunk: Buffer) => stdoutChunks.push(chunk));
72-
child.stderr.on("data", (chunk: Buffer) => {
73-
stderr += chunk.toString("utf8");
74-
});
75-
76-
child.on("error", reject);
77-
child.on("close", (code) => {
78-
if (code === 0) {
79-
resolve(Buffer.concat(stdoutChunks).toString("utf8"));
80-
} else {
81-
reject(new Error(stderr.trim() || `git exited with code ${code}`));
82-
}
83-
});
84-
});
85-
}
86-
8771
const MINUS_RE = /^--- (?:a\/)?(.+)$/m;
8872
const PLUS_RE = /^\+\+\+ (?:b\/)?(.+)$/m;
8973
const BINARY_RE = /^Binary files/m;
@@ -129,7 +113,7 @@ async function getGitFileContent(
129113
const { stdout } = await execFileAsync("git", ["show", `${ref}:${filePath}`], {
130114
cwd,
131115
encoding: "utf8",
132-
maxBuffer: 5 * 1024 * 1024,
116+
maxBuffer: MAX_FILE_BYTES,
133117
});
134118
return stdout;
135119
} catch {
@@ -148,30 +132,29 @@ async function readFileContent(repoRoot: string, filePath: string): Promise<stri
148132
}
149133
}
150134

151-
function getOldRef(run: ChapterRunRow): string {
152-
if (run.scopeKind === SCOPE_KIND.COMMITTED) return run.baseSha;
135+
function getContentRefs(run: ChapterRunRow): { oldRef: string; newRef: string | "DISK" } {
136+
if (run.scopeKind === SCOPE_KIND.COMMITTED) {
137+
return { oldRef: run.baseSha, newRef: run.headSha };
138+
}
153139
switch (run.workingTreeRef) {
154140
case WORKING_TREE_REF.UNSTAGED:
155-
return "";
141+
return { oldRef: "", newRef: "DISK" };
156142
case WORKING_TREE_REF.STAGED:
143+
return { oldRef: "HEAD", newRef: "" };
157144
case WORKING_TREE_REF.WORK:
158-
return "HEAD";
145+
return { oldRef: "HEAD", newRef: "DISK" };
159146
default:
160-
return "HEAD";
147+
return { oldRef: "HEAD", newRef: "HEAD" };
161148
}
162149
}
163150

164-
function getNewRef(run: ChapterRunRow): string | "DISK" {
165-
if (run.scopeKind === SCOPE_KIND.COMMITTED) return run.headSha;
166-
switch (run.workingTreeRef) {
167-
case WORKING_TREE_REF.UNSTAGED:
168-
case WORKING_TREE_REF.WORK:
169-
return "DISK";
170-
case WORKING_TREE_REF.STAGED:
171-
return "";
172-
default:
173-
return "HEAD";
174-
}
151+
function fetchContent(
152+
repoRoot: string,
153+
ref: string | "DISK",
154+
filePath: string,
155+
): Promise<string | null> {
156+
if (ref === "DISK") return readFileContent(repoRoot, filePath);
157+
return getGitFileContent(repoRoot, ref, filePath);
175158
}
176159

177160
async function buildFileContents(
@@ -180,21 +163,16 @@ async function buildFileContents(
180163
patch: string,
181164
): Promise<FileContentsMap> {
182165
const files = parseFilePathsFromPatch(patch);
183-
const oldRef = getOldRef(run);
184-
const newRef = getNewRef(run);
166+
const { oldRef, newRef } = getContentRefs(run);
185167

186168
const entries = await Promise.all(
187169
files.map(async ({ oldPath, newPath, isBinary }) => {
188170
const key = newPath ?? oldPath;
189171
if (!key || isBinary) return null;
190172

191173
const [oldContent, newContent] = await Promise.all([
192-
oldPath ? getGitFileContent(repoRoot, oldRef, oldPath) : Promise.resolve(null),
193-
newPath
194-
? newRef === "DISK"
195-
? readFileContent(repoRoot, newPath)
196-
: getGitFileContent(repoRoot, newRef, newPath)
197-
: Promise.resolve(null),
174+
oldPath ? fetchContent(repoRoot, oldRef, oldPath) : Promise.resolve(null),
175+
newPath ? fetchContent(repoRoot, newRef, newPath) : Promise.resolve(null),
198176
]);
199177

200178
return [key, { oldContent, newContent }] as const;

0 commit comments

Comments
 (0)