Public: anyone can read this; the crew can change it.

The terminal's issues list shows the pillar each issue serves, or Later, and `P` narrows the list to one pillar #7342

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

scout/forge:scout/7326-the-terminal-s-issues-list-shows-the-pil → main 2 commits fork

  • Merged. scout/7326-the-terminal-s-issues-list-shows-the-pil was merged into main as bb1a130.

Review

Sign in to review this change.
2 files changed +218 −9
src/tui/screens/issues.ts +80 −9
@@ -11,6 +11,8 @@
1111
1212 import { openIssueThrough } from "../../actions/issues.ts";
1313 import type { IssueKind, Label } from "../../core/issues.ts";
14 import { getIssuesByNumbers, numberKey } from "../../core/issues.ts";
15 import { LATER, pillarsOf, servedBy, type Pillar } from "../../core/pillars.ts";
1416 import { mergedPullNumbers } from "../../core/pulls.ts";
1517 import { repoDir } from "../../git/repos.ts";
1618 import type { Repo } from "../../core/repos.ts";
@@ -53,6 +55,8 @@ import {
5355
5456 const STATES = ["open", "closed", "all"] as const;
5557 const PAGE = 40;
58 /** Rows a narrowed list should have before it stops reading ahead. */
59 const FILLS = 20;
5660
5761 export function issuesScreen(
5862 f: Session,
@@ -88,6 +92,46 @@ export function issuesScreen(
8892 * set: one grouped query for the whole page, never one per row.
8993 */
9094 const verdicts = new Map<number, Verdict>();
95 /**
96 * The pillars of this repository and the one each row's issue serves,
97 * read once per reload: `servedBy` answers for the whole repository in two
98 * queries, so a row costs a map lookup. Empty on a live browse, on a pull
99 * list, and where no umbrella exists — and then there is no tag and no `P`.
100 */
101 let pillars: readonly Pillar[] = [];
102 const tags = new Map<number, string>();
103 /** Whether the repository defers work: then `later` is a stop of its own. */
104 let defers = false;
105 /** 0 is `all`; n is `pillars[n - 1]`; one past the pillars is `later`. */
106 let pillar = 0;
107 const stops = (): number => pillars.length + (defers ? 1 : 0);
108 /** The tag a stop keeps: a pillar's short name, or `later`; undefined for `all`. */
109 const stopTag = (): string | undefined =>
110 pillar === 0 ? undefined : (pillars[pillar - 1]?.short ?? "later");
111 const readPillars = (rows: readonly IssueSummary[]): void => {
112 if (repo === undefined || kind !== "issue") return;
113 const set = pillarsOf(f.db, repo);
114 pillars = set.pillars;
115 defers = set.gate !== undefined || set.later !== undefined;
116 if (pillar > stops()) pillar = 0;
117 if (pillars.length === 0) return;
118 const served = servedBy(f.db, set);
119 const byNumber = getIssuesByNumbers(
120 f.db,
121 rows.map((row) => ({ repoId: repo.id, number: row.number })),
122 );
123 for (const row of rows) {
124 const id = byNumber.get(numberKey(repo.id, row.number))?.id;
125 const name = id === undefined ? undefined : served.get(id);
126 if (name === undefined) continue;
127 tags.set(
128 row.number,
129 name === LATER
130 ? "later"
131 : (set.pillars.find((p) => p.name === name)?.short ?? ""),
132 );
133 }
134 };
91135 const markMerged = async (rows: readonly IssueSummary[]): Promise<void> => {
92136 if (repo === undefined || kind !== "pull") return;
93137 // One query for the page, addressed the way the list has them: by
@@ -129,8 +173,11 @@ export function issuesScreen(
129173 const reload = async (): Promise<void> => {
130174 merged.clear();
131175 verdicts.clear();
176 tags.clear();
132177 items = await take();
133178 loaded = true;
179 readPillars(items);
180 await topUp();
134181 await markMerged(items);
135182 if (!more) counted.set(STATES[segment]!, items.length);
136183 state.clamp(items.length);
@@ -148,21 +195,35 @@ export function issuesScreen(
148195 ctx.invalidate();
149196 };
150197
151 const loadMore = (): void => {
198 const nextPage = async (): Promise<void> => {
152199 const at = cursor;
153200 if (at === undefined) return;
154 runAsync(ctx, async () => {
155 const page = await take(at);
156 await markMerged(page);
157 items = [...items, ...page];
158 if (!more) counted.set(STATES[segment]!, items.length);
159 ctx.invalidate();
160 });
201 const page = await take(at);
202 readPillars(page);
203 await markMerged(page);
204 items = [...items, ...page];
205 if (!more) counted.set(STATES[segment]!, items.length);
206 ctx.invalidate();
207 };
208
209 /**
210 * A pillar narrows only what is loaded, so with one on the pages keep
211 * coming until the narrowed list fills a screen or the pages run out;
212 * a short list never hides matches waiting on the next page.
213 */
214 const topUp = async (): Promise<void> => {
215 while (pillar !== 0 && more && shown().length < FILLS) await nextPage();
216 };
217
218 const loadMore = (): void => {
219 runAsync(ctx, nextPage);
161220 };
162221
163222 const shown = (): IssueSummary[] =>
164223 items.filter(
165 (i) => search.matches(i.title) || search.matches(`#${i.number}`),
224 (i) =>
225 (pillar === 0 || tags.get(i.number) === stopTag()) &&
226 (search.matches(i.title) || search.matches(`#${i.number}`)),
166227 );
167228
168229 const noun = kind === "pull" ? "pull request" : "issue";
@@ -198,6 +259,7 @@ export function issuesScreen(
198259 binding(["enter"], "open"),
199260 binding(["tab"], "open/closed/all"),
200261 binding(["l"], "label"),
262 ...(stops() > 0 ? [binding(["P"], "pillar")] : []),
201263 binding(["a"], mine ? "all authors" : "only mine"),
202264 ...(mayWrite() ? [binding(["c"], `new ${noun}`)] : []),
203265 binding(["/"], "search"),
@@ -236,6 +298,8 @@ export function issuesScreen(
236298 const chips = [
237299 label === undefined ? "" : `label:${label}`,
238300 mine ? "author:me" : "",
301 pillar === 0 ? "" : `pillar:${stopTag()}`,
302 pillar !== 0 && more ? "(more not loaded)" : "",
239303 ]
240304 .filter((t) => t !== "")
241305 .join(" ");
@@ -283,6 +347,7 @@ export function issuesScreen(
283347 // login from another forge must never read as an account here.
284348 meta: metaOf(
285349 verdictMark(item),
350 tags.get(item.number) ?? "",
286351 `#${item.number} ${
287352 item.originAuthor === null || item.originAuthor === ""
288353 ? item.authorName
@@ -321,6 +386,12 @@ export function issuesScreen(
321386 runAsync(ctx, reload);
322387 return true;
323388 }
389 if (e.key === "P" && stops() > 0) {
390 pillar = (pillar + 1) % (stops() + 1);
391 state.clamp(shown().length);
392 runAsync(ctx, topUp);
393 return true;
394 }
324395 if (e.key === "l") {
325396 ctx.push(
326397 pickerScreen(ctx, screen, {
test/tui-issues-pillar.test.ts added +138
@@ -0,0 +1,138 @@
1 /** The issues list tags each row with its pillar (or `later`), and `P` narrows the list to one pillar. */
2
3 import { afterAll, beforeAll, expect, test } from "bun:test";
4 import { mkdtemp, rm } from "node:fs/promises";
5 import { tmpdir } from "node:os";
6 import { join } from "node:path";
7 import { openIssue } from "../src/actions/issues.ts";
8 import { createRepo } from "../src/actions/repos.ts";
9 import { createApp, type App as Forge } from "../src/app.ts";
10 import { addDependency } from "../src/core/dependencies.ts";
11 import type { Issue } from "../src/core/issues.ts";
12 import type { Repo } from "../src/core/repos.ts";
13 import type { User } from "../src/core/users.ts";
14 import { createForgeApp } from "../src/tui/forge.ts";
15 import { issuesScreen } from "../src/tui/screens/issues.ts";
16 import { local } from "../src/tui/screens/where.ts";
17 import { headlessTerminal } from "../src/tui/term/terminal.ts";
18 import { seedAdmin } from "./helpers/admin.ts";
19 import { quietLogs } from "./helpers/globals.ts";
20 import { settleApp } from "./helpers/tui.ts";
21
22 quietLogs();
23
24 let dir: string;
25 let forge: Forge;
26 let alice: User;
27
28 beforeAll(async () => {
29 dir = await mkdtemp(join(tmpdir(), "forge-tui-issues-pillar-"));
30 forge = createApp({ dataDir: dir, port: 0 });
31 const a = await seedAdmin(forge, {
32 username: "alice",
33 email: "[email protected]",
34 password: "a-long-passphrase",
35 });
36 if (!a.ok) throw new Error("signup failed");
37 alice = a.value;
38 });
39 afterAll(async () => {
40 forge.close?.();
41 await rm(dir, { recursive: true, force: true });
42 });
43
44 const make = async (repo: Repo, title: string): Promise<Issue> => {
45 const r = await openIssue(forge, alice, repo, { title, body: "x" });
46 if (!r.ok) throw new Error(`issue failed: ${r.error}`);
47 return r.value;
48 };
49
50 const open = async (repo: Repo) => {
51 const { app, session } = createForgeApp(
52 forge,
53 { sshUsername: "alice", remoteAddress: "127.0.0.1", user: alice },
54 { terminal: headlessTerminal(120, 30), exitHooks: false },
55 );
56 app.start();
57 await settleApp(app);
58 app.push(issuesScreen(session, app.ctx, local(session, repo), "issue"));
59 await settleApp(app);
60 const press = async (key: string): Promise<void> => {
61 app.dispatch({ type: "key", key, ctrl: false, alt: false, shift: false });
62 await settleApp(app);
63 };
64 return { app, press, text: () => app.screen.toText() };
65 };
66
67 test("a row carries its pillar, P narrows the list, and a repository without umbrellas has neither", async () => {
68 const plain = await createRepo(forge, alice, { name: "plain", visibility: "public" });
69 if (!plain.ok) throw new Error("repo failed");
70 await make(plain.value, "Loose work");
71 const bare = await open(plain.value);
72 expect(bare.text()).toContain("Loose work");
73 expect(bare.text()).not.toMatch(/\bP0\b|later/);
74 expect(bare.text()).not.toContain("P pillar");
75 await bare.press("P");
76 expect(bare.text()).toContain("Loose work");
77 expect(bare.text()).not.toContain("pillar:");
78 bare.app.stop();
79
80 const r = await createRepo(forge, alice, { name: "plan", visibility: "public" });
81 if (!r.ok) throw new Error("repo failed");
82 const repo = r.value;
83 const trust = await make(repo, "P0 Trust: be trusted");
84 const speed = await make(repo, "P1 Speed: be quick");
85 const gate = await make(repo, "Later gate: not yet");
86 const a = await make(repo, "Sign the thing");
87 const b = await make(repo, "Cache the thing");
88 const c = await make(repo, "Defer the thing");
89 await make(repo, "Unplaced thing");
90 addDependency(forge.db, trust, a, alice.id);
91 addDependency(forge.db, speed, b, alice.id);
92 addDependency(forge.db, c, gate, alice.id);
93
94 const h = await open(repo);
95 const row = (title: string): string =>
96 h.text().split("\n").find((l) => l.includes(title)) ?? "";
97 expect(row("Sign the thing")).toContain("P0");
98 expect(row("Cache the thing")).toContain("P1");
99 expect(row("Defer the thing")).toContain("later");
100 expect(row("Unplaced thing")).not.toMatch(/P\d|later/);
101 expect(h.text()).toContain("P pillar");
102
103 await h.press("P");
104 expect(h.text()).toContain("pillar:P0");
105 expect(h.text()).toContain("Sign the thing");
106 expect(h.text()).not.toContain("Cache the thing");
107 await h.press("P");
108 expect(h.text()).toContain("pillar:P1");
109 expect(h.text()).toContain("Cache the thing");
110 expect(h.text()).not.toContain("Sign the thing");
111 await h.press("P");
112 expect(h.text()).toContain("pillar:later");
113 expect(h.text()).toContain("Defer the thing");
114 expect(h.text()).not.toContain("Cache the thing");
115 await h.press("P");
116 expect(h.text()).not.toContain("pillar:");
117 expect(h.text()).toContain("Sign the thing");
118 expect(h.text()).toContain("Cache the thing");
119 h.app.stop();
120 });
121
122 test("with a pillar on, pages past the first are read until its issues show", async () => {
123 const r = await createRepo(forge, alice, { name: "paged", visibility: "public" });
124 if (!r.ok) throw new Error("repo failed");
125 const repo = r.value;
126 const trust = await make(repo, "P0 Trust: be trusted");
127 const old = await make(repo, "Oldest trusted thing");
128 addDependency(forge.db, trust, old, alice.id);
129 for (let n = 0; n < 45; n++) await make(repo, `Filler ${n}`);
130
131 const h = await open(repo);
132 // Newest first: the pillar's issue is on the second page.
133 expect(h.text()).not.toContain("Oldest trusted thing");
134 await h.press("P");
135 expect(h.text()).toContain("pillar:P0");
136 expect(h.text()).toContain("Oldest trusted thing");
137 h.app.stop();
138 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs