Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions src/core/diffPaths.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import type { FileDiffMetadata } from "@pierre/diffs";

/** Remove parser-added CR/LF suffixes from diff paths without touching meaningful spaces. */
export function normalizeDiffPath(path: string | undefined) {
return path?.replace(/[\r\n]+$/u, "");
}

/** Sanitize parsed diff metadata path fields before the UI or loaders consume them. */
export function normalizeDiffMetadataPaths(metadata: FileDiffMetadata): FileDiffMetadata {
const name = normalizeDiffPath(metadata.name) ?? metadata.name;
const prevName = normalizeDiffPath(metadata.prevName);

if (name === metadata.name && prevName === metadata.prevName) {
return metadata;
}

return {
...metadata,
name,
prevName,
};
}
24 changes: 24 additions & 0 deletions src/core/loaders.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -720,6 +720,30 @@ describe("loadAppBootstrap", () => {
).rejects.toThrow("`hunk stash show stash@{99}` could not resolve stash entry `stash@{99}`.");
});

test("strips parser-added line endings from rename-only paths", async () => {
const bootstrap = await loadAppBootstrap({
kind: "patch",
text: [
"diff --git a/pi/extensions/loop.ts b/agents/pi/extensions/notify.ts",
"similarity index 100%",
"rename from pi/extensions/loop.ts",
"rename to agents/pi/extensions/notify.ts",
].join("\n"),
options: { mode: "auto" },
});

expect(bootstrap.changeset.files).toHaveLength(1);
expect(bootstrap.changeset.files[0]).toMatchObject({
path: "agents/pi/extensions/notify.ts",
previousPath: "pi/extensions/loop.ts",
metadata: {
name: "agents/pi/extensions/notify.ts",
prevName: "pi/extensions/loop.ts",
type: "rename-pure",
},
});
});

test("treats malformed inline patch text as an empty review instead of throwing", async () => {
const bootstrap = await loadAppBootstrap({
kind: "patch",
Expand Down
20 changes: 13 additions & 7 deletions src/core/loaders.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { createTwoFilesPatch } from "diff";
import { resolve as resolvePath } from "node:path";
import { findAgentFileContext, loadAgentContext } from "./agent";
import { createSkippedBinaryMetadata, isProbablyBinaryFile, patchLooksBinary } from "./binary";
import { normalizeDiffMetadataPaths, normalizeDiffPath } from "./diffPaths";
import {
buildGitDiffArgs,
buildGitShowArgs,
Expand Down Expand Up @@ -123,6 +124,7 @@ function findPatchChunk(metadata: FileDiffMetadata, chunks: string[], index: num
return (
chunks.find((chunk) =>
[metadata.name, metadata.prevName]
.map(normalizeDiffPath)
.filter((value): value is string => Boolean(value))
.map(stripPrefixes)
.some(
Expand All @@ -148,15 +150,19 @@ function buildDiffFile(
agentContext: AgentContext | null,
{ isUntracked, previousPath, isBinary }: BuildDiffFileOptions = {},
): DiffFile {
const normalizedMetadata = normalizeDiffMetadataPaths(metadata);
const path = normalizedMetadata.name;
const resolvedPreviousPath = normalizeDiffPath(previousPath) ?? normalizedMetadata.prevName;

return {
id: `${sourcePrefix}:${index}:${metadata.name}`,
path: metadata.name,
previousPath: previousPath ?? metadata.prevName,
id: `${sourcePrefix}:${index}:${path}`,
path,
previousPath: resolvedPreviousPath,
patch,
language: getFiletypeFromFileName(metadata.name) ?? undefined,
stats: countDiffStats(metadata),
metadata,
agent: findAgentFileContext(agentContext, metadata.name, metadata.prevName),
language: getFiletypeFromFileName(path) ?? undefined,
stats: countDiffStats(normalizedMetadata),
metadata: normalizedMetadata,
agent: findAgentFileContext(agentContext, path, resolvedPreviousPath),
isUntracked,
isBinary: isBinary ?? patchLooksBinary(patch),
};
Expand Down
11 changes: 7 additions & 4 deletions src/opentui/HunkDiffView.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { useMemo } from "react";
import { patchLooksBinary } from "../core/binary";
import { normalizeDiffMetadataPaths, normalizeDiffPath } from "../core/diffPaths";
import type { DiffFile } from "../core/types";
import { findMaxLineNumber } from "../ui/diff/codeColumns";
import { buildSplitRows, buildStackRows } from "../ui/diff/pierre";
Expand Down Expand Up @@ -28,17 +29,19 @@ function countDiffStats(metadata: HunkDiffFile["metadata"]) {
/** Adapt the public diff shape into Hunk's internal file model without exposing app-only fields. */
function toInternalDiffFile(diff: HunkDiffFile): DiffFile {
const patch = diff.patch ?? "";
const metadata = normalizeDiffMetadataPaths(diff.metadata);
const path = normalizeDiffPath(diff.path) ?? metadata.name;

return {
agent: null,
id: diff.id,
isBinary: patchLooksBinary(patch),
language: diff.language,
metadata: diff.metadata,
metadata,
patch,
path: diff.path ?? diff.metadata.name,
previousPath: diff.metadata.prevName,
stats: countDiffStats(diff.metadata),
path,
previousPath: metadata.prevName,
stats: countDiffStats(metadata),
};
}

Expand Down
20 changes: 19 additions & 1 deletion src/ui/lib/files.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { describe, expect, test } from "bun:test";
import { createTestDiffFile, lines } from "../../../test/helpers/diff-helpers";
import { buildSidebarEntries } from "./files";
import { buildSidebarEntries, fileLabelParts } from "./files";

describe("files helpers", () => {
test("buildSidebarEntries hides zero-value sidebar stats", () => {
Expand Down Expand Up @@ -60,4 +60,22 @@ describe("files helpers", () => {
deletionsText: null,
});
});

test("fileLabelParts strips parser-added line endings from rename labels", () => {
const renamedAcrossDirectories = {
...createTestDiffFile({
id: "rename-across-dirs",
path: "agents/pi/extensions/notify.ts",
previousPath: "pi/extensions/loop.ts\n",
before: lines("export const stable = true;"),
after: lines("export const stable = true;"),
}),
stats: { additions: 0, deletions: 0 },
};

expect(fileLabelParts(renamedAcrossDirectories)).toEqual({
filename: "pi/extensions/loop.ts -> agents/pi/extensions/notify.ts",
stateLabel: null,
});
});
});
28 changes: 18 additions & 10 deletions src/ui/lib/files.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { basename, dirname } from "node:path/posix";
import type { FileDiffMetadata } from "@pierre/diffs";
import { normalizeDiffPath } from "../../core/diffPaths";
import type { AgentAnnotation, DiffFile } from "../../core/types";

export interface FileListEntry {
Expand All @@ -22,12 +23,15 @@ export type SidebarEntry = FileListEntry | FileGroupEntry;

/** Build the filename-first label shown inside one sidebar row. */
function sidebarFileName(file: DiffFile) {
if (!file.previousPath || file.previousPath === file.path) {
return basename(file.path);
const path = normalizeDiffPath(file.path) ?? file.path;
const previousPath = normalizeDiffPath(file.previousPath);

if (!previousPath || previousPath === path) {
return basename(path);
}

const previousName = basename(file.previousPath);
const nextName = basename(file.path);
const previousName = basename(previousPath);
const nextName = basename(path);
return previousName === nextName ? nextName : `${previousName} -> ${nextName}`;
}

Expand Down Expand Up @@ -91,7 +95,11 @@ export function filterReviewFiles(files: DiffFile[], query: string): DiffFile[]
}

return files.filter((file) => {
const haystack = [file.path, file.previousPath, file.agent?.summary]
const haystack = [
normalizeDiffPath(file.path),
normalizeDiffPath(file.previousPath),
file.agent?.summary,
]
.filter(Boolean)
.join(" ")
.toLowerCase();
Expand All @@ -105,7 +113,8 @@ export function buildSidebarEntries(files: DiffFile[]): SidebarEntry[] {
let activeGroup: string | null = null;

files.forEach((file, index) => {
const group = dirname(file.path);
const path = normalizeDiffPath(file.path) ?? file.path;
const group = dirname(path);
const nextGroup = group === "." ? null : group;

if (nextGroup !== activeGroup) {
Expand Down Expand Up @@ -148,10 +157,9 @@ export function fileLabelParts(file: DiffFile | undefined): {
return { filename: "No file selected", stateLabel: null };
}

const baseLabel =
file.previousPath && file.previousPath !== file.path
? `${file.previousPath} -> ${file.path}`
: file.path;
const path = normalizeDiffPath(file.path) ?? file.path;
const previousPath = normalizeDiffPath(file.previousPath);
const baseLabel = previousPath && previousPath !== path ? `${previousPath} -> ${path}` : path;

// Determine state label for special cases
let stateLabel: string | null = null;
Expand Down
Loading