diff --git a/src/components/Changes.svelte b/src/components/Changes.svelte index 63963a042..44b3a644e 100644 --- a/src/components/Changes.svelte +++ b/src/components/Changes.svelte @@ -1,7 +1,7 @@ -{#if codeComments} - -
- {#each diff.files as file} -
- -
- {/each} -
-{:else} -
- {#if stickyFileIndex !== undefined} - +
+ {#if stickyFileIndex !== undefined} + + {/if} +
+ + {#snippet row(r)} + {#if r.type === "header"} + {@render header?.()} + {:else} + {/if} -
- - {#snippet row(r)} - {#if r.type === "header"} - {@render header?.()} - {:else} - - {/if} - {/snippet} - -{/if} + {/snippet} + diff --git a/src/components/Diff.svelte b/src/components/Diff.svelte deleted file mode 100644 index 923a96d75..000000000 --- a/src/components/Diff.svelte +++ /dev/null @@ -1,526 +0,0 @@ - - - - - - -{#snippet lineDiff(line: Modification, lineIdx: number, hunkIdx: number)} - {@const thread = findLineThread(line)} -
- -
{ - if (codeComments?.createComment && !thread) { - selectLine(e, file, "left", line, hunkIdx, lineIdx); - } - }}> - {lineNumber(line, "left")} -
- - -
{ - if (codeComments?.createComment && !thread) { - selectLine(e, file, "right", line, hunkIdx, lineIdx); - } - }}> - {lineNumber(line, "right")} -
- -
- {#if line.type === "addition"} - + - {:else if line.type === "deletion"} - - - {/if} -
- - {#if line.highlight && line.highlight.items.length > 0} -
- {@html line.highlight.items - .map( - paint => - `${escape(paint.item)}`, - ) - .join("")} -
- {:else if line.line !== ""} -
{line.line}
- {:else} -

- {/if} - -
- {#if thread} - {#if thread.root.resolved} - toggleCommentExpand(thread.root.id)} /> - {:else} - toggleCommentExpand(thread.root.id)} /> - {/if} - {/if} -
-
- - {#if codeComments && thread && !threadExpandedStates[thread.root.id]} -
-
- {@render commentHeader( - thread.root.location?.path, - rangeAnchorsFromCodeLocation(thread.root.location), - )} - {#if codeComments.changeCommentStatus && roles.isDelegateOrAuthor( codeComments.config.publicKey, codeComments.repoDelegates.map(delegate => delegate.did), thread.root.author.did, )} -
- {#if thread.root.resolved} -
- -
- {:else} -
- -
- {/if} -
- {/if} -
- { - await codeComments.createComment(body, embeds, thread.root.id); - } - : undefined} - editComment={codeComments.editComment} - canEditComment={partial( - roles.isDelegateOrAuthor, - codeComments.config.publicKey, - codeComments.repoDelegates.map(delegate => delegate.did), - )} - deleteComment={codeComments.deleteComment} /> -
- {/if} - - {#if codeComments && selection?.hunkIdx === hunkIdx && selection?.lineIdx === lineIdx} - -
{ - e.stopPropagation(); - }}> -
- {@render commentHeader(selection.file, { - start: selection.start, - })} -
- { - selection = undefined; - }} - focus - placeholder="Leave a comment" - disableAttachments={codeComments.disableAttachments} - submit={async (body, embeds) => { - if (selection?.codeLocation) { - try { - await codeComments.createComment( - body, - embeds, - undefined, - selection.codeLocation, - ); - } catch (e) { - console.error("Comment creation failed", e); - } finally { - selection = undefined; - } - } - }} /> -
- {/if} -{/snippet} - -{#snippet commentHeader(filePath?: string, selectionRange?: SelectionRange)} - {#if filePath && selectionRange} -
- {filePath.split("/").length > 1 ? "…/" : ""}{filePath - .split("/") - .slice(-1)}:{selectionRange.start.side === "left" - ? "L" - : "R"}{selectionRange.start.lineNumber} - {#if selectionRange.end} - -> - {selectionRange.end.side === "left" ? "L" : "R"}{selectionRange.end - .lineNumber} - {/if} -
- {/if} -{/snippet} - -
- {#if file.diff.type === "plain"} - {#each file.diff.hunks as hunk, hunkIdx} -
-
-
-
-
{hunk.header}
-
- -
- {#each hunk.lines as modification, lineIdx} - {@render lineDiff(modification, lineIdx, hunkIdx)} - {/each} -
- {/each} - {/if} -
diff --git a/src/components/DiffRow.svelte b/src/components/DiffRow.svelte index 1777ba9e3..eb7a5abe7 100644 --- a/src/components/DiffRow.svelte +++ b/src/components/DiffRow.svelte @@ -2,32 +2,36 @@ import type { Modification } from "@bindings/diff/Modification"; import escape from "lodash/escape"; + import partial from "lodash/partial"; + import type { CommentContext, SelectionRange } from "@app/lib/diffComments"; + import { + findLineThread, + lineNumber, + rangeAnchorsFromCodeLocation, + } from "@app/lib/diffComments"; import type { DiffRow } from "@app/lib/diffRows"; import { fileDiffToText } from "@app/lib/diffText"; + import * as roles from "@app/lib/roles"; + import CommentToggleInput from "@app/components/CommentToggleInput.svelte"; import CopyButton from "@app/components/CopyButton.svelte"; import Icon from "@app/components/Icon.svelte"; import Path from "@app/components/Path.svelte"; + import ThreadComponent from "@app/components/Thread.svelte"; interface Props { row: DiffRow; // Whether the row's file is expanded (only meaningful for file-header rows). expanded?: boolean; onToggleFile?: (fileIndex: number) => void; + // Present only on the patch-review path; enables inline comments/selection. + comments?: CommentContext; } - const { row, expanded = true, onToggleFile }: Props = $props(); + const { row, expanded = true, onToggleFile, comments }: Props = $props(); - function lineNumber(line: Modification, side: "left" | "right") { - if (side === "left") { - if (line.type === "context") return line.lineNoOld; - if (line.type === "deletion") return line.lineNo; - } else { - if (line.type === "context") return line.lineNoNew; - if (line.type === "addition") return line.lineNo; - } - } + type LineRow = Extract; +{#snippet code(line: Modification)} + {#if line.highlight && line.highlight.items.length > 0} +
+ {@html line.highlight.items + .map( + paint => + `${escape(paint.item)}`, + ) + .join("")} +
+ {:else if line.line !== ""} +
{line.line}
+ {:else} +

+ {/if} +{/snippet} + +{#snippet commentHeader(filePath?: string, selectionRange?: SelectionRange)} + {#if filePath && selectionRange} +
+ {filePath.split("/").length > 1 ? "…/" : ""}{filePath + .split("/") + .slice(-1)}:{selectionRange.start.side === "left" + ? "L" + : "R"}{selectionRange.start.lineNumber} + {#if selectionRange.end} + -> + {selectionRange.end.side === "left" ? "L" : "R"}{selectionRange.end + .lineNumber} + {/if} +
+ {/if} +{/snippet} + +{#snippet readonlyLine(r: LineRow)} + {@const line = r.modification} +
+
{lineNumber(line, "left") ?? ""}
+
{lineNumber(line, "right") ?? ""}
+
+ {#if line.type === "addition"}+{:else if line.type === "deletion"}-{/if} +
+ {@render code(line)} +
+{/snippet} + +{#snippet interactiveLine(r: LineRow, ctx: CommentContext)} + {@const line = r.modification} + {@const cc = ctx.codeComments} + {@const thread = findLineThread(ctx.threadByLine, r.file, line)} + {@const showThread = + thread !== undefined && !ctx.expandedStates[thread.root.id]} + {@const sel = ctx.selection} + {@const inFile = sel !== undefined && sel.fileIdx === r.fileIndex} + {@const isSel = + inFile && sel.hunkIdx === r.hunkIndex && sel.lineIdx === r.lineIndex} + {@const closeOnLine = r.last && !showThread && !isSel} + {@const closeOnThread = r.last && showThread && !isSel} + {@const closeOnForm = r.last && isSel} + {@const selectable = cc.createComment !== undefined && thread === undefined} +
+ +
{ + if (selectable) { + e.preventDefault(); + e.stopPropagation(); + ctx.onSelectLine( + r.fileIndex, + "left", + line, + r.hunkIndex, + r.lineIndex, + r.file, + ); + } + }}> + {lineNumber(line, "left") ?? ""} +
+ +
{ + if (selectable) { + e.preventDefault(); + e.stopPropagation(); + ctx.onSelectLine( + r.fileIndex, + "right", + line, + r.hunkIndex, + r.lineIndex, + r.file, + ); + } + }}> + {lineNumber(line, "right") ?? ""} +
+
+ {#if line.type === "addition"}+{:else if line.type === "deletion"}-{/if} +
+ {@render code(line)} + {#if thread} +
+ ctx.onToggleThread(thread.root.id)} /> +
+ {/if} +
+ + {#if showThread && thread} +
+
+ {@render commentHeader( + thread.root.location?.path, + rangeAnchorsFromCodeLocation(thread.root.location), + )} + {#if cc.changeCommentStatus && roles.isDelegateOrAuthor( cc.config.publicKey, cc.repoDelegates.map(delegate => delegate.did), thread.root.author.did, )} +
+ {#if thread.root.resolved} +
+ +
+ {:else} +
+ +
+ {/if} +
+ {/if} +
+ { + await cc.createComment(body, embeds, thread.root.id); + } + : undefined} + editComment={cc.editComment} + canEditComment={partial( + roles.isDelegateOrAuthor, + cc.config.publicKey, + cc.repoDelegates.map(delegate => delegate.did), + )} + deleteComment={cc.deleteComment} /> +
+ {/if} + + {#if isSel && sel} + +
{ + e.stopPropagation(); + }}> +
+ {@render commentHeader(sel.file, { start: sel.start })} +
+ ctx.onClearSelection()} + focus + placeholder="Leave a comment" + disableAttachments={cc.disableAttachments} + submit={async (body, embeds) => { + try { + await cc.createComment(body, embeds, undefined, sel.codeLocation); + } catch (e) { + console.error("Comment creation failed", e); + } finally { + ctx.onClearSelection(); + } + }} /> +
+ {/if} +{/snippet} + {#if row.type === "file-gap"}
{:else if row.type === "file-header"} @@ -193,6 +464,21 @@ -{row.file.diff.stats.deletions} {/if} + {#if comments && (row.file.status === "added" || row.file.status === "deleted" || row.file.status === "modified")} + {@const counts = comments.threadCountsByFile.get(row.file.path)} + {#if counts !== undefined && counts.unresolved > 0} +
+ + {counts.unresolved} +
+ {/if} + {#if counts !== undefined && counts.resolved > 0} +
+ + {counts.resolved} +
+ {/if} + {/if} fileDiffToText(row.file)} title="Copy file diff" /> {:else if row.type === "hunk-header"} @@ -211,32 +497,8 @@ Empty file {/if} +{:else if comments} + {@render interactiveLine(row, comments)} {:else} - {@const line = row.modification} -
-
{lineNumber(line, "left") ?? ""}
-
{lineNumber(line, "right") ?? ""}
-
- {#if line.type === "addition"}+{:else if line.type === "deletion"}-{/if} -
- {#if line.highlight && line.highlight.items.length > 0} -
- {@html line.highlight.items - .map( - paint => - `${escape(paint.item)}`, - ) - .join("")} -
- {:else if line.line !== ""} -
{line.line}
- {:else} -

- {/if} -
+ {@render readonlyLine(row)} {/if} diff --git a/src/components/FileDiff.svelte b/src/components/FileDiff.svelte deleted file mode 100644 index 78b3c400a..000000000 --- a/src/components/FileDiff.svelte +++ /dev/null @@ -1,150 +0,0 @@ - - - - -{#snippet emptyPlaceholder()} -
- Empty file -
-{/snippet} - - - {#snippet leftHeader()} - {#if file.status === "moved" || file.status === "copied"} - - - → - - - {:else} - - {/if} - - {#if file.status === "added"} - Added - {:else if file.status === "deleted"} - Deleted - {:else if file.status === "moved"} - Moved - {:else if file.status === "copied"} - Copied - {/if} - {/snippet} - - {#snippet rightHeader()} - {#if file.diff.type === "plain" && file.diff.hunks.length > 0} -
- - +{file.diff.stats.additions} - - - -{file.diff.stats.deletions} - -
- {/if} - {#if commentsOfThisFile && commentsOfThisFile.threads.length > 0} - {#if unresolvedThreads > 0} -
- - {unresolvedThreads} -
- {/if} - {#if resolvedThreads > 0} -
- - {resolvedThreads} -
- {/if} - {/if} - {/snippet} - - {#if file.diff.type === "plain"} - {#if file.diff.hunks.length > 0} - - {:else} - {@render emptyPlaceholder()} - {/if} - {:else if file.diff.type === "binary"} -
- Binary file -
- {:else} - {@render emptyPlaceholder()} - {/if} -
diff --git a/src/components/Revision.svelte b/src/components/Revision.svelte index f61639d3b..8b4902da7 100644 --- a/src/components/Revision.svelte +++ b/src/components/Revision.svelte @@ -1,5 +1,4 @@ {#if viewport} +
diff --git a/src/lib/diffComments.ts b/src/lib/diffComments.ts new file mode 100644 index 000000000..29a286647 --- /dev/null +++ b/src/lib/diffComments.ts @@ -0,0 +1,248 @@ +import type { Author } from "@bindings/cob/Author"; +import type { CodeLocation } from "@bindings/cob/thread/CodeLocation"; +import type { Embed } from "@bindings/cob/thread/Embed"; +import type { Thread } from "@bindings/cob/thread/Thread"; +import type { Config } from "@bindings/config/Config"; +import type { FileDiff } from "@bindings/diff/FileDiff"; +import type { Modification } from "@bindings/diff/Modification"; + +export interface CodeComments { + changeCommentStatus?: (commentId: string, resolved: boolean) => Promise; + config: Config; + createComment: ( + body: string, + embeds: Embed[], + replyTo?: string, + location?: CodeLocation, + ) => Promise; + editComment: ( + commentId: string, + body: string, + embeds: Embed[], + ) => Promise; + reactOnComment?: ( + commentId: string, + authors: Author[], + reaction: string, + ) => Promise; + deleteComment?: (commentId: string) => Promise; + // Defaults to `true`. + canReply?: boolean; + // See `ExtendedTextarea`. + disableAttachments?: boolean | string; + repoDelegates: Author[]; + rid: string; + threads: Thread[]; +} + +// Everything the interactive (patch-review) diff path needs, bundled so it can +// be threaded through the single changeset-level virtualizer as one prop. +export interface CommentContext { + codeComments: CodeComments; + threadByLine: Map>; + // Per-file resolved/unresolved thread tallies for the file-header badges. + threadCountsByFile: Map; + selection: Selection | undefined; + expandedStates: Record; + onSelectLine: ( + fileIdx: number, + side: Side, + line: Modification, + hunkIdx: number, + lineIdx: number, + file: FileDiff, + ) => void; + onClearSelection: () => void; + onToggleThread: (threadId: string) => void; +} + +export type Side = "left" | "right"; +export type SelectionAnchor = { side: Side; lineNumber: number }; +export type SelectionRange = { start: SelectionAnchor; end?: SelectionAnchor }; + +export interface Selection { + file: string; + // Index of the file in the changeset; disambiguates lines across files, since + // hunkIdx/lineIdx are only unique within a single file. + fileIdx: number; + start: SelectionAnchor; + end: SelectionAnchor; + lineIdx: number; + hunkIdx: number; + codeLocation: CodeLocation; +} + +export function lineNumber(line: Modification, side: Side): number | undefined { + if (side === "left") { + if (line.type === "context") { + return line.lineNoOld; + } + if (line.type === "deletion") { + return line.lineNo; + } + } else { + if (line.type === "context") { + return line.lineNoNew; + } + if (line.type === "addition") { + return line.lineNo; + } + } +} + +export function determineSelectedAnchor( + side: Side, + line: Modification, +): SelectionAnchor { + // When a user tries to select a side with no changes, use opposite side. + if (side === "left" && line.type === "addition") { + return { side: "right", lineNumber: line.lineNo }; + } else if (side === "right" && line.type === "deletion") { + return { side: "left", lineNumber: line.lineNo }; + } else { + return side === "left" + ? { side: "left", lineNumber: lineNumber(line, "left") as number } + : { side: "right", lineNumber: lineNumber(line, "right") as number }; + } +} + +export function filePath(file: FileDiff, side: Side): string { + if (file.status === "moved" || file.status === "copied") { + if (side === "left") { + return file.oldPath; + } else { + return file.newPath; + } + } else { + return file.path; + } +} + +export function buildSelection( + head: string, + file: FileDiff, + fileIdx: number, + side: Side, + line: Modification, + hunkIdx: number, + lineIdx: number, +): Selection { + const commentAnchor = determineSelectedAnchor(side, line); + + return { + file: filePath(file, side), + fileIdx, + start: commentAnchor, + end: commentAnchor, + hunkIdx, + lineIdx, + codeLocation: { + commit: head, + path: filePath(file, side), + old: + commentAnchor.side === "left" + ? { + type: "lines", + range: { + start: commentAnchor.lineNumber, + end: commentAnchor.lineNumber + 1, + }, + } + : null, + new: + commentAnchor.side === "right" + ? { + type: "lines", + range: { + start: commentAnchor.lineNumber, + end: commentAnchor.lineNumber + 1, + }, + } + : null, + }, + }; +} + +export function rangeAnchorsFromCodeLocation( + location: CodeLocation | null, +): SelectionRange | undefined { + if (location?.old?.type === "lines") { + return { + start: { side: "left", lineNumber: location.old.range.start }, + }; + } else if (location?.new?.type === "lines") { + return { + start: { side: "right", lineNumber: location.new.range.start }, + }; + } +} + +// Index threads by the file and line they anchor to, so each rendered line +// does an O(1) lookup instead of scanning every thread. Rebuilt wholesale by +// callers and only read, so a plain Map is correct here. Keys are qualified +// by the thread's file path — line numbers alone repeat across files. +export function buildThreadByLine( + threads: Thread[], +): Map> { + const map = new Map>(); + for (const t of threads) { + const path = t.root.location?.path; + if (path === undefined) { + continue; + } + const newEnd = t.root.location?.new?.range.end; + const oldEnd = t.root.location?.old?.range.end; + if (newEnd !== undefined && !map.has(`${path}:new:${newEnd}`)) { + map.set(`${path}:new:${newEnd}`, t); + } + if (oldEnd !== undefined && !map.has(`${path}:old:${oldEnd}`)) { + map.set(`${path}:old:${oldEnd}`, t); + } + } + return map; +} + +// Per-file resolved/unresolved thread tallies for the file-header badges. +// Like buildThreadByLine, rebuilt wholesale and only read. +export function buildThreadCountsByFile( + threads: Thread[], +): Map { + const counts = new Map(); + for (const t of threads) { + const path = t.root.location?.path; + if (path === undefined) { + continue; + } + const c = counts.get(path) ?? { resolved: 0, unresolved: 0 }; + if (t.root.resolved) { + c.resolved += 1; + } else { + c.unresolved += 1; + } + counts.set(path, c); + } + return counts; +} + +export function findLineThread( + map: Map>, + file: FileDiff, + line: Modification, +): Thread | undefined { + // Comment locations carry a single path, which is ambiguous for renames + // and copies; like the pre-virtualization per-file diff, those files + // don't get inline threads. + if (file.status === "moved" || file.status === "copied") { + return undefined; + } + if (line.type === "addition") { + return map.get(`${file.path}:new:${line.lineNo + 1}`); + } else if (line.type === "deletion") { + return map.get(`${file.path}:old:${line.lineNo + 1}`); + } else if (line.type === "context") { + return ( + map.get(`${file.path}:new:${line.lineNoNew + 1}`) ?? + map.get(`${file.path}:old:${line.lineNoOld + 1}`) + ); + } +}