Proposals in native diff editors (0.2.0) bf73998c5198
Proposal rows expand to their changed files; a file opens in VS Code's diff editor with the target's state against the projected state, both served from the hub through a read-only oxygenhub-proposal document scheme, so no checkout is needed. View Changes opens every file in one multi-file diff editor. Review threads sit on the projected side at the line the hub reports, and new comments there are anchored to that line's atom. Spec §5.1 and §6 updated.
- Author
- Andy McCall @andymccall <[email protected]>
- When
- · 2026-09-27 23:45 UTC
- Signature
- ✔ signed by Andy McCall
- Hash
bf73998c5198184ddaf751c94dc38e24742cf3c6d57f671dcd16e35fdd3aa003- Depends on
- 486c15b00314 52cef00143db f0b06deac4b9
- Depended on by
- 991516a107fa d21a85954a34 263de209e5ba e04950748142 092b8bf9f2ac 95f69b4a3ac2 f902d5b8bfeb b2dd082c57e4
- In bookmarks
- main
- In tags
- v0.10.0 v0.10.1 v0.10.2 v0.11.0 v0.11.1 v0.11.2 v0.13.0
- Edits
- 47 atoms, 1 tree operation, 11 files
Browse the tree at this change
added src/diff.ts +83 −0
| 1 | + | // Proposals in VS Code's own diff editors (spec §6.1): each side of a |
|
| 2 | + | // changed file is a read-only document served from the hub, the target's |
|
| 3 | + | // state on the left and the projected state on the right, so a proposal |
|
| 4 | + | // can be read and reviewed without checking it out. |
|
| 5 | + | ||
| 6 | + | import * as vscode from "vscode"; |
|
| 7 | + | import type * as api from "./hubapi"; |
|
| 8 | + | import { HubAuth } from "./auth"; |
|
| 9 | + | import { DiffRef, decodeDiffQuery, encodeDiffQuery } from "./hub"; |
|
| 10 | + | import type { RepoRef } from "./proposals"; |
|
| 11 | + | ||
| 12 | + | export const SCHEME = "oxygenhub-proposal"; |
|
| 13 | + | ||
| 14 | + | /** The document URI of one side of a file in a proposal. */ |
|
| 15 | + | export function diffUri(repo: RepoRef, number: number, side: "base" | "head", path: string): vscode.Uri { |
|
| 16 | + | return vscode.Uri.from({ scheme: SCHEME, path: `/${path}`, query: encodeDiffQuery({ base: repo.base, owner: repo.owner, name: repo.name, number, side, path }) }); |
|
| 17 | + | } |
|
| 18 | + | ||
| 19 | + | /** The proposal, side and path a document of this scheme shows. */ |
|
| 20 | + | export function refOf(uri: vscode.Uri): DiffRef | undefined { |
|
| 21 | + | return uri.scheme === SCHEME ? decodeDiffQuery(uri.query, uri.path) : undefined; |
|
| 22 | + | } |
|
| 23 | + | ||
| 24 | + | /** Serves both sides from the hub; a side where the file is absent is empty. */ |
|
| 25 | + | export class ProposalContentProvider implements vscode.TextDocumentContentProvider, vscode.Disposable { |
|
| 26 | + | private readonly emitter = new vscode.EventEmitter<vscode.Uri>(); |
|
| 27 | + | readonly onDidChange = this.emitter.event; |
|
| 28 | + | private readonly open = new Set<string>(); |
|
| 29 | + | ||
| 30 | + | constructor(private readonly auth: HubAuth) {} |
|
| 31 | + | ||
| 32 | + | async provideTextDocumentContent(uri: vscode.Uri): Promise<string> { |
|
| 33 | + | const r = refOf(uri); |
|
| 34 | + | if (!r) { |
|
| 35 | + | return ""; |
|
| 36 | + | } |
|
| 37 | + | this.open.add(uri.toString()); |
|
| 38 | + | try { |
|
| 39 | + | return (await this.auth.clientFor(r.base).proposalContent(r.owner, r.name, r.number, r.side, r.path)) ?? ""; |
|
| 40 | + | } catch (e) { |
|
| 41 | + | void vscode.window.showWarningMessage(`Oxygenhub: could not load ${r.path}: ${(e as Error).message}`); |
|
| 42 | + | return ""; |
|
| 43 | + | } |
|
| 44 | + | } |
|
| 45 | + | ||
| 46 | + | /** Reload every open side of a proposal, after it gains changes. */ |
|
| 47 | + | refresh(base: string, owner: string, name: string, number: number): void { |
|
| 48 | + | for (const s of this.open) { |
|
| 49 | + | const r = refOf(vscode.Uri.parse(s)); |
|
| 50 | + | if (r && r.base === base && r.owner === owner && r.name === name && r.number === number) { |
|
| 51 | + | this.emitter.fire(vscode.Uri.parse(s)); |
|
| 52 | + | } |
|
| 53 | + | } |
|
| 54 | + | } |
|
| 55 | + | ||
| 56 | + | dispose(): void { |
|
| 57 | + | this.emitter.dispose(); |
|
| 58 | + | } |
|
| 59 | + | } |
|
| 60 | + | ||
| 61 | + | function sides(repo: RepoRef, number: number, f: api.ProposalFile): { left: vscode.Uri; right: vscode.Uri } { |
|
| 62 | + | return { left: diffUri(repo, number, "base", f.from ?? f.path), right: diffUri(repo, number, "head", f.path) }; |
|
| 63 | + | } |
|
| 64 | + | ||
| 65 | + | /** One file in a diff editor. */ |
|
| 66 | + | export async function openFileDiff(repo: RepoRef, number: number, f: api.ProposalFile): Promise<void> { |
|
| 67 | + | const { left, right } = sides(repo, number, f); |
|
| 68 | + | const title = `#${number} ${f.from ? `${f.from} → ${f.path}` : f.path} (${f.kind})`; |
|
| 69 | + | await vscode.commands.executeCommand("vscode.diff", left, right, title, { preview: true }); |
|
| 70 | + | } |
|
| 71 | + | ||
| 72 | + | /** Every file of a proposal in one multi-file diff editor. */ |
|
| 73 | + | export async function openAllChanges(repo: RepoRef, p: api.Proposal, files: api.ProposalFile[]): Promise<void> { |
|
| 74 | + | if (!files.length) { |
|
| 75 | + | void vscode.window.showInformationMessage(`Oxygenhub: #${p.number} changes no files.`); |
|
| 76 | + | return; |
|
| 77 | + | } |
|
| 78 | + | const resources = files.map((f) => { |
|
| 79 | + | const { left, right } = sides(repo, p.number, f); |
|
| 80 | + | return [right, left, right]; |
|
| 81 | + | }); |
|
| 82 | + | await vscode.commands.executeCommand("vscode.changes", `#${p.number} ${p.title}`, resources); |
|
| 83 | + | } |
modified CHANGELOG.md +7 −0
| 1 | 1 | # Changelog |
|
| 2 | 2 | ||
| 3 | + | ## 0.2.0 |
|
| 4 | + | ||
| 5 | + | - Proposals expand to their changed files. Click one to open it in VS Code's diff editor: the target's state against the projected state, straight from the hub, no checkout needed. |
|
| 6 | + | - **View Changes** on a proposal (inline button, or in the detail) opens every file in one multi-file diff editor. |
|
| 7 | + | - Review threads appear in those diff editors on the projected side, and new comments there are anchored to the line's atom on the hub. |
|
| 8 | + | - Needs Oxygenhub with the proposal files and content API (2026-09-28). |
|
| 9 | + | ||
| 3 | 10 | ## 0.1.2 |
|
| 4 | 11 | ||
| 5 | 12 | - Fix: the dependency on the Oxygen extension named the wrong id (`andymccall.oxygen-vscode` instead of `oxygen.oxygen-vscode`), so VS Code never activated this extension and Sign in did nothing. |
|
| … | 16 unchanged lines |
modified README.md +2 −1
| … | 6 unchanged lines |
||
| 7 | 7 | ||
| 8 | 8 | - **Sign in** with a hub URL and a personal access token (Settings › Tokens on the hub). |
|
| 9 | 9 | - **Proposals** view: needing me, mine, open; detail with blockers, approvals, checks and changes; check out the projected state; review; seal your approval and land, both through `oxy` in a terminal with your own key. |
|
| 10 | - | - **Review threads** on the checked-out proposal's files, anchored to atoms so they follow lines through rebases. |
|
| 10 | + | - **Diffs in the editor**: expand a proposal to its files and open each in VS Code's diff editor (target against projected state), or View Changes for all of them at once, without checking out. |
|
| 11 | + | - **Review threads** in those diffs and on the checked-out proposal's files, anchored to atoms so they follow lines through rebases. |
|
| 11 | 12 | - **Issues** view: open, create, comment, close, assign. |
|
| 12 | 13 | - **Open on Remote** in the Oxygen extension works for hubs once this extension is installed. |
|
| 13 | 14 | - **Status bar**: the checks of the proposal whose source is your current bookmark, live. |
|
| … | 2 unchanged lines |
modified docs/spec.md +6 −0
| … | 56 unchanged lines |
||
| 57 | 57 | ||
| 58 | 58 | **Detail** is a webview: description, blockers, approvals (sealed or pending), checks, the changes list with signatures, and buttons for the actions. **Check out** fetches from the hub remote and sets a local bookmark `proposal/N` to the projected heads, then switches to it, so the files in the editor are the projected state; review threads then appear in place. |
|
| 59 | 59 | ||
| 60 | + | ### 5.1 Diff editors |
|
| 61 | + | ||
| 62 | + | Each proposal row expands to the files it changes (`GET …/proposals/N/files`), with a diff icon per kind and `+added −removed`. Clicking a file opens VS Code's own diff editor; **View Changes** on the row (or in the detail) opens every file in one multi-file diff editor (`vscode.changes`). Both sides are read-only documents of the `oxygenhub-proposal` scheme, served from `GET …/proposals/N/content/{base|head}/{path}`: the target bookmark's state on the left, the projected state on the right, so a proposal can be read and reviewed without checking it out. A side where the file does not exist is empty; a rename shows its old path on the left. Open sides reload on refresh, since a proposal gains changes when its source bookmark moves. |
|
| 63 | + | ||
| 60 | 64 | ## 6. Review threads |
|
| 61 | 65 | ||
| 62 | 66 | A `CommentController` for the checked-out proposal. Threads come from `GET …/proposals/N/comments`; each anchored comment is placed by asking oxy for the current line of its atom (`oxy blame --json` gives atoms per line; the extension finds the line whose `(hash, index)` matches). Replying posts a comment anchored to the same atom; a new thread on a line posts with the line's atom. Resolve toggles the hub's resolved flag. Threads whose atom is no longer live are shown in a separate "outdated" list in the view. |
|
| 63 | 67 | ||
| 68 | + | In the diff editors (§5.1) the same controller places threads on the projected side only, at the line the hub reports for each anchored comment (the API's `line`, found by blame on the projected state), so no checkout or local `oxy` call is needed. A new thread or reply there posts the file and line; the hub anchors it to that line's atom. |
|
| 69 | + | ||
| 64 | 70 | ## 7. Approvals and landing |
|
| 65 | 71 | ||
| 66 | 72 | **Approve** posts the review, then offers to run `oxy seal --proposal <url>` in the repository's terminal so the approval is backed by the user's seal; the row shows "pending seal" until the hub reports it sealed. **Land** runs `oxy proposal land <url>` in a terminal (the hub's own landing is offered when the hub reports it can land, for repositories without a policy). Both use the integrated terminal so the user sees exactly what runs with their key. |
|
| … | 26 unchanged lines |
modified package.json +23 −1
| … | 1 unchanged lines |
||
| 2 | 2 | "name": "oxygenhub-vscode", |
|
| 3 | 3 | "displayName": "Oxygenhub", |
|
| 4 | 4 | "description": "Proposals, reviews, issues and checks from an Oxygenhub, in the editor. Builds on the Oxygen extension.", |
|
| 5 | - | "version": "0.1.2", |
|
| 5 | + | "version": "0.2.0", |
|
| 6 | 6 | "publisher": "andymccall", |
|
| 7 | 7 | "license": "MIT OR Apache-2.0", |
|
| 8 | 8 | "icon": "assets/images/oxygenhub.png", |
|
| … | 106 unchanged lines |
||
| 115 | 115 | "icon": "$(link-external)" |
|
| 116 | 116 | }, |
|
| 117 | 117 | { |
|
| 118 | + | "command": "oxygenhub.openProposalFile", |
|
| 119 | + | "title": "Open Proposal File Diff", |
|
| 120 | + | "category": "Oxygenhub" |
|
| 121 | + | }, |
|
| 122 | + | { |
|
| 123 | + | "command": "oxygenhub.openProposalChanges", |
|
| 124 | + | "title": "View Changes", |
|
| 125 | + | "category": "Oxygenhub", |
|
| 126 | + | "icon": "$(diff-multiple)" |
|
| 127 | + | }, |
|
| 128 | + | { |
|
| 118 | 129 | "command": "oxygenhub.checkoutProposal", |
|
| 119 | 130 | "title": "Check Out Proposal", |
|
| 120 | 131 | "category": "Oxygenhub", |
|
| … | 92 unchanged lines |
||
| 213 | 224 | ], |
|
| 214 | 225 | "view/item/context": [ |
|
| 215 | 226 | { |
|
| 227 | + | "command": "oxygenhub.openProposalChanges", |
|
| 228 | + | "when": "view == oxygenhub.proposals && viewItem == proposal", |
|
| 229 | + | "group": "inline" |
|
| 230 | + | }, |
|
| 231 | + | { |
|
| 216 | 232 | "command": "oxygenhub.openOnHub", |
|
| 217 | 233 | "when": "view == oxygenhub.proposals && viewItem == proposal", |
|
| 218 | 234 | "group": "inline" |
|
| … | 67 unchanged lines |
||
| 286 | 302 | "when": "commentController == oxygenhub", |
|
| 287 | 303 | "group": "navigation" |
|
| 288 | 304 | } |
|
| 305 | + | ], |
|
| 306 | + | "commandPalette": [ |
|
| 307 | + | { |
|
| 308 | + | "command": "oxygenhub.openProposalFile", |
|
| 309 | + | "when": "false" |
|
| 310 | + | } |
|
| 289 | 311 | ] |
|
| 290 | 312 | } |
|
| 291 | 313 | }, |
|
| … | 18 unchanged lines |
modified src/extension.ts +35 −1
| … | 3 unchanged lines |
||
| 4 | 4 | import * as vscode from "vscode"; |
|
| 5 | 5 | import { HubAuth } from "./auth"; |
|
| 6 | 6 | import { ChecksBar } from "./checks"; |
|
| 7 | + | import { ProposalContentProvider, SCHEME, openAllChanges, openFileDiff, refOf } from "./diff"; |
|
| 7 | 8 | import { hubUrl } from "./hub"; |
|
| 8 | 9 | import { IssueNode, IssuesProvider, showIssue } from "./issues"; |
|
| 9 | 10 | import type { OxygenAPI } from "./oxygenapi"; |
|
| 10 | - | import { HubRepo, ProposalNode, ProposalsProvider, showDetail } from "./proposals"; |
|
| 11 | + | import { HubRepo, ProposalFileNode, ProposalNode, ProposalsProvider, showDetail } from "./proposals"; |
|
| 11 | 12 | import { makeProvider } from "./provider"; |
|
| 12 | 13 | import { ReviewController } from "./review"; |
|
| 13 | 14 | ||
| … | 3 unchanged lines |
||
| 17 | 18 | let issues: IssuesProvider; |
|
| 18 | 19 | let review: ReviewController; |
|
| 19 | 20 | let bar: ChecksBar; |
|
| 21 | + | let content: ProposalContentProvider; |
|
| 20 | 22 | ||
| 21 | 23 | export async function activate(context: vscode.ExtensionContext): Promise<void> { |
|
| 22 | 24 | auth = new HubAuth(context.secrets); |
|
| … | 13 unchanged lines |
||
| 36 | 38 | issues = new IssuesProvider(() => proposals.repos(), auth); |
|
| 37 | 39 | review = new ReviewController(auth); |
|
| 38 | 40 | bar = new ChecksBar(auth, () => proposals.repos()); |
|
| 41 | + | content = new ProposalContentProvider(auth); |
|
| 42 | + | context.subscriptions.push(content, vscode.workspace.registerTextDocumentContentProvider(SCHEME, content)); |
|
| 39 | 43 | context.subscriptions.push(vscode.window.registerTreeDataProvider("oxygenhub.proposals", proposals), vscode.window.registerTreeDataProvider("oxygenhub.issues", issues), review, bar); |
|
| 40 | 44 | context.subscriptions.push(auth.onDidChange(() => refreshAll())); |
|
| 41 | 45 | ||
| … | 26 unchanged lines |
||
| 68 | 72 | const target = node.kind === "proposal" ? { kind: "proposal", number: node.p.number } : { kind: "issue", number: node.i.number }; |
|
| 69 | 73 | void vscode.env.openExternal(vscode.Uri.parse(hubUrl(node.repo.base, node.repo.owner, node.repo.name, target))); |
|
| 70 | 74 | }), |
|
| 75 | + | vscode.commands.registerCommand("oxygenhub.openProposalFile", async (node: ProposalFileNode) => { |
|
| 76 | + | await openFileDiff(node.repo, node.p.number, node.f); |
|
| 77 | + | await review.loadDiff(node.repo, node.p.number).catch(() => undefined); |
|
| 78 | + | }), |
|
| 79 | + | vscode.commands.registerCommand("oxygenhub.openProposalChanges", (...args: unknown[]) => |
|
| 80 | + | withProposal(args, async (repo, number) => { |
|
| 81 | + | try { |
|
| 82 | + | const client = auth.clientFor(repo.base); |
|
| 83 | + | const [p, files] = await Promise.all([client.proposal(repo.owner, repo.name, number), client.proposalFiles(repo.owner, repo.name, number)]); |
|
| 84 | + | await openAllChanges(repo, p, files); |
|
| 85 | + | await review.loadDiff(repo, number); |
|
| 86 | + | } catch (e) { |
|
| 87 | + | void vscode.window.showErrorMessage(`Oxygenhub: ${(e as Error).message}`); |
|
| 88 | + | } |
|
| 89 | + | }), |
|
| 90 | + | ), |
|
| 71 | 91 | vscode.commands.registerCommand("oxygenhub.checkoutProposal", (...args: unknown[]) => withProposal(args, checkout)), |
|
| 72 | 92 | vscode.commands.registerCommand("oxygenhub.review", (...args: unknown[]) => withProposal(args, reviewProposal)), |
|
| 73 | 93 | vscode.commands.registerCommand("oxygenhub.sealApproval", (...args: unknown[]) => withProposal(args, (repo, number) => runInTerminal(repo, `oxy seal --proposal ${hubUrl(repo.base, repo.owner, repo.name, { kind: "proposal", number })}`))), |
|
| … | 33 unchanged lines |
||
| 107 | 127 | ||
| 108 | 128 | function refreshAll(): void { |
|
| 109 | 129 | proposals.refresh(); |
|
| 130 | + | refreshOpenDiffs(); |
|
| 110 | 131 | issues.refresh(); |
|
| 111 | 132 | void bar.refresh(); |
|
| 112 | 133 | } |
|
| 113 | 134 | ||
| 135 | + | /** Reload the sides of open proposal diffs, which may have gained changes. */ |
|
| 136 | + | function refreshOpenDiffs(): void { |
|
| 137 | + | const seen = new Set<string>(); |
|
| 138 | + | for (const doc of vscode.workspace.textDocuments) { |
|
| 139 | + | const r = refOf(doc.uri); |
|
| 140 | + | const key = r && `${r.base}/${r.owner}/${r.name}#${r.number}`; |
|
| 141 | + | if (r && key && !seen.has(key)) { |
|
| 142 | + | seen.add(key); |
|
| 143 | + | content.refresh(r.base, r.owner, r.name, r.number); |
|
| 144 | + | } |
|
| 145 | + | } |
|
| 146 | + | } |
|
| 147 | + | ||
| 114 | 148 | async function guessHubs(): Promise<string[]> { |
|
| 115 | 149 | const out: string[] = []; |
|
| 116 | 150 | for (const r of oxygen?.repositories ?? []) { |
|
| … | 162 unchanged lines |
modified src/hub.ts +48 −0
| … | 103 unchanged lines |
||
| 104 | 104 | return this.request("POST", `/repos/${owner}/${name}/proposals/${number}/reviews`, { verdict, body }); |
|
| 105 | 105 | } |
|
| 106 | 106 | ||
| 107 | + | /** The files a proposal changes, target state against projected state. */ |
|
| 108 | + | proposalFiles(owner: string, name: string, number: number): Promise<api.ProposalFile[]> { |
|
| 109 | + | return this.request("GET", `/repos/${owner}/${name}/proposals/${number}/files`); |
|
| 110 | + | } |
|
| 111 | + | ||
| 112 | + | /** A file's text on one side of a proposal; undefined when it does not exist there. */ |
|
| 113 | + | async proposalContent(owner: string, name: string, number: number, side: "base" | "head", path: string): Promise<string | undefined> { |
|
| 114 | + | const headers: Record<string, string> = {}; |
|
| 115 | + | if (this.token) { |
|
| 116 | + | headers.authorization = `Bearer ${this.token}`; |
|
| 117 | + | } |
|
| 118 | + | const res = await this.fetchImpl(`${this.base}/api/v1/repos/${owner}/${name}/proposals/${number}/content/${side}/${path.split("/").map(encodeURIComponent).join("/")}`, { headers }); |
|
| 119 | + | if (res.status === 404) { |
|
| 120 | + | return undefined; |
|
| 121 | + | } |
|
| 122 | + | if (!res.ok) { |
|
| 123 | + | throw new HubError(res.status, res.statusText, await res.text()); |
|
| 124 | + | } |
|
| 125 | + | return res.text(); |
|
| 126 | + | } |
|
| 127 | + | ||
| 107 | 128 | proposalChecks(owner: string, name: string, number: number): Promise<api.Check[]> { |
|
| 108 | 129 | return this.request("GET", `/repos/${owner}/${name}/proposals/${number}/checks`); |
|
| 109 | 130 | } |
|
| … | 73 unchanged lines |
||
| 183 | 204 | const mine = p.approvals.find((a) => a.account === username); |
|
| 184 | 205 | return !mine || !mine.sealed; |
|
| 185 | 206 | } |
|
| 207 | + | ||
| 208 | + | /** Which proposal, side and file a diff document shows. */ |
|
| 209 | + | export interface DiffRef { |
|
| 210 | + | base: string; |
|
| 211 | + | owner: string; |
|
| 212 | + | name: string; |
|
| 213 | + | number: number; |
|
| 214 | + | side: "base" | "head"; |
|
| 215 | + | path: string; |
|
| 216 | + | } |
|
| 217 | + | ||
| 218 | + | /** The query string of a diff document's URI. */ |
|
| 219 | + | export function encodeDiffQuery(r: DiffRef): string { |
|
| 220 | + | return new URLSearchParams({ hub: r.base, owner: r.owner, name: r.name, number: String(r.number), side: r.side }).toString(); |
|
| 221 | + | } |
|
| 222 | + | ||
| 223 | + | /** Parse a diff document's query and path back; undefined if it is not one. */ |
|
| 224 | + | export function decodeDiffQuery(query: string, path: string): DiffRef | undefined { |
|
| 225 | + | const q = new URLSearchParams(query); |
|
| 226 | + | const number = Number(q.get("number")); |
|
| 227 | + | const side = q.get("side"); |
|
| 228 | + | const [base, owner, name] = [q.get("hub"), q.get("owner"), q.get("name")]; |
|
| 229 | + | if (!base || !owner || !name || !Number.isInteger(number) || number < 1 || (side !== "base" && side !== "head")) { |
|
| 230 | + | return undefined; |
|
| 231 | + | } |
|
| 232 | + | return { base, owner, name, number, side, path: path.replace(/^\/+/, "") }; |
|
| 233 | + | } |
modified src/hubapi.d.ts +9 −0
| … | 150 unchanged lines |
||
| 151 | 151 | body: string; |
|
| 152 | 152 | created_ms: number; |
|
| 153 | 153 | id: number; |
|
| 154 | + | line?: number | null; |
|
| 154 | 155 | path?: string | null; |
|
| 155 | 156 | resolved: boolean; |
|
| 156 | 157 | }; |
|
| 157 | 158 | ||
| 159 | + | export type ProposalFile = { |
|
| 160 | + | added: number; |
|
| 161 | + | from?: string | null; |
|
| 162 | + | kind: string; |
|
| 163 | + | path: string; |
|
| 164 | + | removed: number; |
|
| 165 | + | }; |
|
| 166 | + | ||
| 158 | 167 | export type Review = { |
|
| 159 | 168 | author: string; |
|
| 160 | 169 | body: string; |
|
| … | 227 unchanged lines |
modified src/proposals.ts +47 −3
| … | 15 unchanged lines |
||
| 16 | 16 | remote: string; |
|
| 17 | 17 | } |
|
| 18 | 18 | ||
| 19 | + | /** Just enough to name a repository on a hub. */ |
|
| 20 | + | export type RepoRef = Pick<HubRepo, "base" | "owner" | "name">; |
|
| 21 | + | ||
| 19 | 22 | /** Find the hub remotes of every repository the Oxygen extension knows. */ |
|
| 20 | 23 | export async function hubRepos(oxygen: OxygenAPI, auth: HubAuth): Promise<HubRepo[]> { |
|
| 21 | 24 | const out: HubRepo[] = []; |
|
| … | 16 unchanged lines |
||
| 38 | 41 | return out; |
|
| 39 | 42 | } |
|
| 40 | 43 | ||
| 41 | - | type Node = { kind: "repo"; repo: HubRepo } | { kind: "bucket"; repo: HubRepo; label: string; items: api.Proposal[] } | { kind: "proposal"; repo: HubRepo; p: api.Proposal }; |
|
| 44 | + | type Node = |
|
| 45 | + | | { kind: "repo"; repo: HubRepo } |
|
| 46 | + | | { kind: "bucket"; repo: HubRepo; label: string; items: api.Proposal[] } |
|
| 47 | + | | { kind: "proposal"; repo: HubRepo; p: api.Proposal } |
|
| 48 | + | | { kind: "file"; repo: HubRepo; p: api.Proposal; f: api.ProposalFile }; |
|
| 49 | + | ||
| 50 | + | const FILE_ICONS: Record<string, string> = { added: "diff-added", removed: "diff-removed", renamed: "diff-renamed", modified: "diff-modified" }; |
|
| 42 | 51 | ||
| 43 | 52 | export class ProposalsProvider implements vscode.TreeDataProvider<Node> { |
|
| 44 | 53 | private readonly emitter = new vscode.EventEmitter<void>(); |
|
| 45 | 54 | readonly onDidChangeTreeData = this.emitter.event; |
|
| 46 | 55 | private cache = new Map<string, api.Proposal[]>(); |
|
| 56 | + | private files = new Map<string, api.ProposalFile[]>(); |
|
| 47 | 57 | ||
| 48 | 58 | constructor( |
|
| 49 | 59 | private readonly oxygen: () => OxygenAPI | undefined, |
|
| … | 2 unchanged lines |
||
| 52 | 62 | ||
| 53 | 63 | refresh(): void { |
|
| 54 | 64 | this.cache.clear(); |
|
| 65 | + | this.files.clear(); |
|
| 55 | 66 | this.emitter.fire(); |
|
| 56 | 67 | } |
|
| 57 | 68 | ||
| … | 13 unchanged lines |
||
| 71 | 82 | return list; |
|
| 72 | 83 | } |
|
| 73 | 84 | ||
| 85 | + | /** The files a proposal changes, cached until the next refresh. */ |
|
| 86 | + | async filesOf(repo: HubRepo, p: api.Proposal): Promise<api.ProposalFile[]> { |
|
| 87 | + | const key = `${repo.base}/${repo.owner}/${repo.name}#${p.number}`; |
|
| 88 | + | const hit = this.files.get(key); |
|
| 89 | + | if (hit) { |
|
| 90 | + | return hit; |
|
| 91 | + | } |
|
| 92 | + | const list = await this.auth.clientFor(repo.base).proposalFiles(repo.owner, repo.name, p.number); |
|
| 93 | + | this.files.set(key, list); |
|
| 94 | + | return list; |
|
| 95 | + | } |
|
| 96 | + | ||
| 74 | 97 | getTreeItem(node: Node): vscode.TreeItem { |
|
| 75 | 98 | switch (node.kind) { |
|
| 76 | 99 | case "repo": { |
|
| … | 9 unchanged lines |
||
| 86 | 109 | } |
|
| 87 | 110 | case "proposal": { |
|
| 88 | 111 | const p = node.p; |
|
| 89 | - | const item = new vscode.TreeItem(`#${p.number} ${p.title}`, vscode.TreeItemCollapsibleState.None); |
|
| 112 | + | const item = new vscode.TreeItem(`#${p.number} ${p.title}`, vscode.TreeItemCollapsibleState.Collapsed); |
|
| 90 | 113 | const sealed = p.approvals.filter((a) => a.sealed).length; |
|
| 91 | 114 | const chips = [p.state === "draft" ? "draft" : "", p.conflicted ? "conflicts" : "", p.landable ? "landable" : "", sealed ? `${sealed} sealed` : ""].filter(Boolean).join(" · "); |
|
| 92 | 115 | item.description = `${p.author} → ${p.target}${chips ? " · " + chips : ""}`; |
|
| … | 3 unchanged lines |
||
| 96 | 119 | item.command = { title: "Open", command: "oxygenhub.openProposal", arguments: [node] }; |
|
| 97 | 120 | return item; |
|
| 98 | 121 | } |
|
| 122 | + | case "file": { |
|
| 123 | + | const f = node.f; |
|
| 124 | + | const slash = f.path.lastIndexOf("/"); |
|
| 125 | + | const item = new vscode.TreeItem(f.path.slice(slash + 1), vscode.TreeItemCollapsibleState.None); |
|
| 126 | + | const dir = slash > 0 ? f.path.slice(0, slash) : ""; |
|
| 127 | + | item.description = [dir, f.from ? `from ${f.from}` : "", f.added || f.removed ? `+${f.added} −${f.removed}` : f.kind].filter(Boolean).join(" · "); |
|
| 128 | + | item.tooltip = `${f.path} (${f.kind})`; |
|
| 129 | + | item.iconPath = new vscode.ThemeIcon(FILE_ICONS[f.kind] ?? "diff"); |
|
| 130 | + | item.contextValue = "proposalFile"; |
|
| 131 | + | item.command = { title: "Open Diff", command: "oxygenhub.openProposalFile", arguments: [node] }; |
|
| 132 | + | return item; |
|
| 133 | + | } |
|
| 99 | 134 | } |
|
| 100 | 135 | } |
|
| 101 | 136 | ||
| … | 23 unchanged lines |
||
| 125 | 160 | if (node.kind === "bucket") { |
|
| 126 | 161 | return node.items.map((p) => ({ kind: "proposal", repo: node.repo, p })); |
|
| 127 | 162 | } |
|
| 163 | + | if (node.kind === "proposal") { |
|
| 164 | + | try { |
|
| 165 | + | const files = await this.filesOf(node.repo, node.p); |
|
| 166 | + | return files.map((f) => ({ kind: "file", repo: node.repo, p: node.p, f })); |
|
| 167 | + | } catch (e) { |
|
| 168 | + | void vscode.window.showWarningMessage(`Oxygenhub: ${(e as Error).message}`); |
|
| 169 | + | } |
|
| 170 | + | } |
|
| 128 | 171 | return []; |
|
| 129 | 172 | } |
|
| 130 | 173 | } |
|
| 131 | 174 | ||
| 132 | 175 | export type ProposalNode = Extract<Node, { kind: "proposal" }>; |
|
| 176 | + | export type ProposalFileNode = Extract<Node, { kind: "file" }>; |
|
| 133 | 177 | ||
| 134 | 178 | /** The detail webview. */ |
|
| 135 | 179 | export function showDetail(repo: HubRepo, p: api.Proposal, checks: api.Check[]): void { |
|
| … | 8 unchanged lines |
||
| 144 | 188 | panel.webview.html = `<!doctype html><html><body style="font-family: var(--vscode-font-family); padding: 12px"> |
|
| 145 | 189 | <h2>#${p.number} ${esc(p.title)} <small>${p.state}</small></h2> |
|
| 146 | 190 | <p>${esc(p.author)} wants <b>${esc(JSON.stringify(p.source))}</b> into <b>${esc(p.target)}</b> · ${p.changes.length} change(s)${p.conflicted ? " · <b>conflicts</b>" : ""}</p> |
|
| 147 | - | <p><a href="${url}">Open on hub</a> · <a href="command:oxygenhub.checkoutProposal?${arg}">Check out</a> · <a href="command:oxygenhub.review?${arg}">Review</a> · <a href="command:oxygenhub.sealApproval?${arg}">Seal approval</a> · <a href="command:oxygenhub.land?${arg}">Land</a></p> |
|
| 191 | + | <p><a href="${url}">Open on hub</a> · <a href="command:oxygenhub.openProposalChanges?${arg}">View changes</a> · <a href="command:oxygenhub.checkoutProposal?${arg}">Check out</a> · <a href="command:oxygenhub.review?${arg}">Review</a> · <a href="command:oxygenhub.sealApproval?${arg}">Seal approval</a> · <a href="command:oxygenhub.land?${arg}">Land</a></p> |
|
| 148 | 192 | <div>${p.body ? `<pre style="white-space: pre-wrap">${esc(p.body)}</pre>` : ""}</div> |
|
| 149 | 193 | <h3>Landing</h3><ul>${blockers}</ul> |
|
| 150 | 194 | <h3>Approvals</h3><ul>${approvals}</ul> |
|
| … | 5 unchanged lines |
modified src/review.ts +60 −3
| … | 4 unchanged lines |
||
| 5 | 5 | import * as vscode from "vscode"; |
|
| 6 | 6 | import type * as api from "./hubapi"; |
|
| 7 | 7 | import { HubAuth } from "./auth"; |
|
| 8 | - | import type { HubRepo } from "./proposals"; |
|
| 8 | + | import type { HubRepo, RepoRef } from "./proposals"; |
|
| 9 | + | import { diffUri, refOf } from "./diff"; |
|
| 9 | 10 | ||
| 10 | 11 | interface BlameLine { |
|
| 11 | 12 | line: number; |
|
| … | 6 unchanged lines |
||
| 18 | 19 | private readonly controller = vscode.comments.createCommentController("oxygenhub", "Oxygenhub review"); |
|
| 19 | 20 | private threads: vscode.CommentThread[] = []; |
|
| 20 | 21 | private active: { repo: HubRepo; number: number } | undefined; |
|
| 22 | + | /** Threads in diff editors, per proposal (`base/owner/name#n`). */ |
|
| 23 | + | private readonly diffThreads = new Map<string, vscode.CommentThread[]>(); |
|
| 21 | 24 | ||
| 22 | 25 | constructor(private readonly auth: HubAuth) { |
|
| 23 | 26 | this.controller.commentingRangeProvider = { |
|
| 24 | - | provideCommentingRanges: (document) => (this.active && document.uri.fsPath.startsWith(this.active.repo.handle.root) ? [new vscode.Range(0, 0, Math.max(0, document.lineCount - 1), 0)] : []), |
|
| 27 | + | provideCommentingRanges: (document) => { |
|
| 28 | + | const whole = [new vscode.Range(0, 0, Math.max(0, document.lineCount - 1), 0)]; |
|
| 29 | + | // In a proposal's diff, only the projected side takes comments. |
|
| 30 | + | const r = refOf(document.uri); |
|
| 31 | + | if (r) { |
|
| 32 | + | return r.side === "head" ? whole : []; |
|
| 33 | + | } |
|
| 34 | + | return this.active && document.uri.fsPath.startsWith(this.active.repo.handle.root) ? whole : []; |
|
| 35 | + | }, |
|
| 25 | 36 | }; |
|
| 26 | 37 | } |
|
| 27 | 38 | ||
| 39 | + | /** |
|
| 40 | + | * Show a proposal's threads in its diff documents: every anchored |
|
| 41 | + | * comment on the projected side, at the line the hub places it. |
|
| 42 | + | */ |
|
| 43 | + | async loadDiff(repo: RepoRef, number: number): Promise<void> { |
|
| 44 | + | const key = `${repo.base}/${repo.owner}/${repo.name}#${number}`; |
|
| 45 | + | this.diffThreads.get(key)?.forEach((t) => t.dispose()); |
|
| 46 | + | const comments = await this.auth.clientFor(repo.base).proposalComments(repo.owner, repo.name, number); |
|
| 47 | + | const byLine = new Map<string, api.Comment[]>(); |
|
| 48 | + | for (const c of comments) { |
|
| 49 | + | if (c.path && c.line) { |
|
| 50 | + | const k = `${c.path}\n${c.line}`; |
|
| 51 | + | byLine.set(k, [...(byLine.get(k) ?? []), c]); |
|
| 52 | + | } |
|
| 53 | + | } |
|
| 54 | + | const threads: vscode.CommentThread[] = []; |
|
| 55 | + | for (const [k, cs] of byLine) { |
|
| 56 | + | const [path, line] = k.split("\n"); |
|
| 57 | + | const thread = this.controller.createCommentThread(diffUri(repo, number, "head", path), new vscode.Range(Number(line) - 1, 0, Number(line) - 1, 0), cs.map(toComment)); |
|
| 58 | + | thread.label = cs.every((c) => c.resolved) ? "resolved" : "review"; |
|
| 59 | + | thread.state = cs.every((c) => c.resolved) ? vscode.CommentThreadState.Resolved : vscode.CommentThreadState.Unresolved; |
|
| 60 | + | thread.collapsibleState = vscode.CommentThreadCollapsibleState.Expanded; |
|
| 61 | + | threads.push(thread); |
|
| 62 | + | } |
|
| 63 | + | this.diffThreads.set(key, threads); |
|
| 64 | + | } |
|
| 65 | + | ||
| 28 | 66 | /** Show the threads of a proposal; call after checking it out. */ |
|
| 29 | 67 | async load(repo: HubRepo, number: number): Promise<void> { |
|
| 30 | 68 | this.clear(); |
|
| … | 36 unchanged lines |
||
| 67 | 105 | ||
| 68 | 106 | /** Post a reply to a thread, anchored to the line it sits on. */ |
|
| 69 | 107 | async reply(reply: vscode.CommentReply): Promise<void> { |
|
| 108 | + | const r = refOf(reply.thread.uri); |
|
| 109 | + | if (r) { |
|
| 110 | + | const line = (reply.thread.range?.start.line ?? 0) + 1; |
|
| 111 | + | const repo: RepoRef = { base: r.base, owner: r.owner, name: r.name }; |
|
| 112 | + | try { |
|
| 113 | + | await postLineComment(this.auth, repo, r.number, r.path, line, reply.text); |
|
| 114 | + | } catch (e) { |
|
| 115 | + | void vscode.window.showErrorMessage(`Oxygenhub: ${(e as Error).message}`); |
|
| 116 | + | return; |
|
| 117 | + | } |
|
| 118 | + | // A new thread from the gutter is replaced by the hub's copy. |
|
| 119 | + | if (!reply.thread.comments.length) { |
|
| 120 | + | reply.thread.dispose(); |
|
| 121 | + | } |
|
| 122 | + | await this.loadDiff(repo, r.number); |
|
| 123 | + | return; |
|
| 124 | + | } |
|
| 70 | 125 | const a = this.active; |
|
| 71 | 126 | if (!a) { |
|
| 72 | 127 | return; |
|
| … | 13 unchanged lines |
||
| 86 | 141 | ||
| 87 | 142 | dispose(): void { |
|
| 88 | 143 | this.clear(); |
|
| 144 | + | this.diffThreads.forEach((ts) => ts.forEach((t) => t.dispose())); |
|
| 145 | + | this.diffThreads.clear(); |
|
| 89 | 146 | this.controller.dispose(); |
|
| 90 | 147 | } |
|
| 91 | 148 | } |
|
| … | 3 unchanged lines |
||
| 95 | 152 | } |
|
| 96 | 153 | ||
| 97 | 154 | /** A comment anchored to the atom at `path:line` of the projected state. */ |
|
| 98 | - | async function postLineComment(auth: HubAuth, repo: HubRepo, number: number, path: string, line: number, body: string): Promise<void> { |
|
| 155 | + | async function postLineComment(auth: HubAuth, repo: RepoRef, number: number, path: string, line: number, body: string): Promise<void> { |
|
| 99 | 156 | await auth.clientFor(repo.base).commentProposal(repo.owner, repo.name, number, body, path, line); |
|
| 100 | 157 | } |
modified test/hub.test.mjs +28 −1
| … | 5 unchanged lines |
||
| 6 | 6 | import { mkdtempSync, writeFileSync, existsSync, mkdirSync } from "node:fs"; |
|
| 7 | 7 | import { tmpdir } from "node:os"; |
|
| 8 | 8 | import { join } from "node:path"; |
|
| 9 | - | import { HubClient, HubError } from "../out/hub.mjs"; |
|
| 9 | + | import { HubClient, HubError, encodeDiffQuery, decodeDiffQuery } from "../out/hub.mjs"; |
|
| 10 | 10 | ||
| 11 | 11 | const HUB = join(import.meta.dirname, "../../oxygenhub/target/release/oxygenhub"); |
|
| 12 | 12 | const OXY = join(import.meta.dirname, "../../oxygen/target/release/oxy"); |
|
| … | 33 unchanged lines |
||
| 46 | 46 | oxy("bookmark", "create", "feat"); |
|
| 47 | 47 | oxy("bookmark", "switch", "feat"); |
|
| 48 | 48 | writeFileSync(join(work, "src/b.txt"), "b\n"); |
|
| 49 | + | writeFileSync(join(work, "src/a.txt"), "a\nA\n"); |
|
| 49 | 50 | oxy("commit", "-m", "feat"); |
|
| 50 | 51 | oxy("push", "origin", "--bookmark", "feat"); |
|
| 51 | 52 | const p = await client.createProposal("andy", "demo", { title: "Feat", target: "main", source: "feat", body: "", draft: false }); |
|
| 52 | 53 | assert.equal(p.number, 1); |
|
| 53 | 54 | assert.equal(p.changes.length, 1); |
|
| 55 | + | // The proposal's files and both sides, as the diff editors fetch them. |
|
| 56 | + | const files = await client.proposalFiles("andy", "demo", 1); |
|
| 57 | + | assert.deepEqual( |
|
| 58 | + | files.map((f) => [f.path, f.kind]), |
|
| 59 | + | [ |
|
| 60 | + | ["src/a.txt", "modified"], |
|
| 61 | + | ["src/b.txt", "added"], |
|
| 62 | + | ], |
|
| 63 | + | ); |
|
| 64 | + | assert.equal(await client.proposalContent("andy", "demo", 1, "base", "src/a.txt"), "a\n"); |
|
| 65 | + | assert.equal(await client.proposalContent("andy", "demo", 1, "head", "src/a.txt"), "a\nA\n"); |
|
| 66 | + | assert.equal(await client.proposalContent("andy", "demo", 1, "base", "src/b.txt"), undefined, "absent on the target side"); |
|
| 67 | + | // A review comment from the diff comes back on its line. |
|
| 68 | + | const c = await client.commentProposal("andy", "demo", 1, "why?", "src/a.txt", 2); |
|
| 69 | + | assert.equal(c.line, 2); |
|
| 70 | + | const comments = await client.proposalComments("andy", "demo", 1); |
|
| 71 | + | assert.equal(comments[0].line, 2); |
|
| 54 | 72 | const list = await client.proposals("andy", "demo"); |
|
| 55 | 73 | assert.equal(list[0].title, "Feat"); |
|
| 56 | 74 | const issue = await client.createIssue("andy", "demo", "Bug", "text"); |
|
| … | 8 unchanged lines |
||
| 65 | 83 | proc.kill(); |
|
| 66 | 84 | } |
|
| 67 | 85 | }); |
|
| 86 | + | ||
| 87 | + | test("diff document queries round trip", () => { |
|
| 88 | + | const ref = { base: "https://earth.example:7343", owner: "andy", name: "demo", number: 7, side: "head", path: "src/dir with space/a.txt" }; |
|
| 89 | + | const back = decodeDiffQuery(encodeDiffQuery(ref), "/" + ref.path); |
|
| 90 | + | assert.deepEqual(back, ref); |
|
| 91 | + | assert.equal(decodeDiffQuery("hub=x&owner=a&name=b&number=0&side=head", "/p"), undefined, "number must be positive"); |
|
| 92 | + | assert.equal(decodeDiffQuery("hub=x&owner=a&name=b&number=3&side=left", "/p"), undefined, "side is base or head"); |
|
| 93 | + | assert.equal(decodeDiffQuery("owner=a&name=b&number=3&side=head", "/p"), undefined, "the hub is required"); |
|
| 94 | + | }); |