radicle/radicle-explorer
1
0
Fork 0
mirror of https://seed.radicle.dev/rad:z4V1sjrXqjvFdnCUbxPFqd5p4DtH5.git synced 2026-10-26 07:00:38 +08:00
Code Issues Packages Projects Releases Wiki Activity

Move diff annotation helpers into lib

Browse Source 
The snippet and the code thread had their own copies.
This commit is contained in:
Rūdolfs Ošiņš
2026-10-01 18:20:24 +02:00
parent 7d2081a8aa
commit cbb21e7513
5 changed files with 202 additions and 96 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/components/PierreDiff.svelte
+10 -69
View File
@@ -32,8 +32,13 @@
} from "@app/lib/pierreComments";
import {
anchorOf,
annotationSignature,
commentedLines,
composerLines,
entryStatus,
fileAnnotations,
isCommentableStatus,
itemIds,
} from "@app/lib/pierreComments";
import { parsePatch } from "@app/lib/pierreParse";
import {
@@ -504,7 +509,7 @@
if (!instance) {
return;
}
for (const id of itemIds(parsedFiles)) {
for (const id of itemIds(parsedFiles.map(file => file.name))) {
const item = instance.getItem(id);
if (item) {
instance.updateItem({
@@ -558,18 +563,8 @@
mountedHeaders.clear();
}
// How this entry changed. `fileStatuses` is keyed by path, so the two entries
// of a type change would both read as whichever half the structured diff
// recorded last; Pierre parsed each entry on its own and knows which this is.
function statusOf(fileDiff: FileDiffMetadata): FileStatus | undefined {
const status = fileStatuses?.get(fileDiff.name);
if (status === "added" && fileDiff.type === "deleted") {
return "deleted";
}
if (status === "deleted" && fileDiff.type === "new") {
return "added";
}
return status;
return entryStatus(fileStatuses?.get(fileDiff.name), fileDiff.type);
}
// Collapse or expand a single file, by CodeView item. Bumps `version`:
@@ -986,25 +981,10 @@
};
}
// The item id for each file, in the order they were given. A type change, a
// symlink replaced by a regular file for instance, is one path arriving as two
// entries, and CodeView throws on a duplicate id, so the repeats are numbered.
// The first keeps the bare path, which is what scroll targets and collapsed
// paths resolve by. NUL is the separator because a path cannot contain one.
function itemIds(files: FileDiffMetadata[]): string[] {
// eslint-disable-next-line svelte/prefer-svelte-reactivity -- transient local map, never reactive
const occurrences = new Map<string, number>();
return files.map(file => {
const seen = occurrences.get(file.name) ?? 0;
occurrences.set(file.name, seen + 1);
return seen === 0 ? file.name : `${file.name}\u0000${seen + 1}`;
});
}
function buildItems(
files: FileDiffMetadata[],
): CodeViewItem<LineAnnotation>[] {
const ids = itemIds(files);
const ids = itemIds(files.map(file => file.name));
return files.map((fileDiff, index) => ({
id: ids[index],
type: "diff",
@@ -1046,17 +1026,6 @@
tintStyle = undefined;
}
function commentedLines(location: CodeLocation): number[] {
const range = location.new ?? location.old;
if (!range) return [];
if (range.type === "chars") return [range.line];
const lines: number[] = [];
for (let line = range.range.start; line < range.range.end; line++) {
lines.push(line);
}
return lines;
}
function tintSelectors(
side: AnnotationSide,
line: number,
@@ -1109,23 +1078,6 @@
tintStyle = style;
}
// Which slots a file's annotations occupy — and only that. Replacing a file's
// annotation array makes Pierre drop every measured annotation height in that
// file and lay it out as if the comments had no height at all, which yanks the
// scroll position; so it is only told when the set of slots really changes.
// What goes *in* a slot is pushed straight into the mounted component instead
// (see `syncAnnotationSlots`), which is all a reply, an edit or a resolve
// touches.
function annotationSignature(
annotations: DiffLineAnnotation<LineAnnotation>[] | undefined,
): string {
if (!annotations || annotations.length === 0) return "";
return annotations
.map(({ side, lineNumber }) => `${side}:${lineNumber}`)
.sort()
.join("|");
}
// Open the new-comment composer on what the gutter marker covers. Pierre
// reports the line range it was dragged over, on the side it started from.
function openComposer(
@@ -1140,18 +1092,7 @@
if (!isCommentableStatus(fileStatuses?.get(path))) {
return;
}
const side = range.side ?? "additions";
// A drag in a split diff can end on the other side, where the end line
// counts a different file. There is no range that means both, so keep the
// line it started from.
const crossSide = range.endSide !== undefined && range.endSide !== side;
const end = crossSide ? range.start : range.end;
composer = {
path,
side,
firstLine: Math.min(range.start, end),
lastLine: Math.max(range.start, end),
};
composer = { path, ...composerLines(range) };
composerBody = "";
}
@@ -1172,7 +1113,7 @@
}
untrack(() => {
let changed = false;
for (const id of itemIds(parsedFiles)) {
for (const id of itemIds(parsedFiles.map(file => file.name))) {
const item = instance.getItem(id);
if (item?.type !== "diff") {
continue;
src/components/PierreSnippet.svelte
+5 -6
View File
@@ -10,7 +10,10 @@
import type { CodeComments } from "@app/lib/codeComments";
import { forwardPatchActivityContext } from "@app/lib/patchActivityContext";
import type { LineAnnotation } from "@app/lib/pierreComments";
import { fileAnnotations } from "@app/lib/pierreComments";
import {
annotationSignature,
fileAnnotations,
} from "@app/lib/pierreComments";
import {
codeLineHeight,
getWorkerPool,
@@ -90,11 +93,7 @@
const annotationList = $derived.by(() =>
fileAnnotations(path, threads, undefined),
);
const annotationKey = $derived(
annotationList
.map(({ side, lineNumber }) => `${side}:${lineNumber}`)
.join("|"),
);
const annotationKey = $derived(annotationSignature(annotationList));
let stableAnnotations = $state.raw<DiffLineAnnotation<LineAnnotation>[]>([]);
let stableKey: string | undefined;
$effect(() => {
src/components/ReviewCodeThread.svelte
+2 -15
View File
@@ -10,6 +10,7 @@
import { diffOptions } from "@app/lib/diffOptions.svelte";
import { cachedGetDiffText } from "@app/lib/invoke";
import { sliceForRange } from "@app/lib/patchSlice";
import { lineRangeOf } from "@app/lib/pierreComments";
import PierreSnippet from "@app/components/PierreSnippet.svelte";
@@ -86,20 +87,6 @@
resolvedBy,
};
}
/// Which lines of which side a comment is about, in the numbering
/// `sliceForRange` matches on.
function rangeOf(
location: CodeLocation,
): { side: "old" | "new"; start: number; end: number } | undefined {
const range = location.new ?? location.old;
if (!range) return undefined;
const side = location.new ? "new" : "old";
if (range.type === "chars") {
return { side, start: range.line, end: range.line + 1 };
}
return { side, start: range.range.start, end: range.range.end };
}
</script>
<style>
@@ -154,7 +141,7 @@
{#each threads as thread (thread.root.id)}
{@const location = thread.root.location}
{#if location}
{@const range = rangeOf(location)}
{@const range = lineRangeOf(location)}
{@const slice =
range &&
sliceForRange(filePatch, range.side, range.start, range.end)}
src/lib/pierreComments.ts
+89 -6
View File
@@ -103,13 +103,13 @@ export function locationOf(
/// The lines a comment covers, as text: `R12` for one line on the new side,
/// `L3-L9` for a range on the old one.
export function formatAnchorLines(location: CodeLocation): string | undefined {
const range = location.new ?? location.old;
const range = lineRangeOf(location);
if (!range) return undefined;
const marker = location.new ? "R" : "L";
if (range.type === "chars") return `${marker}${range.line}`;
const start = range.range.start;
const end = range.range.end - 1;
return start >= end ? `${marker}${end}` : `${marker}${start}-${marker}${end}`;
const marker = range.side === "new" ? "R" : "L";
const end = range.end - 1;
return range.start >= end
? `${marker}${end}`
: `${marker}${range.start}-${marker}${end}`;
}
/// A comment's anchor with the file it is in, e.g. `foo.ts:R12` or
@@ -201,3 +201,86 @@ export function commentCountsByPath(
}
return counts;
}
// Which lines of which side a comment is about, end exclusive.
export function lineRangeOf(
location: CodeLocation,
): { side: "old" | "new"; start: number; end: number } | undefined {
const range = location.new ?? location.old;
if (!range) return undefined;
const side = location.new ? "new" : "old";
if (range.type === "chars") {
return { side, start: range.line, end: range.line + 1 };
}
return { side, start: range.range.start, end: range.range.end };
}
export function commentedLines(location: CodeLocation): number[] {
const range = lineRangeOf(location);
if (!range) return [];
const lines: number[] = [];
for (let line = range.start; line < range.end; line++) {
lines.push(line);
}
return lines;
}
// Which slots a file's annotations occupy, and only that. Replacing a file's
// annotation array makes Pierre drop every measured annotation height in that
// file and lay it out as if the comments had no height at all, which yanks the
// scroll position; so it is only told when the set of slots really changes.
// What goes *in* a slot is pushed straight into the mounted component instead
// (see `syncAnnotationSlots` in PierreDiff), which is all a reply, an edit or a
// resolve touches.
export function annotationSignature(
annotations: DiffLineAnnotation<LineAnnotation>[] | undefined,
): string {
return (annotations ?? [])
.map(({ side, lineNumber }) => `${side}:${lineNumber}`)
.sort()
.join("|");
}
// The lines a gutter drag covers. A drag in a split diff can end on the other
// side, where the end line counts a different file. There is no range that
// means both, so keep the line it started from.
export function composerLines(range: {
start: number;
end: number;
side?: AnnotationSide;
endSide?: AnnotationSide;
}): { side: AnnotationSide; firstLine: number; lastLine: number } {
const side = range.side ?? "additions";
const crossSide = range.endSide !== undefined && range.endSide !== side;
const end = crossSide ? range.start : range.end;
return {
side,
firstLine: Math.min(range.start, end),
lastLine: Math.max(range.start, end),
};
}
// How a diff entry changed. `fileStatuses` is keyed by path, so the two entries
// of a type change would both read as whichever half the structured diff
// recorded last; Pierre parsed each entry on its own and knows which this is.
export function entryStatus(
status: FileStatus | undefined,
entryType: string,
): FileStatus | undefined {
if (status === "added" && entryType === "deleted") return "deleted";
if (status === "deleted" && entryType === "new") return "added";
return status;
}
// An id per file, in order. A type change is one path arriving as two entries,
// and CodeView throws on a duplicate id, so the repeats are numbered. The
// first keeps the bare path, which is what scroll targets and collapsed paths
// resolve by. NUL is the separator because a path cannot contain one.
export function itemIds(names: string[]): string[] {
const occurrences = new Map<string, number>();
return names.map(name => {
const seen = occurrences.get(name) ?? 0;
occurrences.set(name, seen + 1);
return seen === 0 ? name : `${name}\u0000${seen + 1}`;
});
}
tests/unit/pierreComments.test.ts
+96
View File
@@ -6,11 +6,17 @@ import { describe, expect, test } from "vitest";
import type { ComposerTarget } from "@app/lib/pierreComments";
import {
anchorOf,
annotationSignature,
commentCountsByPath,
commentedLines,
composerLines,
entryStatus,
fileAnnotations,
formatAnchor,
formatAnchorLines,
isCommentableStatus,
itemIds,
lineRangeOf,
locationOf,
} from "@app/lib/pierreComments";
@@ -258,3 +264,93 @@ describe("commentCountsByPath", () => {
expect(counts.get("a.ts")).toEqual({ resolved: 0, unresolved: 1 });
});
});
describe("line ranges", () => {
test("lineRangeOf prefers the new side and ends after the last line", () => {
expect(
lineRangeOf(at("a.ts", { old: lines(1, 2), new: lines(3, 5) })),
).toEqual({ side: "new", start: 3, end: 5 });
expect(lineRangeOf(at("a.ts", { old: lines(1, 2) }))).toEqual({
side: "old",
start: 1,
end: 2,
});
expect(
lineRangeOf(
at("a.ts", {
new: { type: "chars", line: 7, range: { start: 0, end: 3 } },
}),
),
).toEqual({ side: "new", start: 7, end: 8 });
expect(lineRangeOf(at("a.ts", {}))).toBeUndefined();
});
test("commentedLines lists each line of the range", () => {
expect(commentedLines(at("a.ts", { new: lines(3, 6) }))).toEqual([3, 4, 5]);
expect(commentedLines(at("a.ts", {}))).toEqual([]);
});
});
test("annotationSignature only depends on which slots are taken", () => {
const a = { side: "additions" as const, lineNumber: 1, metadata: {} };
const b = { side: "deletions" as const, lineNumber: 2, metadata: {} };
expect(annotationSignature([b, a] as never)).toBe(
annotationSignature([a, b] as never),
);
expect(annotationSignature([a] as never)).not.toBe(
annotationSignature([b] as never),
);
expect(annotationSignature(undefined)).toBe("");
});
describe("composerLines", () => {
test("orders a range dragged upwards", () => {
expect(composerLines({ start: 9, end: 4, side: "deletions" })).toEqual({
side: "deletions",
firstLine: 4,
lastLine: 9,
});
});
test("keeps the starting line of a drag that ends on the other side", () => {
expect(
composerLines({
start: 3,
end: 8,
side: "additions",
endSide: "deletions",
}),
).toEqual({ side: "additions", firstLine: 3, lastLine: 3 });
});
test("defaults to the new side", () => {
expect(composerLines({ start: 2, end: 5, endSide: "additions" })).toEqual({
side: "additions",
firstLine: 2,
lastLine: 5,
});
});
});
test.each([
["added", "deleted", "deleted"],
["deleted", "new", "added"],
["added", "new", "added"],
["modified", "change", "modified"],
[undefined, "new", undefined],
] as const)(
"entryStatus of a %j path's %j entry is %j",
(status, type, expected) => {
expect(entryStatus(status, type)).toBe(expected);
},
);
test("itemIds numbers repeated paths, keeping the first bare", () => {
expect(itemIds(["a", "b", "a", "a"])).toEqual([
"a",
"b",
"a\u00002",
"a\u00003",
]);
});