From 4c221d9f9a8a0431d430fdac4e46611894d8c3c1 Mon Sep 17 00:00:00 2001 From: Seongmin Lee Date: Tue, 4 Aug 2026 10:10:14 +0300 Subject: [PATCH] web: PR view Signed-off-by: Seongmin Lee --- web/src/lib/actions/resizable.ts | 69 +++ web/src/lib/api/gitmirror.test.ts | 134 +++++ web/src/lib/api/gitmirror.ts | 86 +++ web/src/lib/api/knotmirror.ts | 22 + web/src/lib/api/pull.ts | 110 ++++ web/src/lib/api/pullCompose.test.ts | 141 +++++ web/src/lib/api/pullCompose.ts | 69 +++ web/src/lib/api/pullDiff.test.ts | 173 ++++++ web/src/lib/api/pullDiff.ts | 166 ++++++ web/src/lib/api/pullPage.ts | 136 +++++ web/src/lib/api/pullRoute.test.ts | 164 ++++++ web/src/lib/api/pullRoute.ts | 177 ++++++ web/src/lib/api/records.ts | 36 ++ web/src/lib/api/repo.ts | 2 + .../components/comment/CommentEditor.svelte | 4 + web/src/lib/components/comment/comments.ts | 2 + .../repo/pulls/PullActions.stories.svelte | 19 + .../components/repo/pulls/PullActions.svelte | 293 ++++++++++ .../lib/components/repo/pulls/PullCard.svelte | 27 + .../repo/pulls/PullCardContent.svelte | 46 ++ .../repo/pulls/PullComment.stories.svelte | 49 ++ .../components/repo/pulls/PullComment.svelte | 177 ++++++ .../repo/pulls/PullCommitList.stories.svelte | 45 ++ .../repo/pulls/PullCommitList.svelte | 83 +++ .../components/repo/pulls/PullCompose.svelte | 550 ++++++++++++++++++ .../repo/pulls/PullDiffFiles.svelte | 114 ++++ .../repo/pulls/PullDiffPanel.stories.svelte | 50 ++ .../repo/pulls/PullDiffPanel.svelte | 272 +++++++++ .../repo/pulls/PullDiscussion.stories.svelte | 85 +++ .../repo/pulls/PullDiscussion.svelte | 436 ++++++++++++++ .../components/repo/pulls/PullFileTree.svelte | 95 +++ .../repo/pulls/PullInfoBar.stories.svelte | 36 ++ .../components/repo/pulls/PullInfoBar.svelte | 79 +++ .../lib/components/repo/pulls/PullList.svelte | 23 + .../repo/pulls/PullMetaPanel.svelte | 190 ++++++ .../components/repo/pulls/PullSearch.svelte | 42 ++ .../repo/pulls/PullStatePill.stories.svelte | 23 + .../repo/pulls/PullStatePill.svelte | 47 ++ .../components/repo/pulls/PullToolbar.svelte | 79 +++ web/src/lib/components/repo/types.ts | 11 + web/src/lib/oauth-client-metadata.json | 2 +- .../routes/[handle]/[repo]/+layout@.svelte | 16 +- .../routes/[handle]/[repo]/pulls/+page.svelte | 22 + web/src/routes/[handle]/[repo]/pulls/+page.ts | 67 +++ .../[repo]/pulls/[aturi]/+page.svelte | 1 + .../[handle]/[repo]/pulls/[aturi]/+page.ts | 12 + .../[aturi]/[version]/[[range]]/+page.svelte | 298 ++++++++++ .../[aturi]/[version]/[[range]]/+page.ts | 127 ++++ .../[handle]/[repo]/pulls/new/+page.server.ts | 6 + .../[handle]/[repo]/pulls/new/+page.svelte | 29 + .../routes/[handle]/[repo]/pulls/new/+page.ts | 137 +++++ 51 files changed, 5073 insertions(+), 6 deletions(-) create mode 100644 web/src/lib/actions/resizable.ts create mode 100644 web/src/lib/api/gitmirror.test.ts create mode 100644 web/src/lib/api/gitmirror.ts create mode 100644 web/src/lib/api/pull.ts create mode 100644 web/src/lib/api/pullCompose.test.ts create mode 100644 web/src/lib/api/pullCompose.ts create mode 100644 web/src/lib/api/pullDiff.test.ts create mode 100644 web/src/lib/api/pullDiff.ts create mode 100644 web/src/lib/api/pullPage.ts create mode 100644 web/src/lib/api/pullRoute.test.ts create mode 100644 web/src/lib/api/pullRoute.ts create mode 100644 web/src/lib/components/repo/pulls/PullActions.stories.svelte create mode 100644 web/src/lib/components/repo/pulls/PullActions.svelte create mode 100644 web/src/lib/components/repo/pulls/PullCard.svelte create mode 100644 web/src/lib/components/repo/pulls/PullCardContent.svelte create mode 100644 web/src/lib/components/repo/pulls/PullComment.stories.svelte create mode 100644 web/src/lib/components/repo/pulls/PullComment.svelte create mode 100644 web/src/lib/components/repo/pulls/PullCommitList.stories.svelte create mode 100644 web/src/lib/components/repo/pulls/PullCommitList.svelte create mode 100644 web/src/lib/components/repo/pulls/PullCompose.svelte create mode 100644 web/src/lib/components/repo/pulls/PullDiffFiles.svelte create mode 100644 web/src/lib/components/repo/pulls/PullDiffPanel.stories.svelte create mode 100644 web/src/lib/components/repo/pulls/PullDiffPanel.svelte create mode 100644 web/src/lib/components/repo/pulls/PullDiscussion.stories.svelte create mode 100644 web/src/lib/components/repo/pulls/PullDiscussion.svelte create mode 100644 web/src/lib/components/repo/pulls/PullFileTree.svelte create mode 100644 web/src/lib/components/repo/pulls/PullInfoBar.stories.svelte create mode 100644 web/src/lib/components/repo/pulls/PullInfoBar.svelte create mode 100644 web/src/lib/components/repo/pulls/PullList.svelte create mode 100644 web/src/lib/components/repo/pulls/PullMetaPanel.svelte create mode 100644 web/src/lib/components/repo/pulls/PullSearch.svelte create mode 100644 web/src/lib/components/repo/pulls/PullStatePill.stories.svelte create mode 100644 web/src/lib/components/repo/pulls/PullStatePill.svelte create mode 100644 web/src/lib/components/repo/pulls/PullToolbar.svelte create mode 100644 web/src/routes/[handle]/[repo]/pulls/+page.svelte create mode 100644 web/src/routes/[handle]/[repo]/pulls/+page.ts create mode 100644 web/src/routes/[handle]/[repo]/pulls/[aturi]/+page.svelte create mode 100644 web/src/routes/[handle]/[repo]/pulls/[aturi]/+page.ts create mode 100644 web/src/routes/[handle]/[repo]/pulls/[aturi]/[version]/[[range]]/+page.svelte create mode 100644 web/src/routes/[handle]/[repo]/pulls/[aturi]/[version]/[[range]]/+page.ts create mode 100644 web/src/routes/[handle]/[repo]/pulls/new/+page.server.ts create mode 100644 web/src/routes/[handle]/[repo]/pulls/new/+page.svelte create mode 100644 web/src/routes/[handle]/[repo]/pulls/new/+page.ts diff --git a/web/src/lib/actions/resizable.ts b/web/src/lib/actions/resizable.ts new file mode 100644 index 00000000..f4fc4f25 --- /dev/null +++ b/web/src/lib/actions/resizable.ts @@ -0,0 +1,69 @@ +import type { Action } from "svelte/action"; + +// port of the appview's `fragments/resizable` ResizablePanel: a grip that drags +// a sibling panel's width, and resets it on double click +export interface ResizableOptions { + // the panel this grip sizes; looked up lazily so it can mount after the grip + target: () => HTMLElement | null | undefined; + // `before` when the panel sits left of the grip, `after` when it sits right + direction?: "before" | "after"; + min?: number; + max?: number; +} + +export const resizable: Action = (node, options) => { + let current = options; + let startX = 0; + let startWidth = 0; + let panel: HTMLElement | null | undefined; + + const move = (event: MouseEvent) => { + if (!panel) return; + const delta = current.direction === "after" ? startX - event.clientX : event.clientX - startX; + const width = startWidth + delta; + if (width < (current.min ?? 100) || width > (current.max ?? Infinity)) return; + panel.style.width = `${width}px`; + panel.style.flexShrink = "0"; + }; + + const up = () => { + node.classList.remove("resizing"); + document.body.style.cursor = ""; + document.body.style.userSelect = ""; + document.removeEventListener("mousemove", move); + document.removeEventListener("mouseup", up); + }; + + const down = (event: MouseEvent) => { + panel = current.target(); + if (!panel) return; + event.preventDefault(); + node.classList.add("resizing"); + document.body.style.cursor = "col-resize"; + document.body.style.userSelect = "none"; + startX = event.clientX; + startWidth = panel.offsetWidth; + document.addEventListener("mousemove", move); + document.addEventListener("mouseup", up); + }; + + const reset = (event: MouseEvent) => { + const element = current.target(); + if (!element) return; + event.preventDefault(); + element.style.width = ""; + element.style.flexShrink = ""; + }; + + node.addEventListener("mousedown", down); + node.addEventListener("dblclick", reset); + + return { + update: (next: ResizableOptions) => (current = next), + destroy: () => { + node.removeEventListener("mousedown", down); + node.removeEventListener("dblclick", reset); + up(); + } + }; +}; diff --git a/web/src/lib/api/gitmirror.test.ts b/web/src/lib/api/gitmirror.test.ts new file mode 100644 index 00000000..43e8db44 --- /dev/null +++ b/web/src/lib/api/gitmirror.test.ts @@ -0,0 +1,134 @@ +import { describe, expect, it, vi } from "vitest"; +import { createBobbinClient } from "./client"; +import { + diffFileKind, + diffFileName, + diffFileStat, + listCommits, + toMirrorCommitSummary, + type MirrorCommit, + type MirrorFileDiff +} from "./gitmirror"; + +const signature = (when: string) => ({ name: "alice", email: "alice@example.com", when }); + +const commit = (oid: string, extra: Partial = {}): MirrorCommit => ({ + oid, + parents: [], + tree: "tree", + author: signature("2025-09-22T10:40:35+09:00"), + committer: signature("2025-09-22T10:44:00+09:00"), + message: "subject", + extraHeaders: [], + ...extra +}); + +describe("gitmirror.listCommits", () => { + it("queries the temp2 nsid with the repo did", async () => { + const fetchMock = vi.fn().mockResolvedValue( + new Response(JSON.stringify({ commits: [commit("abc")] }), { + status: 200, + headers: { "content-type": "application/json" } + }) + ); + const ctx = createBobbinClient({ serviceUrl: "https://bobbin.test", fetch: fetchMock }); + + const page = await listCommits(ctx, { + repo: "did:plc:repo" as never, + ranges: ["base..head"], + limit: 2 + }); + + const url = new URL(String(fetchMock.mock.calls[0][0])); + expect(url.pathname).toBe("/xrpc/sh.tangled.git.temp2.listCommits"); + // ranges is an array param, so it repeats rather than joining + expect(url.searchParams.getAll("ranges")).toEqual(["base..head"]); + expect(Object.fromEntries(url.searchParams)).toMatchObject({ + repo: "did:plc:repo", + limit: "2" + }); + expect(page.commits).toHaveLength(1); + }); +}); + +describe("gitmirror.toMirrorCommitSummary", () => { + it("splits the message and picks up the change id", () => { + const summary = toMirrorCommitSummary( + commit("9a925efef6a0e6dfcd2d4317b4a1eee8752928b8", { + message: "pulls: log the range\n\nthe lexicon has no range param", + extraHeaders: [ + { key: "gpgsig", value: "..." }, + { key: "change-id", value: "kmzwvxqouwxnvzsnvtwyprxlpuoymqrt" } + ] + }) + ); + + expect(summary).toEqual({ + hash: "9a925efef6a0e6dfcd2d4317b4a1eee8752928b8", + shortHash: "9a925efe", + subject: "pulls: log the range", + body: "the lexicon has no range param", + authorName: "alice", + authorEmail: "alice@example.com", + // the committer date wins, like the other commit converters + when: "2025-09-22T10:44:00+09:00", + changeId: "kmzwvxqouwxnvzsnvtwyprxlpuoymqrt" + }); + }); +}); + +const src = (path: string, extra: Partial = {}) => ({ + path, + oid: "oid", + size: 10, + isBinary: false, + isSubmodule: false, + ...extra +}); + +const fileDiff = ( + lhs: string, + rhs: string, + hunks: MirrorFileDiff["hunks"] = [], + extra: Partial = {} +): MirrorFileDiff => ({ + lhsSrc: src(lhs), + rhsSrc: src(rhs), + hunks, + hasSyntacticChanges: false, + ...extra +}); + +describe("gitmirror diff mappers", () => { + it("names a file by its surviving side", () => { + expect(diffFileName(fileDiff("a.ts", "a.ts"))).toBe("a.ts"); + expect(diffFileName(fileDiff("gone.ts", ""))).toBe("gone.ts"); + expect(diffFileName(fileDiff("", "added.ts"))).toBe("added.ts"); + }); + + it("reads the change kind off the two paths", () => { + expect(diffFileKind(fileDiff("a.ts", "a.ts"))).toBe("changed"); + expect(diffFileKind(fileDiff("", "a.ts"))).toBe("new"); + expect(diffFileKind(fileDiff("a.ts", ""))).toBe("deleted"); + expect(diffFileKind(fileDiff("a.ts", "b.ts"))).toBe("renamed"); + }); + + it("counts line pairs: one side is pure, both sides is a replacement", () => { + const hunks = [ + { + novelLhs: [3], + novelRhs: [3, 4], + // line 3 replaced, line 4 added + lines: [{ lhs: 3, rhs: 3 }, { rhs: 4 }] + }, + { + novelLhs: [9], + novelRhs: [], + // line 9 deleted + lines: [{ lhs: 9 }] + } + ]; + expect(diffFileStat(fileDiff("a.ts", "a.ts", hunks))).toEqual({ insertions: 2, deletions: 2 }); + expect(diffFileStat(fileDiff("a.ts", "a.ts"))).toEqual({ insertions: 0, deletions: 0 }); + }); +}); diff --git a/web/src/lib/api/gitmirror.ts b/web/src/lib/api/gitmirror.ts new file mode 100644 index 00000000..67a278d1 --- /dev/null +++ b/web/src/lib/api/gitmirror.ts @@ -0,0 +1,86 @@ +import { jsonGet } from "./_request"; +import { splitMessage, type CommitSummary } from "./repo"; +import type { BobbinContext, XrpcRequestInit } from "./client"; +import type * as ShTangledGitDefs from "./lexicons/types/sh/tangled/git/defs"; +import type * as GetDiff from "./lexicons/types/sh/tangled/git/temp2/getDiff"; +import type * as GetInterdiff from "./lexicons/types/sh/tangled/git/temp2/getInterdiff"; +import type * as ListCommits from "./lexicons/types/sh/tangled/git/temp2/listCommits"; +import type * as MergeCheck from "./lexicons/types/sh/tangled/git/temp2/mergeCheck"; + +// gitmirror's own xrpc surface, proxied through bobbin — its grpc service is +// internal and streaming, so the read paths the frontend needs live here +const LIST_COMMITS_NSID = "sh.tangled.git.temp2.listCommits"; +const GET_DIFF_NSID = "sh.tangled.git.temp2.getDiff"; +const GET_INTERDIFF_NSID = "sh.tangled.git.temp2.getInterdiff"; +const MERGE_CHECK_NSID = "sh.tangled.git.temp2.mergeCheck"; + +export type MirrorCommit = ListCommits.Commit; + +export const listCommits = ( + ctx: BobbinContext, + params: ListCommits.$params, + init?: XrpcRequestInit +) => jsonGet(ctx, LIST_COMMITS_NSID, params, init); + +export type MirrorFileDiff = ShTangledGitDefs.FileDiff; + +export const getDiff = (ctx: BobbinContext, params: GetDiff.$params, init?: XrpcRequestInit) => + jsonGet(ctx, GET_DIFF_NSID, params, init); + +// `base*` is the older version's span, `head*` the newer one's: gitmirror +// rebases base1..base2 onto head1 and diffs the result against head2 +export const getInterdiff = ( + ctx: BobbinContext, + params: GetInterdiff.$params, + init?: XrpcRequestInit +) => jsonGet(ctx, GET_INTERDIFF_NSID, params, init); + +export type MirrorConflict = MergeCheck.Conflict; + +export const mergeCheck = ( + ctx: BobbinContext, + params: MergeCheck.$params, + init?: XrpcRequestInit +) => jsonGet(ctx, MERGE_CHECK_NSID, params, init); + +export type MirrorFileKind = "new" | "deleted" | "renamed" | "changed"; + +// a side with no path is a side that doesn't exist: the file was added or deleted +export const diffFileName = (file: MirrorFileDiff): string => file.rhsSrc.path || file.lhsSrc.path; + +export const diffFileKind = (file: MirrorFileDiff): MirrorFileKind => { + if (!file.lhsSrc.path) return "new"; + if (!file.rhsSrc.path) return "deleted"; + return file.lhsSrc.path === file.rhsSrc.path ? "changed" : "renamed"; +}; + +// the payload has no line text, only aligned line numbers: a pair with one side +// missing is a pure insertion or deletion, a pair with both is a replaced line +export const diffFileStat = (file: MirrorFileDiff): { insertions: number; deletions: number } => { + let insertions = 0; + let deletions = 0; + for (const hunk of file.hunks) { + for (const line of hunk.lines) { + if (line.rhs !== undefined) insertions++; + if (line.lhs !== undefined) deletions++; + } + } + return { insertions, deletions }; +}; + +export const toMirrorCommitSummary = (commit: MirrorCommit): CommitSummary => { + const [subject, body] = splitMessage(commit.message); + return { + hash: commit.oid, + shortHash: commit.oid.slice(0, 8), + subject, + body, + authorName: commit.author.name, + authorEmail: commit.author.email, + when: commit.committer.when ?? commit.author.when, + // the pull page links a commit row as `firstParent..commit` + parent: commit.parents[0], + // jj stashes the change id in a commit header, same lookup the appview does + changeId: commit.extraHeaders.find((header) => header.key === "change-id")?.value + }; +}; diff --git a/web/src/lib/api/knotmirror.ts b/web/src/lib/api/knotmirror.ts index d59472cc..10691656 100644 --- a/web/src/lib/api/knotmirror.ts +++ b/web/src/lib/api/knotmirror.ts @@ -15,6 +15,8 @@ const BRANCHES_NSID = "sh.tangled.git.temp.listBranches"; const TAGS_NSID = "sh.tangled.git.temp.listTags"; const LANGUAGES_NSID = "sh.tangled.git.temp.listLanguages"; const GET_TAG_NSID = "sh.tangled.git.temp.getTag"; +const MERGE_BASE_NSID = "sh.tangled.git.temp.getMergeBase"; +const GET_BRANCH_NSID = "sh.tangled.git.temp.getBranch"; const MAX_README_BYTES = 1 << 20; @@ -196,3 +198,23 @@ export const languages = ( params: { repo: string; ref: string }, init?: XrpcRequestInit ) => jsonGet(ctx, LANGUAGES_NSID, params, init); + +export const getMergeBase = ( + ctx: BobbinContext, + params: { repo: string; base: string; head: string }, + init?: XrpcRequestInit +) => jsonGet<{ commit: string }>(ctx, MERGE_BASE_NSID, params, init); + +export interface KnotMirrorBranch { + name: string; + hash: string; + when: string; + message?: string; + author?: { name: string; email: string; when: string }; +} + +export const getBranch = ( + ctx: BobbinContext, + params: { repo: string; name: string }, + init?: XrpcRequestInit +) => jsonGet(ctx, GET_BRANCH_NSID, params, init); diff --git a/web/src/lib/api/pull.ts b/web/src/lib/api/pull.ts new file mode 100644 index 00000000..cf847fc0 --- /dev/null +++ b/web/src/lib/api/pull.ts @@ -0,0 +1,110 @@ +import { ok } from "@atcute/client"; +import { mainSchema as putRecordSchema } from "@atcute/atproto/types/repo/putRecord"; +import { mainSchema as deleteRecordSchema } from "@atcute/atproto/types/repo/deleteRecord"; +import type { Nsid, RecordKey } from "@atcute/lexicons/syntax"; +import type { OAuthUserAgent } from "@atcute/oauth-browser-client"; +import { now as tidNow } from "@atcute/tid"; +import { createClient, mintServiceAuth, serviceDidForHost } from "$lib/auth/agent"; +import { toResponseError } from "./_request"; +import type { PullRecord, PullState, PullStatusRecord, RecordView } from "./records"; + +const PULL_COLLECTION = "sh.tangled.repo.pull" as Nsid; +const PULL_STATUS_COLLECTION = "sh.tangled.repo.pull.status" as Nsid; + +export const putPull = async ( + agent: OAuthUserAgent, + rkey: string, + record: PullRecord +): Promise> => { + const rpc = createClient(agent); + const { uri, cid } = await ok( + rpc.call(putRecordSchema, { + input: { + repo: agent.sub, + collection: PULL_COLLECTION, + rkey: rkey as RecordKey, + record + } + }) + ); + return { uri, cid, value: record }; +}; + +export const deletePull = async (agent: OAuthUserAgent, rkey: string): Promise => { + const rpc = createClient(agent); + await ok( + rpc.call(deleteRecordSchema, { + input: { repo: agent.sub, collection: PULL_COLLECTION, rkey: rkey as RecordKey } + }) + ); +}; + +/** + * Build the status record for a pull transition. Statuses are append-only: every transition is + * a fresh record at a new tid, and readers take the newest one they trust. + */ +export const pullStatusRecord = (pull: string, status: PullState): PullStatusRecord => ({ + $type: "sh.tangled.repo.pull.status", + pull: pull as PullStatusRecord["pull"], + status: `sh.tangled.repo.pull.status.${status}`, + createdAt: new Date().toISOString() as PullStatusRecord["createdAt"] +}); + +/** Writes into the acting user's repo, not the pull author's. */ +export const putPullStatus = async ( + agent: OAuthUserAgent, + pull: string, + status: PullState +): Promise> => { + const rpc = createClient(agent); + const record = pullStatusRecord(pull, status); + const { uri, cid } = await ok( + rpc.call(putRecordSchema, { + input: { + repo: agent.sub, + collection: PULL_STATUS_COLLECTION, + rkey: tidNow() as RecordKey, + record + } + }) + ); + return { uri, cid, value: record }; +}; + +const KEEP_COMMIT_NSID = "sh.tangled.git.keepCommit"; + +export interface KeepCommitInput { + /** repo did the commit lives in */ + repo: string; + /** the commit to pin */ + oid: string; + /** at-uri of the record that owns the keep ref; the rkey has to be final */ + record: string; +} + +export const keepCommit = async ( + agent: OAuthUserAgent, + knot: string, + input: KeepCommitInput, + fetch: typeof globalThis.fetch = globalThis.fetch +): Promise<{ commit: string }> => { + const token = await mintServiceAuth(agent, { + aud: serviceDidForHost(knot), + lxm: KEEP_COMMIT_NSID + }); + const response = await fetch(`https://${knot}/xrpc/${KEEP_COMMIT_NSID}`, { + method: "POST", + headers: { + "content-type": "application/json", + accept: "application/json", + authorization: `Bearer ${token}` + }, + body: JSON.stringify({ + repo: input.repo, + record: input.record, + source: { $type: `${KEEP_COMMIT_NSID}#commit`, repo: input.repo, oid: input.oid } + }) + }); + if (!response.ok) throw await toResponseError(response); + return (await response.json()) as { commit: string }; +}; diff --git a/web/src/lib/api/pullCompose.test.ts b/web/src/lib/api/pullCompose.test.ts new file mode 100644 index 00000000..ff25ca49 --- /dev/null +++ b/web/src/lib/api/pullCompose.test.ts @@ -0,0 +1,141 @@ +import { describe, expect, it } from "vitest"; +import { + composeQuery, + defaultSourceBranch, + defaultTargetBranch, + parseSource, + sortBranchesByRecency, + sourceBranchChoices +} from "./pullCompose"; +import type { BranchEntry } from "./repo"; + +const branch = (name: string, when?: string, isDefault = false): BranchEntry => ({ + reference: { name, hash: `hash-${name}` }, + commit: when ? { Committer: { Name: "n", Email: "e", When: when } } : undefined, + is_default: isDefault +}); + +// main is the default; feat is the newest of the rest +const branches = [ + branch("main", "2024-01-01T00:00:00Z", true), + branch("old", "2024-01-02T00:00:00Z"), + branch("feat", "2024-03-01T00:00:00Z") +]; + +const names = (list: BranchEntry[]) => list.map((entry) => entry.reference.name); + +describe("sortBranchesByRecency", () => { + it("puts the newest commit first", () => { + expect(names(sortBranchesByRecency(branches))).toEqual(["feat", "old", "main"]); + }); + + it("sinks branches with no commit without reordering them", () => { + const list = [ + branch("empty-a"), + branch("dated", "2024-01-01T00:00:00Z"), + branch("empty-b") + ]; + expect(names(sortBranchesByRecency(list))).toEqual(["dated", "empty-a", "empty-b"]); + }); +}); + +describe("sourceBranchChoices", () => { + it("drops the default branch and sorts the rest", () => { + expect(names(sourceBranchChoices(branches))).toEqual(["feat", "old"]); + }); +}); + +describe("defaultTargetBranch", () => { + it("keeps a value that names a real branch", () => { + expect(defaultTargetBranch(branches, "old")).toBe("old"); + }); + + it("falls back to the default branch when the value is unknown or absent", () => { + expect(defaultTargetBranch(branches, "gone")).toBe("main"); + expect(defaultTargetBranch(branches, "")).toBe("main"); + }); + + it("gives up when no branch is marked default", () => { + expect(defaultTargetBranch([branch("a", "2024-01-01T00:00:00Z")], "")).toBe(""); + }); +}); + +describe("defaultSourceBranch", () => { + const choices = sourceBranchChoices(branches); + const forkBranches = [ + branch("fork-old", "2024-01-01T00:00:00Z"), + branch("fork-new", "2024-05-01T00:00:00Z") + ]; + + it("picks the most recent non-default branch", () => { + expect(defaultSourceBranch("branch", "", choices, [])).toBe("feat"); + }); + + it("replaces a value that is not a candidate", () => { + // main is the default branch, so it never appears in the source list + expect(defaultSourceBranch("branch", "main", choices, [])).toBe("feat"); + }); + + it("keeps a value that is a candidate", () => { + expect(defaultSourceBranch("branch", "old", choices, [])).toBe("old"); + }); + + it("reads the fork's branches in fork mode", () => { + expect(defaultSourceBranch("fork", "", choices, forkBranches)).toBe("fork-old"); + expect(defaultSourceBranch("fork", "fork-new", choices, forkBranches)).toBe("fork-new"); + // a branch of the target repo is not a candidate against a fork + expect(defaultSourceBranch("fork", "feat", choices, forkBranches)).toBe("fork-old"); + }); + + it("leaves the value alone in patch mode", () => { + expect(defaultSourceBranch("patch", "whatever", choices, forkBranches)).toBe("whatever"); + }); + + it("returns nothing when there are no candidates", () => { + expect(defaultSourceBranch("branch", "feat", [], [])).toBe(""); + }); +}); + +describe("parseSource", () => { + it("accepts the three modes case-insensitively", () => { + expect(parseSource("branch")).toBe("branch"); + expect(parseSource("FORK")).toBe("fork"); + expect(parseSource("patch")).toBe("patch"); + }); + + it("rejects anything else", () => { + expect(parseSource("")).toBeNull(); + expect(parseSource(null)).toBeNull(); + expect(parseSource("stack")).toBeNull(); + }); +}); + +describe("composeQuery", () => { + it("omits the default source and a fork picked outside fork mode", () => { + expect( + composeQuery({ + source: "branch", + sourceBranch: "feat", + targetBranch: "main", + fork: "did:x" + }) + ).toBe("sourceBranch=feat&targetBranch=main"); + }); + + it("carries the fork only in fork mode", () => { + expect( + composeQuery({ + source: "fork", + sourceBranch: "feat", + targetBranch: "main", + fork: "did:x" + }) + ).toBe("source=fork&sourceBranch=feat&targetBranch=main&fork=did%3Ax"); + }); + + it("is empty when nothing is selected", () => { + expect( + composeQuery({ source: "branch", sourceBranch: "", targetBranch: "", fork: "" }) + ).toBe(""); + }); +}); diff --git a/web/src/lib/api/pullCompose.ts b/web/src/lib/api/pullCompose.ts new file mode 100644 index 00000000..74cef72f --- /dev/null +++ b/web/src/lib/api/pullCompose.ts @@ -0,0 +1,69 @@ +import type { BranchEntry } from "./repo"; + +export type PullSource = "patch" | "branch" | "fork"; + +const SOURCES: PullSource[] = ["patch", "branch", "fork"]; + +export const parseSource = (raw: string | null | undefined): PullSource | null => { + const value = raw?.toLowerCase() as PullSource | undefined; + return value && SOURCES.includes(value) ? value : null; +}; + +const branchTime = (branch: BranchEntry): number => { + const when = branch.commit?.Committer?.When; + return when ? Date.parse(when) : NaN; +}; + +/** newest commit first; branches with no commit sort last, order otherwise kept */ +export const sortBranchesByRecency = (branches: readonly BranchEntry[]): BranchEntry[] => + branches + .map((branch, index) => ({ branch, index, time: branchTime(branch) })) + .sort((a, b) => { + const aMissing = Number.isNaN(a.time); + const bMissing = Number.isNaN(b.time); + if (aMissing || bMissing) + return aMissing === bMissing ? a.index - b.index : aMissing ? 1 : -1; + return b.time - a.time || a.index - b.index; + }) + .map((entry) => entry.branch); + +/** you can't open a PR from the default branch, so it never appears as a source */ +export const sourceBranchChoices = (branches: readonly BranchEntry[]): BranchEntry[] => + sortBranchesByRecency(branches.filter((branch) => !branch.is_default)); + +const named = (branches: readonly BranchEntry[], name: string): boolean => + branches.some((branch) => branch.reference.name === name); + +export const defaultTargetBranch = (branches: readonly BranchEntry[], current: string): string => { + if (current && named(branches, current)) return current; + return branches.find((branch) => branch.is_default)?.reference.name ?? ""; +}; + +export const defaultSourceBranch = ( + source: PullSource, + current: string, + branchChoices: readonly BranchEntry[], + forkBranches: readonly BranchEntry[] +): string => { + if (source === "patch") return current; + const candidates = source === "fork" ? forkBranches : branchChoices; + if (current && named(candidates, current)) return current; + return candidates[0]?.reference.name ?? ""; +}; + +export interface ComposeSelection { + source: PullSource; + sourceBranch: string; + targetBranch: string; + /** repo did of the chosen fork */ + fork: string; +} + +export const composeQuery = (selection: ComposeSelection): string => { + const query = new URLSearchParams(); + if (selection.source !== "branch") query.set("source", selection.source); + if (selection.sourceBranch) query.set("sourceBranch", selection.sourceBranch); + if (selection.targetBranch) query.set("targetBranch", selection.targetBranch); + if (selection.source === "fork" && selection.fork) query.set("fork", selection.fork); + return query.toString(); +}; diff --git a/web/src/lib/api/pullDiff.test.ts b/web/src/lib/api/pullDiff.test.ts new file mode 100644 index 00000000..7a913821 --- /dev/null +++ b/web/src/lib/api/pullDiff.test.ts @@ -0,0 +1,173 @@ +import { describe, expect, it, vi } from "vitest"; +import { createBobbinClient } from "./client"; +import { gitTarget } from "./gitclient"; +import { loadPullDiff, loadPullInterdiff } from "./pullDiff"; +import type { MirrorFileDiff } from "./gitmirror"; + +const src = (path: string, extra: Partial = {}) => ({ + path, + oid: `oid-${path}`, + size: 10, + isBinary: false, + isSubmodule: false, + ...extra +}); + +const json = (body: unknown) => + new Response(JSON.stringify(body), { + status: 200, + headers: { "content-type": "application/json" } + }); + +const params = { + baseRepo: "did:plc:repo" as never, + baseCommit: "base", + headRepo: "did:plc:repo" as never, + headCommit: "head" +}; + +// contents keyed by `:`, so each side can differ +const harness = (diffs: MirrorFileDiff[], contents: Record) => { + const calls: string[] = []; + const fetchMock = vi.fn().mockImplementation(async (input) => { + const url = new URL(String(input)); + calls.push(url.pathname); + if (url.pathname.endsWith("temp2.getDiff") || url.pathname.endsWith("temp2.getInterdiff")) { + return json({ diffs }); + } + const key = `${url.searchParams.get("ref")}:${url.searchParams.get("path")}`; + const content = contents[key]; + if (content === undefined) return new Response("nope", { status: 404 }); + return json({ path: url.searchParams.get("path"), content, encoding: "utf-8", size: 1 }); + }); + + const ctx = createBobbinClient({ serviceUrl: "https://bobbin.test", fetch: fetchMock }); + const git = gitTarget( + { bobbinUrl: "https://bobbin.test", knotMirrorUrl: "" } as never, + { uri: "at://did:plc:alice/sh.tangled.repo/core", repoDid: "did:plc:repo" }, + fetchMock + ); + return { ctx, git, calls }; +}; + +describe("loadPullDiff", () => { + it("pairs each file's two sides and sums the stat", async () => { + const { ctx, git, calls } = harness( + [ + { + lhsSrc: src("a.ts"), + rhsSrc: src("a.ts"), + hunks: [{ novelLhs: [0], novelRhs: [0, 1], lines: [{ lhs: 0, rhs: 0 }, { rhs: 1 }] }], + hasSyntacticChanges: false + }, + { + lhsSrc: src(""), + rhsSrc: src("new.ts"), + hunks: [{ novelLhs: [], novelRhs: [0], lines: [{ rhs: 0 }] }], + hasSyntacticChanges: false + } + ], + { "base:a.ts": "one\n", "head:a.ts": "two\nthree\n", "head:new.ts": "hello\n" } + ); + + const diff = await loadPullDiff(ctx, git, params, "unified"); + + expect(diff.stat).toEqual({ insertions: 3, deletions: 1, files_changed: 2 }); + expect(diff.truncated).toBe(0); + expect(diff.files[0]).toMatchObject({ + name: "a.ts", + kind: "changed", + oldFile: { name: "a.ts", contents: "one\n" }, + newFile: { name: "a.ts", contents: "two\nthree\n" } + }); + // a file with no left side gets an empty left side, not a failed fetch + expect(diff.files[1]).toMatchObject({ + name: "new.ts", + kind: "new", + oldFile: { contents: "" }, + newFile: { contents: "hello\n" } + }); + expect(calls.filter((path) => path.endsWith("repo.blob"))).toHaveLength(3); + // the server prerenders each pair so the first paint ships highlighted + expect(diff.prerendered?.["a.ts→a.ts"]).toContain("three"); + }); + + it("notes the files it cannot render instead of dropping them", async () => { + const { ctx, git } = harness( + [ + { + lhsSrc: src("logo.png", { isBinary: true }), + rhsSrc: src("logo.png", { isBinary: true }), + hunks: [], + hasSyntacticChanges: false + }, + { + lhsSrc: src("vendor", { isSubmodule: true }), + rhsSrc: src("vendor", { isSubmodule: true }), + hunks: [], + hasSyntacticChanges: false + }, + { + lhsSrc: src("gone.ts"), + rhsSrc: src("gone.ts"), + hunks: [], + hasSyntacticChanges: false + } + ], + {} + ); + + const diff = await loadPullDiff(ctx, git, params, "unified"); + + expect(diff.files.map((file) => file.note)).toEqual([ + "This is a binary file and will not be displayed.", + "Submodule pointer, not shown.", + // both blob fetches 404 + "Contents could not be loaded." + ]); + expect(diff.files.every((file) => file.oldFile === undefined)).toBe(true); + }); +}); + +describe("loadPullInterdiff", () => { + const interdiffParams = { + baseRepo: "did:plc:repo" as never, + baseCommit1: "v1base", + baseCommit2: "v1head", + headRepo: "did:plc:repo" as never, + headCommit1: "v2base", + headCommit2: "v2head" + }; + + it("takes the left side inline and fetches only the right", async () => { + const { ctx, git, calls } = harness( + [ + { + lhsSrc: src("a.ts", { content: "one\n" }), + rhsSrc: src("a.ts"), + hunks: [{ novelLhs: [0], novelRhs: [0], lines: [{ lhs: 0, rhs: 0 }] }], + hasSyntacticChanges: false + }, + // the rebased tree only exists inside one gitmirror request, so a left + // side with no inline text can never be recovered + { + lhsSrc: src("b.ts"), + rhsSrc: src("b.ts"), + hunks: [], + hasSyntacticChanges: false + } + ], + { "v2head:a.ts": "two\n", "v2head:b.ts": "two\n" } + ); + + const diff = await loadPullInterdiff(ctx, git, interdiffParams, "unified"); + + expect(diff.files[0]).toMatchObject({ + oldFile: { name: "a.ts", contents: "one\n" }, + newFile: { name: "a.ts", contents: "two\n" } + }); + expect(diff.files[1].note).toBe("Contents could not be loaded."); + // one blob per file, the right side only + expect(calls.filter((path) => path.endsWith("repo.blob"))).toHaveLength(2); + }); +}); diff --git a/web/src/lib/api/pullDiff.ts b/web/src/lib/api/pullDiff.ts new file mode 100644 index 00000000..119972b0 --- /dev/null +++ b/web/src/lib/api/pullDiff.ts @@ -0,0 +1,166 @@ +import { browser } from "$app/environment"; +import { MAX_INLINE_SIZE } from "./blob"; +import { blob } from "./gitclient"; +import { + diffFileKind, + diffFileStat, + getDiff, + getInterdiff, + type MirrorFileDiff, + type MirrorFileKind +} from "./gitmirror"; +import { pierreDiffOptions, type DiffStyle } from "$lib/components/repo/pierre"; +import type { FileContents } from "@pierre/diffs"; +import type { BobbinContext } from "./client"; +import type { GitTarget } from "./gitclient"; +import type { DiffStat } from "./diff"; +import type * as GetDiff from "./lexicons/types/sh/tangled/git/temp2/getDiff"; +import type * as GetInterdiff from "./lexicons/types/sh/tangled/git/temp2/getInterdiff"; + +const FILE_LIMIT = 25; + +export interface PullDiffFile { + // stable across sides, used for anchors and the prerender map + key: string; + name: string; + oldName?: string; + kind: MirrorFileKind; + stat: { insertions: number; deletions: number }; + // set when there is nothing to render: binary, submodule, or over the cap + note?: string; + oldFile?: FileContents; + newFile?: FileContents; +} + +export interface PullDiff { + files: PullDiffFile[]; + stat: DiffStat; + // how many files were listed but not fetched + truncated: number; + // server-prerendered shadow dom per file key, only for the initial ssr + prerendered?: Record; +} + +// the diff endpoint answers metadata only — `diffSrc` is path/oid/size/flags and +// a hunk is aligned line numbers — so the text comes from the blob endpoint and +// pierre diffs the two sides itself +const sideContents = async (git: GitTarget, ref: string, path: string): Promise => { + if (!path) return ""; + const output = await blob(git, { ref, path }, MAX_INLINE_SIZE).catch(() => null); + if (!output || output.encoding !== "utf-8") return null; + return output.content ?? ""; +}; + +// both sides of one file, `null` for a side whose text could not be read +type Sides = (file: MirrorFileDiff) => Promise<[string | null, string | null]>; + +const buildDiff = async ( + diffs: MirrorFileDiff[], + style: DiffStyle, + sides: Sides +): Promise => { + const stat: DiffStat = { insertions: 0, deletions: 0, files_changed: diffs.length }; + const files = await Promise.all( + diffs.map(async (file, index): Promise => { + const fileStat = diffFileStat(file); + stat.insertions += fileStat.insertions; + stat.deletions += fileStat.deletions; + + const kind = diffFileKind(file); + const name = file.rhsSrc.path || file.lhsSrc.path; + const base: PullDiffFile = { + key: `${file.lhsSrc.path}→${file.rhsSrc.path}`, + name, + oldName: kind === "renamed" ? file.lhsSrc.path : undefined, + kind, + stat: fileStat + }; + + if (file.lhsSrc.isSubmodule || file.rhsSrc.isSubmodule) { + return { ...base, note: "Submodule pointer, not shown." }; + } + if (file.lhsSrc.isBinary || file.rhsSrc.isBinary) { + return { ...base, note: "This is a binary file and will not be displayed." }; + } + if (index >= FILE_LIMIT) { + return { ...base, note: "Not rendered: too many files in this diff." }; + } + + const [oldContents, newContents] = await sides(file); + if (oldContents === null || newContents === null) { + return { ...base, note: "Contents could not be loaded." }; + } + + return { + ...base, + // pierre infers the language from the name, same as the blob page + oldFile: { name: file.lhsSrc.path || name, contents: oldContents }, + newFile: { name, contents: newContents } + }; + }) + ); + + return { + files, + stat, + truncated: Math.max(diffs.length - FILE_LIMIT, 0), + prerendered: await prerender(files, style) + }; +}; + +export const loadPullDiff = async ( + ctx: BobbinContext, + git: GitTarget, + params: GetDiff.$params, + style: DiffStyle +): Promise => { + const { diffs } = await getDiff(ctx, params); + return buildDiff(diffs, style, (file) => + Promise.all([ + // the left side is read at baseCommit, while gitmirror diffs from + // merge_base(base, head). identical when base is the merge base, which + // is what a pull version stores + sideContents(git, params.baseCommit, file.lhsSrc.path), + sideContents(git, params.headCommit, file.rhsSrc.path) + ]) + ); +}; + +// an interdiff's right side is version2's head tree, so it reads like any other +// diff — but its left side is a tree gitmirror rebased in memory for this one +// request, reachable by no ref, so that text can only arrive inlined +export const loadPullInterdiff = async ( + ctx: BobbinContext, + git: GitTarget, + params: GetInterdiff.$params, + style: DiffStyle +): Promise => { + const { diffs } = await getInterdiff(ctx, params); + return buildDiff(diffs, style, async (file) => [ + file.lhsSrc.path ? (file.lhsSrc.content ?? null) : "", + await sideContents(git, params.headCommit2, file.rhsSrc.path) + ]); +}; + +// the first paint ships highlighted, like the commit page does +const prerender = async ( + files: PullDiffFile[], + style: DiffStyle +): Promise | undefined> => { + if (browser) return undefined; + const { preloadMultiFileDiff } = await import("@pierre/diffs/ssr"); + const options = pierreDiffOptions(style); + const entries = await Promise.all( + files + .filter((file) => file.oldFile && file.newFile) + .map(async (file) => { + const { prerenderedHTML } = await preloadMultiFileDiff({ + oldFile: file.oldFile as FileContents, + newFile: file.newFile as FileContents, + options + }); + return [file.key, prerenderedHTML] as const; + }) + ); + return Object.fromEntries(entries); +}; diff --git a/web/src/lib/api/pullPage.ts b/web/src/lib/api/pullPage.ts new file mode 100644 index 00000000..d011280b --- /dev/null +++ b/web/src/lib/api/pullPage.ts @@ -0,0 +1,136 @@ +import { listCommits, toMirrorCommitSummary } from "./gitmirror"; +import { loadPullDiff, loadPullInterdiff, type PullDiff } from "./pullDiff"; +import { + resolveChange, + type PullDiffView, + type PullInterdiffView, + type Revspec +} from "./pullRoute"; +import type { Did } from "@atcute/lexicons/syntax"; +import type { BobbinContext } from "./client"; +import type { GitTarget } from "./gitclient"; +import type { CommitSummary } from "./repo"; +import type { DiffStyle } from "$lib/components/repo/pierre"; + +const COMMIT_LIMIT = 100; + +export interface PullPageDeps { + ctx: BobbinContext; + git: GitTarget; + // todo: fork-based pulls read their head side from the source repo + repo: Did; + diffStyle: DiffStyle; +} + +// what either mode hands the page +export interface PullPageDiff { + commits: CommitSummary[]; + commitsError?: string; + diff?: PullDiff; + diffError?: string; +} + +const listLog = async (deps: PullPageDeps, range: Revspec): Promise => { + const page = await listCommits(deps.ctx, { + repo: deps.repo, + ranges: [`${range.base}..${range.head}`], + limit: COMMIT_LIMIT + }); + return page.commits.map(toMirrorCommitSummary); +}; + +const message = (cause: unknown) => (cause instanceof Error ? cause.message : String(cause)); + +const diffOf = (deps: PullPageDeps, spec: Revspec) => + loadPullDiff( + deps.ctx, + deps.git, + { + baseRepo: deps.repo, + baseCommit: spec.base, + headRepo: deps.repo, + headCommit: spec.head + }, + deps.diffStyle + ); + +// both dids feed one scratch repo, so the pair is the same as a diff's +const interdiffOf = (deps: PullPageDeps, a: Revspec, b: Revspec) => + loadPullInterdiff( + deps.ctx, + deps.git, + { + baseRepo: deps.repo, + baseCommit1: a.base, + baseCommit2: a.head, + headRepo: deps.repo, + headCommit1: b.base, + headCommit2: b.head + }, + deps.diffStyle + ); + +// `version` is the whole round being viewed: the commit list always covers it, +// whatever range the diff itself was narrowed to +export const prepareDiff = async ( + deps: PullPageDeps, + view: PullDiffView, + version: Revspec +): Promise => { + const result: PullPageDiff = { commits: [] }; + + // the two failures are independent — a dead log still leaves a readable diff + await Promise.all([ + listLog(deps, version).then( + (commits) => (result.commits = commits), + (cause) => (result.commitsError = message(cause)) + ), + diffOf(deps, view).then( + (diff) => (result.diff = diff), + (cause) => (result.diffError = message(cause)) + ) + ]); + + return result; +}; + +export const prepareInterdiff = async ( + deps: PullPageDeps, + view: PullInterdiffView +): Promise => { + const result: PullPageDiff = { commits: [] }; + + let priorCommits: CommitSummary[] = []; + try { + [result.commits, priorCommits] = await Promise.all([ + listLog(deps, view.to), + view.changeId ? listLog(deps, view.from) : [] + ]); + } catch (cause) { + result.commitsError = message(cause); + } + + try { + if (!view.changeId) { + result.diff = await interdiffOf(deps, view.from, view.to); + } else if (result.commitsError) { + result.diffError = result.commitsError; + } else { + const target = resolveChange(view.changeId, priorCommits, result.commits, { + from: view.from.base, + to: view.to.base + }); + if (target.kind === "missing") { + result.diffError = "Can't find commit with given change-id."; + } else if (target.kind === "diff") { + result.diff = await diffOf(deps, target.spec); + } else { + result.diff = await interdiffOf(deps, target.from, target.to); + } + } + } catch (cause) { + result.diffError = message(cause); + } + + return result; +}; diff --git a/web/src/lib/api/pullRoute.test.ts b/web/src/lib/api/pullRoute.test.ts new file mode 100644 index 00000000..075fd176 --- /dev/null +++ b/web/src/lib/api/pullRoute.test.ts @@ -0,0 +1,164 @@ +import { describe, expect, it } from "vitest"; +import { + activeCommitId, + activeVersion, + isRedirect, + parseDiffRoute, + parseInterdiffRoute, + parseRange, + pullHref, + resolveChange, + seeAll, + type PullView +} from "./pullRoute"; + +const versions = [ + { base: "b0", head: "h0" }, + { base: "b1", head: "h1" } +]; + +// dispatches on `..` the way the loader does +const view = (versionParam: string, rangeParam?: string): PullView => { + const result = versionParam.includes("..") + ? parseInterdiffRoute(versions, versionParam, rangeParam) + : parseDiffRoute(versions, versionParam, rangeParam); + if (isRedirect(result)) throw new Error(`unexpected redirect: ${result.redirect}`); + return result; +}; + +describe("parseRange", () => { + it("accepts every shape the appview accepts", () => { + expect(parseRange("")).toEqual({ base: "", head: "" }); + expect(parseRange(" ")).toEqual({ base: "", head: "" }); + expect(parseRange("abc")).toEqual({ base: "", head: "abc" }); + expect(parseRange("a..b")).toEqual({ base: "a", head: "b" }); + expect(parseRange("..b")).toEqual({ base: "", head: "b" }); + expect(parseRange("a..")).toEqual({ base: "a", head: "" }); + }); + + it("rejects empty and multi ranges", () => { + expect(parseRange("..")).toBeNull(); + expect(parseRange("a...b")).toBeNull(); + expect(parseRange("a..b..c")).toBeNull(); + }); +}); + +describe("parseDiffRoute", () => { + it("resolves latest and numbers to the version's own ends", () => { + expect(view("latest")).toEqual({ + mode: "diff", + version: 1, + base: "b1", + head: "h1", + isDiffBase: true, + isDiffHead: true + }); + expect(view("0")).toMatchObject({ version: 0, base: "b0", head: "h0" }); + }); + + it("takes an explicit range, and the base/head magic words", () => { + expect(view("1", "c1..c2")).toMatchObject({ + base: "c1", + head: "c2", + isDiffBase: false, + isDiffHead: false + }); + // a bare rev is a head, and the words fall back to the version's ends + expect(view("1", "c2")).toMatchObject({ base: "b1", head: "c2", isDiffBase: true }); + expect(view("1", "base..head")).toMatchObject({ base: "b1", head: "h1" }); + }); + + it("marks see-all only when both sides defaulted", () => { + expect(seeAll(view("1"))).toBe(true); + expect(seeAll(view("1", "base..head"))).toBe(true); + expect(seeAll(view("1", "c1..c2"))).toBe(false); + expect(seeAll(view("1", "..c2"))).toBe(false); + }); + + it("redirects a bad version to latest and a bad range to the version", () => { + expect(parseDiffRoute(versions, "9")).toEqual({ redirect: "latest" }); + expect(parseDiffRoute(versions, "foo")).toEqual({ redirect: "latest" }); + expect(parseDiffRoute(versions, "-1")).toEqual({ redirect: "latest" }); + expect(parseDiffRoute(versions, "1", "a..b..c")).toEqual({ redirect: "version" }); + }); +}); + +describe("parseInterdiffRoute", () => { + it("parses both versions, resolves their ends, and takes the change filter", () => { + expect(view("0..1")).toEqual({ + mode: "interdiff", + version1: 0, + version2: 1, + from: { base: "b0", head: "h0" }, + to: { base: "b1", head: "h1" }, + changeId: "" + }); + expect(view("0..1", "all")).toMatchObject({ changeId: "" }); + expect(view("0..1", "kmzwvxqo")).toMatchObject({ changeId: "kmzwvxqo" }); + }); + + it("bounds-checks both sides instead of panicking", () => { + expect(parseInterdiffRoute(versions, "0..9")).toEqual({ redirect: "latest" }); + expect(parseInterdiffRoute(versions, "foo..1")).toEqual({ redirect: "latest" }); + expect(parseInterdiffRoute(versions, "..1")).toEqual({ redirect: "latest" }); + }); + + it("reports the newer version as active, and see-all without a filter", () => { + expect(activeVersion(view("0..1"))).toBe(1); + expect(activeCommitId(view("0..1", "kmzwvxqo"))).toBe("kmzwvxqo"); + expect(seeAll(view("0..1"))).toBe(true); + expect(seeAll(view("0..1", "kmzwvxqo"))).toBe(false); + }); +}); + +describe("resolveChange", () => { + const bases = { from: "v1base", to: "v2base" }; + const older = [{ hash: "aaa", parent: "aaaparent", changeId: "kmzwvxqo" }]; + const newer = [ + { hash: "bbb", parent: "bbbparent", changeId: "kmzwvxqo" }, + { hash: "ccc", parent: "cccparent", changeId: "nvtsyrpq" } + ]; + + it("interdiffs the two commits carrying the change", () => { + expect(resolveChange("kmzwvxqo", older, newer, bases)).toEqual({ + kind: "interdiff", + from: { base: "aaaparent", head: "aaa" }, + to: { base: "bbbparent", head: "bbb" } + }); + }); + + it("plain-diffs a change that only exists in the newer version", () => { + expect(resolveChange("nvtsyrpq", older, newer, bases)).toEqual({ + kind: "diff", + spec: { base: "cccparent", head: "ccc" } + }); + }); + + it("reports a change the newer version does not have", () => { + expect(resolveChange("zzzzzzzz", older, newer, bases)).toEqual({ kind: "missing" }); + // a repo without jj change ids can never match + expect(resolveChange("kmzwvxqo", older, [{ hash: "bbb" }], bases)).toEqual({ kind: "missing" }); + }); + + it("falls back to the version base for a root commit", () => { + expect( + resolveChange("kmzwvxqo", [{ hash: "aaa", changeId: "kmzwvxqo" }], newer, bases) + ).toEqual({ + kind: "interdiff", + from: { base: "v1base", head: "aaa" }, + to: { base: "bbbparent", head: "bbb" } + }); + }); +}); + +describe("pullHref", () => { + const uri = "at://did:plc:alice/sh.tangled.repo.pull/3lzg7tkxvq222"; + + it("encodes the at-uri into a single segment", () => { + expect(pullHref("alice.test/core", uri)).toBe( + "/alice.test/core/pulls/at%3A%2F%2Fdid%3Aplc%3Aalice%2Fsh.tangled.repo.pull%2F3lzg7tkxvq222" + ); + expect(pullHref("alice.test/core", uri, 1, "c1..c2")).toMatch(/\/1\/c1\.\.c2$/); + expect(pullHref("alice.test/core", uri, "latest")).toMatch(/\/latest$/); + }); +}); diff --git a/web/src/lib/api/pullRoute.ts b/web/src/lib/api/pullRoute.ts new file mode 100644 index 00000000..09cbdb66 --- /dev/null +++ b/web/src/lib/api/pullRoute.ts @@ -0,0 +1,177 @@ +// the pull page's url grammar: +// +// /pulls//latest diff of the newest version +// /pulls//1 diff of version 1 +// /pulls//1/c1..c2 that diff narrowed to a commit range +// /pulls//0..1 interdiff between two versions +// /pulls//0..1/ that interdiff narrowed to one change + +export interface Revspec { + base: string; + head: string; +} + +// parses `..`, where either side may be missing +export const parseRange = (input: string): Revspec | null => { + const spec = input.trim(); + if (spec === "") return { base: "", head: "" }; + // `...` is a symmetric difference in git, and two ranges are not a range + if (spec.includes("...") || spec.split("..").length > 2) return null; + if (!spec.includes("..")) return { base: "", head: spec }; + + const [base, head] = spec.split("..", 2).map((part) => part.trim()); + if (base === "" && head === "") return null; + return { base, head }; +}; + +export interface PullDiffView { + mode: "diff"; + version: number; + base: string; + head: string; + // true when that side was defaulted, which is what makes the commit card's + // "See all changes" row the active one + isDiffBase: boolean; + isDiffHead: boolean; +} + +export interface PullInterdiffView { + mode: "interdiff"; + version1: number; + version2: number; + // the two versions' own ends, resolved here so nothing downstream re-indexes + from: Revspec; + to: Revspec; + // "" means all changes + changeId: string; +} + +export type PullView = PullDiffView | PullInterdiffView; + +// a bad version sends you to the newest one, a bad range only drops itself +export type PullRouteRedirect = { redirect: "latest" | "version" }; + +export const isRedirect = ( + result: T | PullRouteRedirect +): result is PullRouteRedirect => "redirect" in result; + +const index = (raw: string, count: number): number | null => { + if (!/^\d+$/.test(raw)) return null; + const value = Number(raw); + return value < count ? value : null; +}; + +// `/..[/]` +export const parseInterdiffRoute = ( + versions: readonly Revspec[], + versionParam: string, + rangeParam?: string +): PullInterdiffView | PullRouteRedirect => { + const range = parseRange(versionParam); + if (!range) return { redirect: "latest" }; + const version1 = index(range.base, versions.length); + const version2 = index(range.head, versions.length); + // the appview indexes its versions raw here and panics on a bad number + if (version1 === null || version2 === null) return { redirect: "latest" }; + const changeId = rangeParam === "all" ? "" : (rangeParam ?? ""); + return { + mode: "interdiff", + version1, + version2, + // copied field by field: a version carries its comments too, and `view` is + // serialized to the page + from: { base: versions[version1].base, head: versions[version1].head }, + to: { base: versions[version2].base, head: versions[version2].head }, + changeId + }; +}; + +// `/[/..]` +export const parseDiffRoute = ( + versions: readonly Revspec[], + versionParam: string, + rangeParam?: string +): PullDiffView | PullRouteRedirect => { + const version = + versionParam === "latest" ? versions.length - 1 : index(versionParam, versions.length); + if (version === null || version < 0) return { redirect: "latest" }; + + const spec = parseRange(rangeParam ?? ""); + if (!spec) return { redirect: "version" }; + + // `base` and `head` are the appview's magic words for the version's own ends + const isDiffBase = spec.base === "" || spec.base === "base"; + const isDiffHead = spec.head === "" || spec.head === "head"; + return { + mode: "diff", + version, + base: isDiffBase ? versions[version].base : spec.base, + head: isDiffHead ? versions[version].head : spec.head, + isDiffBase, + isDiffHead + }; +}; + +// the version whose commits are listed, and whose card the rail highlights +export const activeVersion = (view: PullView): number => + view.mode === "diff" ? view.version : view.version2; + +// the commit row to mark, compared on short hashes like the appview does +export const activeCommitId = (view: PullView): string => + view.mode === "diff" ? view.head : view.changeId; + +export const seeAll = (view: PullView): boolean => + view.mode === "diff" ? view.isDiffBase && view.isDiffHead : view.changeId === ""; + +// only the fields a change-id lookup reads, so this module keeps no deps +interface ChangeCommit { + hash: string; + parent?: string; + changeId?: string; +} + +export type ChangeTarget = + | { kind: "missing" } + // the change is new in the second version, so there is nothing to interdiff + | { kind: "diff"; spec: Revspec } + | { kind: "interdiff"; from: Revspec; to: Revspec }; + +// `/../` narrows an interdiff to the one commit carrying that +// jj change id on each side, exactly as `appview/pulls/single.go` does +export const resolveChange = ( + changeId: string, + fromCommits: readonly ChangeCommit[], + toCommits: readonly ChangeCommit[], + bases: { from: string; to: string } +): ChangeTarget => { + const find = (commits: readonly ChangeCommit[]) => + commits.find((commit) => commit.changeId === changeId); + const to = find(toCommits); + // the row was picked from the second version's log, so this is a typed url + if (!to) return { kind: "missing" }; + + // the appview takes the first parent, which is the zero hash on a root commit + // and would be rejected; the version's own base stands in for that + const spec = (commit: ChangeCommit, base: string): Revspec => ({ + base: commit.parent ?? base, + head: commit.hash + }); + const from = find(fromCommits); + return from + ? { kind: "interdiff", from: spec(from, bases.from), to: spec(to, bases.to) } + : { kind: "diff", spec: spec(to, bases.to) }; +}; + +// `///pulls//[/]`, the only place the at-uri +// gets re-encoded. generated links use numbers, `latest` is only ever typed +export const pullHref = ( + repoBase: string, + uri: string, + version?: number | string, + range?: string +): string => { + const parts = [repoBase, "pulls", encodeURIComponent(uri)]; + if (version !== undefined) parts.push(String(version)); + if (range) parts.push(range); + return `/${parts.join("/")}`; +}; diff --git a/web/src/lib/api/records.ts b/web/src/lib/api/records.ts index 544e021c..eb6e023f 100644 --- a/web/src/lib/api/records.ts +++ b/web/src/lib/api/records.ts @@ -3,10 +3,12 @@ import { jsonGet } from "./_request"; import type * as ShTangledActorProfile from "./lexicons/types/sh/tangled/actor/profile"; import type * as ShTangledFeedComment from "./lexicons/types/sh/tangled/feed/comment"; import type * as ShTangledFeedReaction from "./lexicons/types/sh/tangled/feed/reaction"; +import type * as ShTangledPullGetPullView from "./lexicons/types/sh/tangled/pull/getPullView"; import type * as ShTangledRepo from "./lexicons/types/sh/tangled/repo"; import type * as ShTangledRepoIssue from "./lexicons/types/sh/tangled/repo/issue"; import type * as ShTangledRepoIssueState from "./lexicons/types/sh/tangled/repo/issue/state"; import type * as ShTangledRepoPull from "./lexicons/types/sh/tangled/repo/pull"; +import type * as ShTangledRepoPullStatus from "./lexicons/types/sh/tangled/repo/pull/status"; import type * as ShTangledString from "./lexicons/types/sh/tangled/string"; export interface RecordView { @@ -24,6 +26,7 @@ export type ProfileRecord = ShTangledActorProfile.Main; export type IssueRecord = ShTangledRepoIssue.Main; export type IssueStateRecord = ShTangledRepoIssueState.Main; export type PullRecord = ShTangledRepoPull.Main; +export type PullStatusRecord = ShTangledRepoPullStatus.Main; export type StringRecord = ShTangledString.Main; export type CommentRecord = ShTangledFeedComment.Main; export type ReactionRecord = ShTangledFeedReaction.Main; @@ -43,6 +46,18 @@ export const getRepoByName = ( init?: XrpcRequestInit ) => jsonGet>(ctx, "sh.tangled.repo.getRepoByName", { owner, name }, init); +export interface RepoListPage { + cursor?: string | null; + items: RecordView[]; +} + +export const listRepos = ( + ctx: BobbinContext, + subject: string, + filter: { limit?: number; cursor?: string; order?: "asc" | "desc" } = {}, + init?: XrpcRequestInit +) => jsonGet(ctx, "sh.tangled.repo.listRepos", { subject, ...filter }, init); + export const getProfile = (ctx: BobbinContext, did: string, init?: XrpcRequestInit) => jsonGet>( ctx, @@ -125,6 +140,27 @@ export const listReactions = ( export const getPull = (ctx: BobbinContext, pull: string, init?: XrpcRequestInit) => jsonGet>(ctx, "sh.tangled.repo.getPull", { pull }, init); +export type PullViewDetailed = ShTangledPullGetPullView.$output; + +export const getPullView = (ctx: BobbinContext, uri: string, init?: XrpcRequestInit) => + jsonGet(ctx, "sh.tangled.pull.getPullView", { uri }, init); + +export type PullState = "open" | "closed" | "merged"; + +export interface PullListItem { + cid?: string; + commentCount: number; + state: string; + stateUpdatedAt?: string; + uri: string; + value: PullRecord; +} + +export interface PullListPage { + cursor?: string | null; + items: PullListItem[]; +} + export const getRepos = (ctx: BobbinContext, repos: readonly string[], init?: XrpcRequestInit) => jsonGet>(ctx, "sh.tangled.repo.getRepos", { repos }, init); diff --git a/web/src/lib/api/repo.ts b/web/src/lib/api/repo.ts index 76597d18..cc1d6f9c 100644 --- a/web/src/lib/api/repo.ts +++ b/web/src/lib/api/repo.ts @@ -150,6 +150,8 @@ export interface CommitSummary { authorHandle?: string; when: string; changeId?: string; + // first parent, only filled in where a caller needs to link a commit range + parent?: string; } export const didFromSignature = (email: string): string | undefined => diff --git a/web/src/lib/components/comment/CommentEditor.svelte b/web/src/lib/components/comment/CommentEditor.svelte index f8a3ae5d..a78dbf87 100644 --- a/web/src/lib/components/comment/CommentEditor.svelte +++ b/web/src/lib/components/comment/CommentEditor.svelte @@ -20,6 +20,8 @@ // when set, the comment strongRefs this parent comment (threaded reply) replyToUri?: string; replyToCid?: string; + // required by the lexicon when the subject is a pull: which round this lands in + pullRoundIdx?: number; authorDid: string; authorHandle: string; // reused on edit so the record keeps its identity; omitted when composing @@ -41,6 +43,7 @@ subjectCid, replyToUri, replyToCid, + pullRoundIdx, authorDid, authorHandle, rkey, @@ -74,6 +77,7 @@ body: { $type: "sh.tangled.markup.markdown", text: body }, createdAt: createdAtValue }; + if (pullRoundIdx !== undefined) record.pullRoundIdx = pullRoundIdx; if (replyToUri && replyToCid) { record.replyTo = { uri: replyToUri, cid: replyToCid } as CommentRecord["replyTo"]; } diff --git a/web/src/lib/components/comment/comments.ts b/web/src/lib/components/comment/comments.ts index eb9efde8..1eebfa0f 100644 --- a/web/src/lib/components/comment/comments.ts +++ b/web/src/lib/components/comment/comments.ts @@ -7,6 +7,8 @@ export interface CommentView { authorDid: string; authorHandle: string; createdAt: string; + // the appview's comment header shows "Edited