diff --git a/packages/panel/src/diff-view/contract.ts b/packages/panel/src/diff-view/contract.ts index ad8af37c54..f70af12766 100644 --- a/packages/panel/src/diff-view/contract.ts +++ b/packages/panel/src/diff-view/contract.ts @@ -130,6 +130,8 @@ export interface DiffListFile { readonly lead: boolean; /** Whether the lines between the file's hunks may be opened, its text read whole the first time one is (`DiffListProps.readTexts`). */ readonly expandable: boolean; + /** The file's whole text before and after, where it is drawn whole, every unchanged line among its changes; none draws its hunks. */ + readonly whole?: DiffFileTexts | undefined; } /** Lines lit in one file of a list: the file, by its path, and the lines. */ diff --git a/packages/panel/src/diff-view/index.ts b/packages/panel/src/diff-view/index.ts index fa261b0a7a..a48fc8ef12 100644 --- a/packages/panel/src/diff-view/index.ts +++ b/packages/panel/src/diff-view/index.ts @@ -36,6 +36,7 @@ import { import { type Highlighting, startHighlighting } from "./highlight-pool.js"; import { adoptShadowStyles } from "./inline-styles.js"; import { pressedOnCode, selectedLines, selectionRect } from "./text-selection.js"; +import { wholeOf } from "./whole-diff.js"; /** * index.ts -- the diff bundle: a patch drawn by Pierre's diffs as every diff setting says, in the panel's colours. @@ -694,16 +695,22 @@ function mountList(host: HTMLElement, props: DiffListProps): DiffListHandle { held.file.lead === file.lead && held.file.collapsed === file.collapsed && held.file.expandable === file.expandable && + held.file.whole === file.whole && sameNotes(held.file.annotations, file.annotations) ) { return held.item; } const read = - held?.file.patch === file.patch && held.file.lead === file.lead ? held.read : patchOf(file); + held?.file.patch === file.patch && + held.file.lead === file.lead && + held.file.whole === undefined + ? held.read + : patchOf(file); const item: CodeViewDiffItem = { id: file.path, type: "diff", - fileDiff: drawnAs(read, file.expandable), + fileDiff: + file.whole === undefined ? drawnAs(read, file.expandable) : wholeOf(read, file.whole), collapsed: file.collapsed, annotations: annotationsOf(file.annotations), version: (held?.item.version ?? 0) + 1, diff --git a/packages/panel/src/diff-view/whole-diff.test.ts b/packages/panel/src/diff-view/whole-diff.test.ts new file mode 100644 index 0000000000..ec2d726709 --- /dev/null +++ b/packages/panel/src/diff-view/whole-diff.test.ts @@ -0,0 +1,55 @@ +/** + * whole-diff.test.ts -- a file drawn whole is one hunk holding every line, + * its changes among them, and a file that only moved, its texts alike, is + * every one of its lines as context rather than no hunk at all. Synthetic + * files only. + */ + +import assert from "node:assert/strict"; +import { parsePatchFiles } from "@pierre/diffs"; +import { test } from "vitest"; +import { wholeOf } from "./whole-diff.js"; + +/** A patch as the list reads it, of one file named as given. */ +function read(patch: string) { + const file = parsePatchFiles(patch)[0]?.files[0]; + return file ?? assert.fail("the patch reads"); +} + +test("a changed file drawn whole is one hunk of every line, its change among them", () => { + const drawn = wholeOf( + read("diff --git a/a.ts b/a.ts\n--- a/a.ts\n+++ b/a.ts\n@@ -2,1 +2,1 @@\n-b\n+B\n"), + { before: "a\nb\nc\n", after: "a\nB\nc\n" }, + ); + assert.equal(drawn.hunks.length, 1); + assert.equal(drawn.hunks[0]?.additionStart, 1); + assert.equal(drawn.hunks[0]?.additionCount, 3); +}); + +test("a file that only moved, drawn whole, is every one of its lines as context, numbered alike on both sides", () => { + const moved = read( + "diff --git a/old.ts b/new.ts\nsimilarity index 100%\nrename from old.ts\nrename to new.ts\n", + ); + const texts = { before: "a\nb\nc\n", after: "a\nb\nc\n" }; + const drawn = wholeOf(moved, texts); + + assert.equal(drawn.hunks.length, 1); + const hunk = drawn.hunks[0] ?? assert.fail(); + assert.deepEqual( + [hunk.additionStart, hunk.additionCount, hunk.deletionStart, hunk.deletionCount], + [1, 3, 1, 3], + ); + assert.deepEqual(hunk.hunkContent, [ + { type: "context", lines: 3, additionLineIndex: 0, deletionLineIndex: 0 }, + ]); + assert.equal(drawn.unifiedLineCount, 3); + assert.equal(drawn.prevName, "old.ts"); + assert.equal(wholeOf(moved, texts), drawn, "made once for its texts"); +}); + +test("a file of no line that only moved stays without a hunk", () => { + const moved = read( + "diff --git a/old.ts b/new.ts\nsimilarity index 100%\nrename from old.ts\nrename to new.ts\n", + ); + assert.equal(wholeOf(moved, { before: "", after: "" }).hunks.length, 0); +}); diff --git a/packages/panel/src/diff-view/whole-diff.ts b/packages/panel/src/diff-view/whole-diff.ts new file mode 100644 index 0000000000..09af7cd399 --- /dev/null +++ b/packages/panel/src/diff-view/whole-diff.ts @@ -0,0 +1,60 @@ +/** + * whole-diff.ts -- a Files changed file drawn whole, as stage-cli's Expand full file draws one: its texts before and after diffed as one hunk holding every line. + * + * Pierre diffs two texts to the hunks around their changes; with every + * line kept as context those hunks are one, the whole file. Two texts alike, + * a file that only moved, diff to no hunk at all, so that file's lines are + * laid out here as one hunk of context, numbered alike on both sides, and + * a file of no line stays as it was parsed. + */ + +import { type FileDiffMetadata, type Hunk, parseDiffFromFile } from "@pierre/diffs"; +import type { DiffFileTexts } from "./contract.js"; + +/** + * A file drawn whole: its texts before and after diffed with every line + * kept as context, so its changes stand among all of its unchanged lines + * in one hunk, as a file opened in full; named as its patch names it. + */ +export function wholeOf(read: FileDiffMetadata, whole: DiffFileTexts): FileDiffMetadata { + const kept = WHOLE_DIFFS.get(whole); + if (kept !== undefined) return kept; + const parsed = parseDiffFromFile( + { name: read.prevName ?? read.name, contents: whole.before }, + { name: read.name, contents: whole.after }, + { context: Number.MAX_SAFE_INTEGER }, + ); + // Note that texts alike, a file that only moved, diff to no hunk at all, so its lines are laid out as one of context. + const drawn = parsed.hunks.length === 0 ? unchangedWhole(parsed) : parsed; + WHOLE_DIFFS.set(whole, drawn); + return drawn; +} + +/** A file whose texts are alike drawn whole: every one of its lines in one hunk of context, numbered alike on both sides. */ +function unchangedWhole(parsed: FileDiffMetadata): FileDiffMetadata { + const lines = parsed.additionLines.length; + if (lines === 0) return parsed; + const hunk: Hunk = { + collapsedBefore: 0, + additionStart: 1, + additionCount: lines, + additionLines: 0, + additionLineIndex: 0, + deletionStart: 1, + deletionCount: lines, + deletionLines: 0, + deletionLineIndex: 0, + hunkContent: [{ type: "context", lines, additionLineIndex: 0, deletionLineIndex: 0 }], + hunkSpecs: `@@ -1,${lines} +1,${lines} @@\n`, + splitLineStart: 0, + splitLineCount: lines, + unifiedLineStart: 0, + unifiedLineCount: lines, + noEOFCRDeletions: false, + noEOFCRAdditions: false, + }; + return { ...parsed, hunks: [hunk], splitLineCount: lines, unifiedLineCount: lines }; +} + +/** Each whole file's diff, made once for its texts, so a file drawn again for its notes keeps the diff Pierre drew. */ +const WHOLE_DIFFS = new WeakMap(); diff --git a/packages/panel/src/tasks/agent-changes-tab.test.ts b/packages/panel/src/tasks/agent-changes-tab.test.ts index 0a96a55855..9f707d88cc 100644 --- a/packages/panel/src/tasks/agent-changes-tab.test.ts +++ b/packages/panel/src/tasks/agent-changes-tab.test.ts @@ -2019,3 +2019,167 @@ test("a file read whole that is not the contents GitHub listed its patch against assert.deepEqual(await readTexts("src/seats.ts"), { before: BASE_TEXT, after: HEAD_TEXT }); assert.equal(await readTexts("src/moved.ts"), undefined); }); + +test("a file that only moved, shown whole, draws its review threads on their lines, and one of no line keeps saying it moved", async () => { + commits = { headSha: "a".repeat(40) }; + files = [ + changed("src/moved.ts", { + status: CHANGED_FILE_STATUS.RENAMED, + previousPath: "src/old.ts", + patch: "", + }), + changed("src/empty.ts", { + status: CHANGED_FILE_STATUS.RENAMED, + previousPath: "src/was-empty.ts", + patch: "", + }), + ]; + answerFileReads((path) => ({ + text: { + path, + repository: "acme/relay", + branch: "a".repeat(40), + text: path === "src/empty.ts" ? "" : HEAD_TEXT, + }, + })); + const threads: ChangesThreads = { + list: [{ ...seatsThread(2, "on-2"), path: "src/moved.ts" }], + draw: (thread) => createElement("p", null, thread.comments[0]?.body), + }; + await drawTab({ threads }); + const drawnAs = (path: string) => lastDrawn().files.find((file) => file.path === path); + assert.deepEqual(drawnAs("src/moved.ts")?.annotations, [ + { key: "lead", side: "additions", line: 0 }, + ]); + + for (const path of ["src/moved.ts", "src/empty.ts"]) { + const lead = lastDrawn().renderAnnotation(path, "lead"); + document.body.append(lead); + await flush(); + const show = lead.querySelector(`button[aria-label="Show file: ${path}"]`); + act(() => show?.click()); + await flush(); + } + + assert.deepEqual(drawnAs("src/moved.ts")?.annotations, [ + { key: `${PAGE}#on-2`, side: "additions", line: 2 }, + ]); + assert.equal(drawnAs("src/empty.ts")?.lead, true, "a file of no line says it moved still"); + assert.equal( + lastDrawn() + .renderAnnotation("src/empty.ts", "lead") + .querySelector('button[aria-label^="Show file"]'), + null, + "with nothing more to show", + ); +}); + +test("a whole file read for an earlier answer, landing after a refresh, leaves the files expanded since as they are", async () => { + commits = { headSha: "a".repeat(40) }; + files = [ + changed("src/seats.ts", { patch: HEAD_PATCH }), + changed("src/plan.ts", { patch: HEAD_PATCH }), + ]; + const held: Array<() => void> = []; + fakeAppBridge({ + act: (request: Act) => { + const value = + request.kind === ACT_KIND.TASKS_FILE + ? { + text: { + path: request.payload.path, + repository: "acme/relay", + branch: "a".repeat(40), + text: HEAD_TEXT, + }, + } + : undefined; + const outcome: ActOutcome = { status: ACT_OUTCOME_STATUS.DONE, value }; + // The first file's read is held until the test lets it land. + if (request.kind === ACT_KIND.TASKS_FILE && request.payload.path === "src/seats.ts") { + return new Promise((resolve) => held.push(() => resolve(outcome))); + } + return Promise.resolve(outcome); + }, + }); + handSettings(); + await drawTab(); + const expand = (path: string) => + [...header(path).querySelectorAll("button")].find( + (button) => button.getAttribute("aria-label") === "Expand full file", + ); + + act(() => expand("src/seats.ts")?.click()); + await flush(); + const options = document.querySelector('button[aria-label="Changes actions"]'); + act(() => options?.click()); + const refresh = [...document.querySelectorAll('[role="menuitem"]')].find( + (each) => each.textContent?.includes("Refresh changes"), + ); + act(() => refresh?.click()); + await flush(); + act(() => expand("src/plan.ts")?.click()); + await flush(); + assert.notEqual(lastDrawn().files.find((file) => file.path === "src/plan.ts")?.whole, undefined); + + for (const land of held) land(); + await flush(); + + const whole = (path: string) => lastDrawn().files.find((file) => file.path === path)?.whole; + assert.notEqual(whole("src/plan.ts"), undefined, "the file expanded since stays whole"); + assert.equal(whole("src/seats.ts"), undefined, "and the earlier answer's read is dropped"); +}); + +test("a file's header draws the whole file on Expand full file, read once, and back to its changes; a file that only moved shows its lines, from Show file, rather than saying so", async () => { + commits = { headSha: "a".repeat(40) }; + files = [ + changed("src/seats.ts", { patch: HEAD_PATCH }), + changed("src/moved.ts", { + status: CHANGED_FILE_STATUS.RENAMED, + previousPath: "src/old.ts", + patch: "", + }), + changed("src/new.ts", { status: CHANGED_FILE_STATUS.ADDED }), + ]; + const asked = answerFileReads((path) => ({ + text: { path, repository: "acme/relay", branch: "a".repeat(40), text: HEAD_TEXT }, + })); + await drawTab(); + const toggle = (path: string) => + [...header(path).querySelectorAll("button")].find((button) => + ["Expand full file", "Collapse to changes"].includes(button.getAttribute("aria-label") ?? ""), + ); + assert.equal(toggle("src/new.ts"), undefined, "an added file has nothing to expand"); + const drawnAs = (path: string) => lastDrawn().files.find((file) => file.path === path); + assert.equal(drawnAs("src/moved.ts")?.lead, true, "a file that only moved says so"); + + act(() => toggle("src/seats.ts")?.click()); + await flush(); + assert.deepEqual(drawnAs("src/seats.ts")?.whole, { before: BASE_TEXT, after: HEAD_TEXT }); + const lead = lastDrawn().renderAnnotation("src/moved.ts", "lead"); + document.body.append(lead); + await flush(); + const showFile = lead.querySelector( + 'button[aria-label="Show file: src/moved.ts"]', + ); + assert.ok(showFile, "what a file that only moved says offers its lines"); + act(() => showFile.click()); + await flush(); + assert.deepEqual(drawnAs("src/moved.ts")?.whole, { before: HEAD_TEXT, after: HEAD_TEXT }); + assert.equal(drawnAs("src/moved.ts")?.lead, false, "and once whole, shows its lines"); + + act(() => toggle("src/seats.ts")?.click()); + await flush(); + assert.equal( + drawnAs("src/seats.ts")?.whole, + undefined, + "Collapse to changes draws its hunks again", + ); + act(() => toggle("src/seats.ts")?.click()); + await flush(); + assert.equal( + asked.filter((request) => request.kind === ACT_KIND.TASKS_FILE).length, + 2, + "each file is read once", + ); +}); diff --git a/packages/panel/src/tasks/agent-changes-tab.tsx b/packages/panel/src/tasks/agent-changes-tab.tsx index 5f45f95061..9da8853138 100644 --- a/packages/panel/src/tasks/agent-changes-tab.tsx +++ b/packages/panel/src/tasks/agent-changes-tab.tsx @@ -15,6 +15,7 @@ import type { CatalogModel } from "@luke/features/models-wire"; import { TASK_FILE_REFUSAL, type TaskFileRefusal } from "@luke/features/task-wire"; import { ACT_KIND } from "@luke/host/acts"; import { + BUTTON_SIZE, Button, Checkbox, CopyIcon, @@ -26,6 +27,8 @@ import { HoverCardHead, HoverCardNote, IconButton, + TEXT_BUTTON_TONE, + TextButton, } from "@luke/ui"; import { fileName } from "@luke/voice/code-pane-model"; import { @@ -34,8 +37,10 @@ import { ClipboardCopyIcon, CodeIcon, FileDiffIcon, + FoldVerticalIcon, ImageIcon, RotateCwIcon, + UnfoldVerticalIcon, } from "lucide-react"; import { type ReactNode, useCallback, useEffect, useMemo, useRef, useState } from "react"; import { useAct } from "../act.js"; @@ -83,6 +88,7 @@ import { textsOf, threadsOn, treeOrder, + wholeable, } from "./changes-model.js"; import { ChangesStrip } from "./changes-strip.js"; import { agentStillWriting, pullRequestNumberLabel } from "./coding-agent-model.js"; @@ -486,6 +492,8 @@ function entryOf( /** Which of the file's lines show their threads while Minimize inline comments is on, and the door that shows or hides one; none while it is off. */ minimized: FileMinimized | undefined, hover: ThreadHover | undefined, + /** The file drawn whole, where it is, whether it may be, and the door that draws it so or back to its changes. */ + whole: FileWhole, ): DiffListEntry { const path = file.path; const image = imageTypeOfPath(path) === undefined || images === undefined ? undefined : images; @@ -494,7 +502,9 @@ function entryOf( const pictured = image !== undefined && !source && imageDrawable(file, image) ? image : undefined; // Note that a pictured file draws no lines, so its threads stand above it, as on a binary file. const drawnAs = pictured === undefined ? file : { ...file, patch: null }; - const { placed, above } = threadsOn(threads, drawnAs); + // A file drawn whole draws every one of its lines, so its threads are placed on them. + const drawnWhole = pictured === undefined && !emptyWhole(whole.texts) ? whole.texts : undefined; + const { placed, above } = threadsOn(threads, drawnAs, drawnWhole); const notes: DiffNote[] = minimized === undefined ? placed.map((each) => ({ @@ -524,12 +534,30 @@ function entryOf( ); }; // A pictured image, or a file with no hunks, stands as what is drawn in the place of its lines. - const notice = fileNotice(file); + // A file that only moved, drawn whole, shows its lines rather than saying it only moved. + const notice = drawnWhole === undefined ? fileNotice(file) : undefined; + // A file that only moved offers its lines from what it says, its read waiting for the press. + const shown = + notice === CHANGES_WORDS.UNCHANGED_CONTENTS && whole.offered && whole.texts === undefined ? ( + <> + {" ยท "} + + {LIST_WORDS.SHOW_FILE} + + + ) : null; const lead = pictured !== undefined ? ( ) : notice === undefined ? undefined : ( -

{notice}

+

+ {notice} + {shown} +

); if (lead !== undefined) { notes.unshift({ @@ -570,6 +598,7 @@ function entryOf( collapsed, lead: lead !== undefined, expandable: lead === undefined && gaps.open, + whole: lead === undefined ? whole.texts : undefined, header: ( list.toggleSource(path)} /> )} + {!whole.offered || pictured !== undefined ? null : ( + + )} {list.viewed === undefined || list.markViewed === undefined ? null : ( list.markViewed?.(path)} /> )} @@ -644,6 +676,7 @@ function useEntries( texts: FileTexts, minimizedOf: (path: string) => FileMinimized | undefined, hover: ThreadHover | undefined, + wholeOf: (file: CodingAgentChangedFile) => FileWhole, ): readonly DiffListEntry[] { const kept = useRef(new Map()); const next = new Map(); @@ -652,18 +685,22 @@ function useEntries( const comments = commenting?.notes.get(file.path) ?? NO_NOTES; const gaps = { open: texts.expandable(file), refused: texts.refused.get(file.path) }; const minimized = minimizedOf(file.path); + const whole = wholeOf(file); const inputs = [ ...entryInputs(file, list, own, threads?.draw, images, comments), gaps.open, gaps.refused, minimized, own.length === 0 ? undefined : hover, + whole.texts, + whole.offered, + whole.toggle, ]; const was = kept.current.get(file.path); const entry = was !== undefined && sameInputs(was.inputs, inputs) ? was.entry - : entryOf(file, list, own, threads?.draw, images, comments, gaps, minimized, hover); + : entryOf(file, list, own, threads?.draw, images, comments, gaps, minimized, hover, whole); next.set(file.path, { inputs, entry }); return entry; }); @@ -671,6 +708,69 @@ function useEntries( return entries; } +/** A file drawn whole: its texts where it is, whether it may be, and the door that draws it whole or back to its changes. */ +interface FileWhole { + readonly texts: DiffFileTexts | undefined; + readonly offered: boolean; + readonly toggle: () => void; +} + +/** Whether a file drawn whole has no line to draw, which keeps saying what it is rather than drawing nothing. */ +function emptyWhole(texts: DiffFileTexts | undefined): boolean { + return texts !== undefined && texts.before === "" && texts.after === ""; +} + +/** No file drawn whole. */ +const NONE_WHOLE: ReadonlyMap = new Map(); + +/** + * Which files are drawn whole, as stage-cli's Expand full file draws one: + * a file's texts read as its gaps read them (`FileTexts`'s door, once and + * kept), every unchanged line then drawn among its changes, until Collapse + * to changes; every file back to its changes with a new answer. A file whose + * gaps never open is never offered, and one whose read is refused stays as + * it is, its refusal said as a gap's is. + */ +function useWholeFiles(texts: FileTexts): (file: CodingAgentChangedFile) => FileWhole { + const { read, refused } = texts; + const [held, setHeld] = useState<{ + readonly of: unknown; + readonly files: ReadonlyMap; + }>({ of: read, files: NONE_WHOLE }); + const files = held.of === read ? held.files : NONE_WHOLE; + const latest = useRef({ read, files }); + latest.current = { read, files }; + const toggle = useCallback((path: string) => { + const { read, files } = latest.current; + const held = (next: ReadonlyMap) => setHeld({ of: read, files: next }); + if (files.has(path)) { + const next = new Map(files); + next.delete(path); + held(next); + return; + } + read?.(path).then( + (whole) => { + // Note that a read of an earlier answer is dropped, which would draw that answer's files over this one's. + if (whole !== undefined && latest.current.read === read) { + held(new Map(latest.current.files).set(path, whole)); + } + }, + () => undefined, + ); + }, []); + const doors = useRef(new Map void>()); + return (file) => { + const door = doors.current.get(file.path) ?? (() => toggle(file.path)); + doors.current.set(file.path, door); + return { + texts: files.get(file.path), + offered: read !== undefined && wholeable(file) && !refused.has(file.path), + toggle: door, + }; + }; +} + /** The lines that show their threads while Minimize inline comments is on: by file, by side, by line. */ type ShownLines = ReadonlyMap>>; @@ -904,6 +1004,28 @@ function ViewedBox({ ); } +/** A file's turn between its changes alone and the whole file, at the end of its header, as stage-cli's is. */ +function WholeToggle({ + whole, + onToggle, +}: { + whole: boolean; + onToggle: () => void; +}): React.JSX.Element { + const label = whole ? LIST_WORDS.COLLAPSE_TO_CHANGES : LIST_WORDS.EXPAND_WHOLE; + return ( + + : } + label={label} + size={BUTTON_SIZE.COMPACT} + pressed={whole} + onClick={onToggle} + /> + + ); +} + /** An SVG's turn between its picture and its source's lines, at the end of its header. */ function SourceToggle({ source, @@ -1011,7 +1133,7 @@ function useFileTexts( })); return (path: string) => { const file = files.find((each) => each.path === path); - if (file === undefined || !expandable(file)) return Promise.resolve(undefined); + if (file === undefined || !wholeable(file)) return Promise.resolve(undefined); const held = kept.get(path); if (held !== undefined) return held; const reading = act(ACT_KIND.TASKS_FILE, { @@ -1117,8 +1239,18 @@ function FilesList({ const look = useDiffLook(roomy); const { settings } = useFileViewSettings(); const minimizedOf = useMinimizedLines(settings.diffMinimizeComments); + const wholeOf = useWholeFiles(texts); const resting = useThreadHover(); - const entries = useEntries(list, threads, images, commenting, texts, minimizedOf, resting.hover); + const entries = useEntries( + list, + threads, + images, + commenting, + texts, + minimizedOf, + resting.hover, + wholeOf, + ); const query = list.query.trim(); return (
diff --git a/packages/panel/src/tasks/changes-list.tsx b/packages/panel/src/tasks/changes-list.tsx index 53ef7a8059..ea4358b270 100644 --- a/packages/panel/src/tasks/changes-list.tsx +++ b/packages/panel/src/tasks/changes-list.tsx @@ -83,6 +83,10 @@ export const LIST_WORDS = { EXPAND_ALL: "Expand all files", SHOW_SOURCE: "Show source", SHOW_IMAGE: "Show image", + EXPAND_WHOLE: "Expand full file", + COLLAPSE_TO_CHANGES: "Collapse to changes", + /** A file that only moved, drawn as its lines. */ + SHOW_FILE: "Show file", VIEWED: "Viewed", MARK_VIEWED: "Mark as viewed", UNMARK_VIEWED: "Mark as not viewed", diff --git a/packages/panel/src/tasks/changes-model.ts b/packages/panel/src/tasks/changes-model.ts index 16217a102b..54e6d9490d 100644 --- a/packages/panel/src/tasks/changes-model.ts +++ b/packages/panel/src/tasks/changes-model.ts @@ -398,6 +398,22 @@ function drawnLines(file: CodingAgentChangedFile): DrawnLines { return lines; } +/** Each whole file's lines, counted once for its texts. */ +const WHOLE_LINES = new WeakMap(); + +/** The lines a file drawn whole draws a row for: every line of each side's text. */ +function wholeLines(whole: DiffFileTexts): DrawnLines { + const kept = WHOLE_LINES.get(whole); + if (kept !== undefined) return kept; + const numbered = (text: string) => new Set(linesOf(text).lines.map((_line, index) => index + 1)); + const lines = { + [DIFF_SIDE.LEFT]: numbered(whole.before), + [DIFF_SIDE.RIGHT]: numbered(whole.after), + }; + WHOLE_LINES.set(whole, lines); + return lines; +} + /** The lines a file's diff draws a row for, read off its patch. */ function linesDrawnBy(file: CodingAgentChangedFile): DrawnLines { const patch = fileNotice(file) === undefined ? file.patch : null; @@ -419,18 +435,22 @@ function linesDrawnBy(file: CodingAgentChangedFile): DrawnLines { * at the top of the diff, in the order the pull request holds them, those * it draws none for: a thread whose line a later push took away, outdated; * one on a line no hunk shows; and every one on a file whose diff is not - * drawn, binary, too large, or empty. Each is keyed by its first comment's - * page, which no other thread shares; one with no comment is drawn nowhere. + * drawn, binary, too large, or empty. A file drawn whole draws a row for + * every one of its lines, so its threads stand on any of them. Each is keyed + * by its first comment's page, which no other thread shares; one with no + * comment is drawn nowhere. */ export function threadsOn( threads: readonly ReviewThread[], file: CodingAgentChangedFile, + /** The file's texts where it is drawn whole; none where it draws its patch. */ + whole?: DiffFileTexts | undefined, ): FileThreads { const placed: PlacedThread[] = []; const above: PlacedThread[] = []; // Note that a file no thread is on reads none of its lines, which would parse its patch. if (!threads.some((thread) => thread.path === file.path)) return { placed, above }; - const lines = drawnLines(file); + const lines = whole === undefined ? drawnLines(file) : wholeLines(whole); for (const thread of threads) { const key = thread.comments[0]?.url; if (thread.path !== file.path || key === undefined) continue; @@ -762,6 +782,16 @@ export function expandable(file: CodingAgentChangedFile): boolean { return changedInPlace && fileNotice(file) === undefined && !isSecretFile(file.path); } +/** Whether a file only moved, its contents as they were: a rename whose patch holds no hunk. */ +function movedOnly(file: CodingAgentChangedFile): boolean { + return file.status === CHANGED_FILE_STATUS.RENAMED && file.patch?.trim() === ""; +} + +/** Whether a file can be drawn whole: one whose gaps open (`expandable`), or one that only moved, all of whose lines stand unchanged; never a secret one. */ +export function wholeable(file: CodingAgentChangedFile): boolean { + return expandable(file) || (movedOnly(file) && !isSecretFile(file.path)); +} + /** * A changed file's whole text before and after, from its text after alone * and its patch: every line no hunk holds is the same on both sides, so the @@ -773,6 +803,8 @@ export function expandable(file: CodingAgentChangedFile): boolean { * whose lines between hunks are not drawn (`expandable`). */ export function textsOf(file: CodingAgentChangedFile, after: string): DiffFileTexts | undefined { + // A file that only moved stood before as it stands now. + if (movedOnly(file) && wholeable(file)) return { before: after, after }; const patch = file.patch; if (!expandable(file) || patch === null) return undefined; const hunks = rawHunks(patch); diff --git a/packages/panel/src/tasks/diff-list.tsx b/packages/panel/src/tasks/diff-list.tsx index 109f4e71aa..6de88e446c 100644 --- a/packages/panel/src/tasks/diff-list.tsx +++ b/packages/panel/src/tasks/diff-list.tsx @@ -62,6 +62,8 @@ export interface DiffListEntry { readonly lead: boolean; /** Whether the lines between its hunks may be opened. */ readonly expandable: boolean; + /** Its whole text before and after, where it is drawn whole. */ + readonly whole?: DiffFileTexts | undefined; readonly header: ReactNode; readonly notes: readonly DiffNote[]; } @@ -167,6 +169,7 @@ function listFileOf(entry: DiffListEntry): DiffListFile { collapsed: entry.collapsed, lead: entry.lead, expandable: entry.expandable, + whole: entry.whole, annotations: entry.notes.map(placeOf), }; LIST_FILES.set(entry, file); @@ -187,6 +190,7 @@ function sameFiles(a: readonly DiffListFile[], b: readonly DiffListFile[]): bool file.collapsed === other.collapsed && file.lead === other.lead && file.expandable === other.expandable && + file.whole === other.whole && sameNotes(file.annotations, other.annotations)) ); })