Skip to content

Commit 3d2431e

Browse files
benvinegarbenvinegar
committed
fix: keep syntax highlighting stable across viewport navigation
1 parent 8830f8b commit 3d2431e

3 files changed

Lines changed: 91 additions & 95 deletions

File tree

‎src/ui/diff/PierreDiffView.tsx‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ import {
1717

1818
const EMPTY_ANNOTATED_HUNK_INDICES = new Set<number>();
1919
const EMPTY_VISIBLE_AGENT_NOTES: VisibleAgentNote[] = [];
20+
const SHARED_HIGHLIGHTED_DIFF_CACHE = new Map<string, HighlightedDiffCode>();
21+
const SHARED_HIGHLIGHT_PROMISES = new Map<string, Promise<HighlightedDiffCode>>();
2022

2123
/** Clamp a label to one terminal row with an ellipsis. */
2224
function fitText(text: string, width: number) {
@@ -895,23 +897,21 @@ export function PierreDiffView({
895897
}) {
896898
const [highlighted, setHighlighted] = useState<HighlightedDiffCode | null>(null);
897899
const [highlightedCacheKey, setHighlightedCacheKey] = useState<string | null>(null);
898-
const highlightedCacheRef = useRef(new Map<string, HighlightedDiffCode>());
899-
const highlightPromiseRef = useRef(new Map<string, Promise<HighlightedDiffCode>>());
900900
const appearanceCacheKey = file ? `${theme.appearance}:${file.id}` : null;
901901

902902
// Selected files load immediately; background prefetch can opt neighboring files in later.
903903
const pendingHighlight = useMemo(() => {
904-
if (!shouldLoadHighlight || !file || !appearanceCacheKey || highlightedCacheRef.current.has(appearanceCacheKey)) {
904+
if (!shouldLoadHighlight || !file || !appearanceCacheKey || SHARED_HIGHLIGHTED_DIFF_CACHE.has(appearanceCacheKey)) {
905905
return null;
906906
}
907907

908-
const existing = highlightPromiseRef.current.get(appearanceCacheKey);
908+
const existing = SHARED_HIGHLIGHT_PROMISES.get(appearanceCacheKey);
909909
if (existing) {
910910
return existing;
911911
}
912912

913913
const pending = loadHighlightedDiff(file, theme.appearance);
914-
highlightPromiseRef.current.set(appearanceCacheKey, pending);
914+
SHARED_HIGHLIGHT_PROMISES.set(appearanceCacheKey, pending);
915915
return pending;
916916
}, [appearanceCacheKey, file, shouldLoadHighlight, theme.appearance]);
917917

@@ -926,7 +926,7 @@ export function PierreDiffView({
926926
return;
927927
}
928928

929-
const cached = highlightedCacheRef.current.get(appearanceCacheKey);
929+
const cached = SHARED_HIGHLIGHTED_DIFF_CACHE.get(appearanceCacheKey);
930930
if (cached) {
931931
setHighlighted(cached);
932932
setHighlightedCacheKey(appearanceCacheKey);
@@ -946,8 +946,8 @@ export function PierreDiffView({
946946
return;
947947
}
948948

949-
highlightPromiseRef.current.delete(appearanceCacheKey);
950-
highlightedCacheRef.current.set(appearanceCacheKey, nextHighlighted);
949+
SHARED_HIGHLIGHT_PROMISES.delete(appearanceCacheKey);
950+
SHARED_HIGHLIGHTED_DIFF_CACHE.set(appearanceCacheKey, nextHighlighted);
951951
setHighlighted(nextHighlighted);
952952
setHighlightedCacheKey(appearanceCacheKey);
953953
})
@@ -956,12 +956,12 @@ export function PierreDiffView({
956956
return;
957957
}
958958

959-
highlightPromiseRef.current.delete(appearanceCacheKey);
959+
SHARED_HIGHLIGHT_PROMISES.delete(appearanceCacheKey);
960960
const fallback = {
961961
deletionLines: [],
962962
additionLines: [],
963963
} satisfies HighlightedDiffCode;
964-
highlightedCacheRef.current.set(appearanceCacheKey, fallback);
964+
SHARED_HIGHLIGHTED_DIFF_CACHE.set(appearanceCacheKey, fallback);
965965
setHighlighted(fallback);
966966
setHighlightedCacheKey(appearanceCacheKey);
967967
});
@@ -976,7 +976,7 @@ export function PierreDiffView({
976976
appearanceCacheKey && highlightedCacheKey === appearanceCacheKey
977977
? highlighted
978978
: appearanceCacheKey
979-
? (highlightedCacheRef.current.get(appearanceCacheKey) ?? null)
979+
? (SHARED_HIGHLIGHTED_DIFF_CACHE.get(appearanceCacheKey) ?? null)
980980
: null;
981981
const notifiedHighlightKeyRef = useRef<string | null>(null);
982982

‎test/app-interactions.test.tsx‎

Lines changed: 1 addition & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -122,44 +122,6 @@ function createWrapBootstrap(): AppBootstrap {
122122
};
123123
}
124124

125-
function createVisibleHighlightBootstrap(): AppBootstrap {
126-
return {
127-
input: {
128-
kind: "git",
129-
staged: false,
130-
options: {
131-
mode: "split",
132-
},
133-
},
134-
changeset: {
135-
id: "changeset:app-visible-highlight-prefetch",
136-
sourceLabel: "repo",
137-
title: "repo working tree",
138-
files: [
139-
createDiffFile(
140-
"alpha",
141-
"alpha.ts",
142-
"export const alphaMarker = 1;\nexport function alphaKeep(value: number) { return value + 1; }\n",
143-
"export const alphaMarker = 2;\nexport function alphaKeep(value: number) { return value * 2; }\n",
144-
),
145-
createDiffFile(
146-
"beta",
147-
"beta.ts",
148-
"export const betaMarker = 1;\nexport function betaKeep(value: number) { return value + 1; }\n",
149-
"export const betaMarker = 2;\nexport function betaKeep(value: number) { return value * 2; }\n",
150-
),
151-
createDiffFile(
152-
"gamma",
153-
"gamma.ts",
154-
"export const gammaMarker = 1;\nexport function gammaKeep(value: number) { return value + 1; }\n",
155-
"export const gammaMarker = 2;\nexport function gammaKeep(value: number) { return value * 2; }\n",
156-
),
157-
],
158-
},
159-
initialMode: "split",
160-
initialTheme: "midnight",
161-
};
162-
}
163125

164126
async function flush(setup: Awaited<ReturnType<typeof testRender>>) {
165127
await act(async () => {
@@ -169,20 +131,6 @@ async function flush(setup: Awaited<ReturnType<typeof testRender>>) {
169131
});
170132
}
171133

172-
function frameHasHighlightedMarker(
173-
frame: { lines: Array<{ spans: Array<{ text: string; fg?: unknown; bg?: unknown }> }> },
174-
marker: string,
175-
) {
176-
return frame.lines.some((line) => {
177-
const text = line.spans.map((span) => span.text).join("");
178-
179-
if (!text.includes(marker)) {
180-
return false;
181-
}
182-
183-
return line.spans.some((span) => span.text.includes(marker) && span.text.trim().length < text.trim().length);
184-
});
185-
}
186134

187135
describe("App interactions", () => {
188136
test("keyboard shortcuts toggle notes, line numbers, and hunk metadata", async () => {
@@ -241,7 +189,7 @@ describe("App interactions", () => {
241189
frame = setup.captureCharFrame();
242190
expect(frame).toContain("this is a very");
243191
expect(frame).toContain("long wrapped line");
244-
expect(frame).toContain("interaction coverage");
192+
expect(frame).toContain("rendering coverage");
245193
} finally {
246194
await act(async () => {
247195
setup.renderer.destroy();
@@ -346,37 +294,6 @@ describe("App interactions", () => {
346294
}
347295
});
348296

349-
test("visible files gain syntax highlighting without requiring hunk navigation", async () => {
350-
const setup = await testRender(<App bootstrap={createVisibleHighlightBootstrap()} />, { width: 240, height: 24 });
351-
352-
try {
353-
let selectedReady = false;
354-
let visibleReady = false;
355-
356-
for (let iteration = 0; iteration < 200; iteration += 1) {
357-
await act(async () => {
358-
await setup.renderOnce();
359-
await Bun.sleep(0);
360-
});
361-
362-
const frame = setup.captureSpans();
363-
selectedReady ||= frameHasHighlightedMarker(frame, "alphaMarker");
364-
visibleReady ||= frameHasHighlightedMarker(frame, "betaMarker");
365-
366-
if (selectedReady && visibleReady) {
367-
break;
368-
}
369-
}
370-
371-
expect(selectedReady).toBe(true);
372-
expect(visibleReady).toBe(true);
373-
} finally {
374-
await act(async () => {
375-
setup.renderer.destroy();
376-
});
377-
}
378-
});
379-
380297
test("quit shortcuts route through the provided onQuit handler in regular and pager modes", async () => {
381298
const regularQuit = mock(() => undefined);
382299
const regularSetup = await testRender(<App bootstrap={createBootstrap()} onQuit={regularQuit} />, { width: 220, height: 24 });

‎test/ui-components.test.tsx‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ const { DiffPane } = await import("../src/ui/components/panes/DiffPane");
1212
const { MenuDropdown } = await import("../src/ui/components/chrome/MenuDropdown");
1313
const { StatusBar } = await import("../src/ui/components/chrome/StatusBar");
1414
const { DiffSectionPlaceholder } = await import("../src/ui/components/panes/DiffSectionPlaceholder");
15+
const { PierreDiffView } = await import("../src/ui/diff/PierreDiffView");
1516

1617
function createDiffFile(id: string, path: string, before: string, after: string, withAgent = false): DiffFile {
1718
const metadata = parseDiffFromFile(
@@ -136,6 +137,21 @@ async function captureFrame(node: ReactNode, width = 120, height = 24) {
136137
}
137138
}
138139

140+
function frameHasHighlightedMarker(
141+
frame: { lines: Array<{ spans: Array<{ text: string; fg?: unknown; bg?: unknown }> }> },
142+
marker: string,
143+
) {
144+
return frame.lines.some((line) => {
145+
const text = line.spans.map((span) => span.text).join("");
146+
147+
if (!text.includes(marker)) {
148+
return false;
149+
}
150+
151+
return line.spans.some((span) => span.text.includes(marker) && span.text.trim().length < text.trim().length);
152+
});
153+
}
154+
139155
describe("UI components", () => {
140156
test("FilesPane renders file rows and diff stats", async () => {
141157
const bootstrap = createBootstrap();
@@ -470,6 +486,69 @@ describe("UI components", () => {
470486
expect(frame).toContain("1 + export const alpha = 2;");
471487
});
472488

489+
test("PierreDiffView reuses highlighted rows after unmounting and remounting a file section", async () => {
490+
const file = createDiffFile(
491+
"cache",
492+
"cache.ts",
493+
"export const cacheMarker = 1;\nexport function cacheKeep(value: number) { return value + 1; }\n",
494+
"export const cacheMarker = 2;\nexport function cacheKeep(value: number) { return value * 2; }\n",
495+
);
496+
const theme = resolveTheme("midnight", null);
497+
498+
const firstSetup = await testRender(
499+
<PierreDiffView file={file} layout="split" theme={theme} width={180} selectedHunkIndex={0} scrollable={false} />,
500+
{ width: 184, height: 10 },
501+
);
502+
503+
try {
504+
let ready = false;
505+
for (let iteration = 0; iteration < 400; iteration += 1) {
506+
await act(async () => {
507+
await firstSetup.renderOnce();
508+
await Bun.sleep(0);
509+
await firstSetup.renderOnce();
510+
await Bun.sleep(0);
511+
});
512+
513+
if (frameHasHighlightedMarker(firstSetup.captureSpans(), "cacheMarker")) {
514+
ready = true;
515+
break;
516+
}
517+
}
518+
519+
expect(ready).toBe(true);
520+
} finally {
521+
await act(async () => {
522+
firstSetup.renderer.destroy();
523+
});
524+
}
525+
526+
const secondSetup = await testRender(
527+
<PierreDiffView
528+
file={file}
529+
layout="split"
530+
theme={theme}
531+
width={180}
532+
selectedHunkIndex={0}
533+
shouldLoadHighlight={false}
534+
scrollable={false}
535+
/>,
536+
{ width: 184, height: 10 },
537+
);
538+
539+
try {
540+
await act(async () => {
541+
await secondSetup.renderOnce();
542+
});
543+
544+
expect(frameHasHighlightedMarker(secondSetup.captureSpans(), "cacheMarker")).toBe(true);
545+
} finally {
546+
await act(async () => {
547+
secondSetup.renderer.destroy();
548+
});
549+
}
550+
});
551+
473552
test("App renders the menu bar, multi-file stream, and AI badges", async () => {
474553
const bootstrap = createBootstrap();
475554
const frame = await captureFrame(<App bootstrap={bootstrap} />, 280, 24);

0 commit comments

Comments
 (0)