Skip to content

Commit 124d3e2

Browse files
committed
fix(diff): highlight expanded context rows
1 parent 141a8ab commit 124d3e2

7 files changed

Lines changed: 305 additions & 15 deletions

File tree

src/ui/components/ui-components.test.tsx

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2219,6 +2219,61 @@ describe("UI components", () => {
22192219
}
22202220
});
22212221

2222+
test("PierreDiffView highlights expanded unchanged source rows", async () => {
2223+
const beforeLines = Array.from({ length: 30 }, (_, index) =>
2224+
index === 0
2225+
? "export const expandedMarker = 1;"
2226+
: `export const line${index + 1} = ${index + 1};`,
2227+
);
2228+
const afterLines = [...beforeLines];
2229+
afterLines[4] = "export const line5 = 999;";
2230+
const after = lines(...afterLines);
2231+
const file = buildTestDiffFile({
2232+
after,
2233+
before: lines(...beforeLines),
2234+
context: 3,
2235+
id: "expanded-highlight",
2236+
path: "expanded-highlight.ts",
2237+
});
2238+
const theme = resolveTheme("midnight", null);
2239+
const setup = await testRender(
2240+
<PierreDiffView
2241+
file={file}
2242+
layout="split"
2243+
theme={theme}
2244+
width={140}
2245+
selectedHunkIndex={0}
2246+
expandedGapKeys={new Set(["before:0"])}
2247+
sourceStatus={{ kind: "loaded", text: after }}
2248+
scrollable={false}
2249+
/>,
2250+
{ width: 144, height: 20 },
2251+
);
2252+
2253+
try {
2254+
let highlighted = false;
2255+
for (let iteration = 0; iteration < 400; iteration += 1) {
2256+
await act(async () => {
2257+
await setup.renderOnce();
2258+
await Bun.sleep(0);
2259+
await setup.renderOnce();
2260+
await Bun.sleep(0);
2261+
});
2262+
2263+
if (frameHasHighlightedMarker(setup.captureSpans(), "expandedMarker")) {
2264+
highlighted = true;
2265+
break;
2266+
}
2267+
}
2268+
2269+
expect(highlighted).toBe(true);
2270+
} finally {
2271+
await act(async () => {
2272+
setup.renderer.destroy();
2273+
});
2274+
}
2275+
});
2276+
22222277
test("PierreDiffView renders word-diff spans with a visibly different background in split view", async () => {
22232278
const file = createTestDiffFile(
22242279
"word-diff",

src/ui/diff/PierreDiffView.tsx

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useMemo } from "react";
1+
import { useCallback, useMemo } from "react";
22
import type { DiffFile, LayoutMode } from "../../core/types";
33
import { AgentInlineNote, AgentInlineNoteGuideCap } from "../components/panes/AgentInlineNote";
44
import type { VisibleAgentNote } from "../lib/agentAnnotations";
@@ -7,12 +7,13 @@ import { reviewRowId } from "../lib/ids";
77
import type { AppTheme } from "../themes";
88
import { findMaxLineNumber, findMaxLineNumberInRows } from "./codeColumns";
99
import { expandCollapsedRows, type FileSourceStatus } from "./expandCollapsedRows";
10-
import { buildSplitRows, buildStackRows } from "./pierre";
10+
import { buildSplitRows, buildStackRows, spansForHighlightedSourceLine } from "./pierre";
1111
import { plannedReviewRowVisible } from "./plannedReviewRows";
1212
import { buildReviewRenderPlan } from "./reviewRenderPlan";
1313
import { resolveVisiblePlannedRowWindow, type VisibleBodyBounds } from "./rowWindowing";
1414
import { diffMessage, DiffRowView, fitText } from "./renderRows";
1515
import { useHighlightedDiff } from "./useHighlightedDiff";
16+
import { useHighlightedSource } from "./useHighlightedSource";
1617

1718
const EMPTY_ANNOTATED_HUNK_INDICES = new Set<number>();
1819
const EMPTY_VISIBLE_AGENT_NOTES: VisibleAgentNote[] = [];
@@ -65,6 +66,23 @@ export function PierreDiffView({
6566
appearance: theme.appearance,
6667
shouldLoadHighlight,
6768
});
69+
const sourceTextForHighlight =
70+
sourceStatus?.kind === "loaded" && expandedGapKeys.size > 0 ? sourceStatus.text : undefined;
71+
const resolvedHighlightedSource = useHighlightedSource({
72+
file,
73+
text: sourceTextForHighlight,
74+
appearance: theme.appearance,
75+
shouldLoadHighlight: shouldLoadHighlight && expandedGapKeys.size > 0,
76+
});
77+
const sourceLineSpans = useCallback(
78+
(line: string | undefined, sourceLineNumber: number) =>
79+
spansForHighlightedSourceLine(
80+
line,
81+
resolvedHighlightedSource?.lines[sourceLineNumber],
82+
theme,
83+
),
84+
[resolvedHighlightedSource, theme],
85+
);
6886

6987
const rows = useMemo(
7088
() =>
@@ -86,10 +104,11 @@ export function PierreDiffView({
86104
expandCollapsedRows(rows, {
87105
layout,
88106
expandedKeys: expandedGapKeys,
107+
sourceLineSpans,
89108
sourceStatus,
90109
side: expansionSide,
91110
}),
92-
[rows, layout, expandedGapKeys, sourceStatus, expansionSide],
111+
[rows, layout, expandedGapKeys, sourceLineSpans, sourceStatus, expansionSide],
93112
);
94113
const plannedRows = useMemo(
95114
() =>

src/ui/diff/expandCollapsedRows.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,33 @@ describe("expandCollapsedRows", () => {
252252
}
253253
expect(inserted.cell.spans[0]?.text.includes("\t")).toBe(false);
254254
});
255+
256+
test("uses caller-provided spans for expanded source lines", () => {
257+
const rows: DiffRow[] = [makeCollapsedRow("before", 0, [2, 3], [2, 3]), makeHunkHeader(0)];
258+
const calls: Array<{ line: string | undefined; sourceLineNumber: number }> = [];
259+
260+
const result = expandCollapsedRows(rows, {
261+
layout: "stack",
262+
expandedKeys: new Set([gapKey("before", 0)]),
263+
sourceStatus: { kind: "loaded", text: SOURCE },
264+
sourceLineSpans: (line, sourceLineNumber) => {
265+
calls.push({ line, sourceLineNumber });
266+
return [{ text: `highlighted:${line ?? ""}`, fg: "#abcdef" }];
267+
},
268+
side: "new",
269+
});
270+
271+
expect(calls).toEqual([
272+
{ line: "beta", sourceLineNumber: 1 },
273+
{ line: "gamma", sourceLineNumber: 2 },
274+
]);
275+
276+
const inserted = result[1];
277+
if (!inserted || inserted.type !== "stack-line") {
278+
throw new Error("expected stack-line context row");
279+
}
280+
expect(inserted.cell.spans).toEqual([{ text: "highlighted:beta", fg: "#abcdef" }]);
281+
});
255282
});
256283

257284
describe("selectGapForKeyboardToggle", () => {

src/ui/diff/expandCollapsedRows.ts

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ export interface ExpandCollapsedRowsOptions {
1919
layout: ExpansionLayout;
2020
expandedKeys: ReadonlySet<string>;
2121
sourceStatus: FileSourceStatus | undefined;
22+
/** Optional syntax-aware span resolver for a zero-based source line. */
23+
sourceLineSpans?: (line: string | undefined, sourceLineNumber: number) => RenderSpan[];
2224
// Whose side's line indices in the source text. Defaults to "new".
2325
// For deleted files (no new side) callers should pass "old" instead.
2426
side?: "old" | "new";
@@ -89,9 +91,8 @@ function buildSplitContextRow(
8991
index: number,
9092
oldLineNumber: number,
9193
newLineNumber: number,
92-
line: string | undefined,
94+
spans: RenderSpan[],
9395
): Extract<DiffRow, { type: "split-line" }> {
94-
const spans = spansFor(line);
9596
const cell = (lineNumber: number): SplitLineCell => ({
9697
kind: "context",
9798
sign: " ",
@@ -117,14 +118,14 @@ function buildStackContextRow(
117118
index: number,
118119
oldLineNumber: number,
119120
newLineNumber: number,
120-
line: string | undefined,
121+
spans: RenderSpan[],
121122
): Extract<DiffRow, { type: "stack-line" }> {
122123
const cell: StackLineCell = {
123124
kind: "context",
124125
sign: " ",
125126
oldLineNumber,
126127
newLineNumber,
127-
spans: spansFor(line),
128+
spans,
128129
};
129130

130131
return {
@@ -148,7 +149,7 @@ export function expandCollapsedRows(
148149
rows: DiffRow[],
149150
options: ExpandCollapsedRowsOptions,
150151
): DiffRow[] {
151-
const { layout, expandedKeys, sourceStatus, side = "new" } = options;
152+
const { layout, expandedKeys, sourceLineSpans, sourceStatus, side = "new" } = options;
152153

153154
if (expandedKeys.size === 0) {
154155
return rows;
@@ -199,6 +200,7 @@ export function expandCollapsedRows(
199200
const newLineNumber = row.newRange[0] + offset;
200201
const sourceLineNumber = (side === "old" ? oldLineNumber : newLineNumber) - 1;
201202
const text = sourceLines[sourceLineNumber];
203+
const spans = sourceLineSpans?.(text, sourceLineNumber) ?? spansFor(text);
202204

203205
result.push(
204206
layout === "split"
@@ -209,7 +211,7 @@ export function expandCollapsedRows(
209211
offset,
210212
oldLineNumber,
211213
newLineNumber,
212-
text,
214+
spans,
213215
)
214216
: buildStackContextRow(
215217
row.fileId,
@@ -218,7 +220,7 @@ export function expandCollapsedRows(
218220
offset,
219221
oldLineNumber,
220222
newLineNumber,
221-
text,
223+
spans,
222224
),
223225
);
224226
}

src/ui/diff/pierre.test.ts

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,14 @@
11
import { describe, expect, test } from "bun:test";
22
import { parseDiffFromFile } from "@pierre/diffs";
33
import type { DiffFile } from "../../core/types";
4-
import { buildSplitRows, buildStackRows, loadHighlightedDiff, type DiffRow } from "./pierre";
4+
import {
5+
buildSplitRows,
6+
buildStackRows,
7+
loadHighlightedDiff,
8+
loadHighlightedSourceLines,
9+
spansForHighlightedSourceLine,
10+
type DiffRow,
11+
} from "./pierre";
512
import { resolveTheme } from "../themes";
613

714
function createDiffFile(): DiffFile {
@@ -181,6 +188,27 @@ describe("Pierre diff rows", () => {
181188
}
182189
});
183190

191+
test("builds syntax spans for highlighted full-source lines", async () => {
192+
const file = createDiffFile();
193+
const theme = resolveTheme("midnight", null);
194+
const text = "export const hiddenMarker = true;\n";
195+
const highlighted = await loadHighlightedSourceLines({
196+
file,
197+
text,
198+
appearance: theme.appearance,
199+
});
200+
const spans = spansForHighlightedSourceLine(
201+
"export const hiddenMarker = true;",
202+
highlighted.lines[0],
203+
theme,
204+
);
205+
206+
expect(spans.map((span) => span.text).join("")).toBe("export const hiddenMarker = true;");
207+
expect(spans.some((span) => span.text.includes("export") && typeof span.fg === "string")).toBe(
208+
true,
209+
);
210+
});
211+
184212
test("remaps Pierre markdown reds and greens away from diff-semantic hues", async () => {
185213
const file = createMarkdownDiffFile();
186214

0 commit comments

Comments
 (0)