Skip to content

Commit 719fd96

Browse files
l10veu602yslee-abuttonclaude
authored
fix(review): show P4 added files in code review (#494)
P4 added files (p4 add) were not appearing in the review UI on Windows. The root cause was \r\n line endings from P4 command output — splitting on \n left \r in file paths, causing Bun.file() to fail silently. - Normalize \r\n to \n in runP4() stdout for all P4 commands - Strip \r\n from file content in getNewFileDiff() - Replace fragile index-based p4 where parsing with -ztag structured output, which is also more robust for stream depots and overlays Co-authored-by: yslee <yslee@abutton.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent d710284 commit 719fd96

1 file changed

Lines changed: 30 additions & 16 deletions

File tree

packages/server/p4.ts

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ async function runP4(
3232
proc.exited,
3333
]);
3434

35-
return { stdout, stderr, exitCode };
35+
return { stdout: stdout.replace(/\r\n/g, "\n"), stderr, exitCode };
3636
}
3737

3838
// --- Path helpers ---
@@ -248,7 +248,7 @@ async function getNewFileDiff(
248248
): Promise<string> {
249249
try {
250250
const content = await Bun.file(localPath).text();
251-
const lines = content.split("\n");
251+
const lines = content.replace(/\r\n/g, "\n").split("\n");
252252
const header = [
253253
`diff --git a/${relativePath} b/${relativePath}`,
254254
"new file mode 100644",
@@ -274,23 +274,37 @@ async function batchResolveDepotPaths(
274274
const result = new Map<string, { localPath: string; relativePath: string }>();
275275
if (depotPaths.length === 0) return result;
276276

277-
const whereResult = await runP4(["where", ...depotPaths], { cwd });
277+
// Use -ztag for structured output — avoids fragile index-based parsing
278+
// that breaks with stream depots, overlays, or unmapped path error lines
279+
const whereResult = await runP4(["-ztag", "where", ...depotPaths], { cwd });
278280
if (whereResult.exitCode !== 0) return result;
279281

280-
const outputLines = whereResult.stdout.trim().split("\n");
281-
for (let i = 0; i < outputLines.length && i < depotPaths.length; i++) {
282-
const whereOutput = outputLines[i];
283-
const rootIdx = whereOutput.indexOf(normalizePath(normalizedRoot).length > 0 ? normalizedRoot : "NOMATCH");
284-
// Find the local path by looking for the client root
285-
const clientRootBackslash = normalizedRoot.replace(/\//g, "\\");
286-
const bsIdx = whereOutput.indexOf(clientRootBackslash);
287-
const fwIdx = whereOutput.indexOf(normalizedRoot);
288-
const idx = bsIdx !== -1 ? bsIdx : fwIdx;
289-
if (idx === -1) continue;
290-
291-
const localPath = normalizePath(whereOutput.slice(idx));
282+
let currentDepot = "";
283+
let currentPath = "";
284+
285+
for (const line of whereResult.stdout.split("\n")) {
286+
const tagMatch = line.match(/^\.\.\. (\w+) (.+)/);
287+
if (!tagMatch) {
288+
if (currentDepot && currentPath) {
289+
const localPath = normalizePath(currentPath);
290+
const relativePath = toRelativePath(localPath, normalizedRoot);
291+
result.set(currentDepot, { localPath, relativePath });
292+
}
293+
currentDepot = "";
294+
currentPath = "";
295+
continue;
296+
}
297+
298+
const [, field, value] = tagMatch;
299+
if (field === "depotFile") currentDepot = value;
300+
if (field === "path") currentPath = value;
301+
}
302+
303+
// Handle last record (no trailing blank line)
304+
if (currentDepot && currentPath) {
305+
const localPath = normalizePath(currentPath);
292306
const relativePath = toRelativePath(localPath, normalizedRoot);
293-
result.set(depotPaths[i], { localPath, relativePath });
307+
result.set(currentDepot, { localPath, relativePath });
294308
}
295309

296310
return result;

0 commit comments

Comments
 (0)