Public: anyone can read this; the crew can change it.
An admin deletes a label from the terminal's Labels screen with `d` after a confirm #7967
Check · Review · Land · Live done — Landed and live in production.
mason/forge:mason/7843-an-admin-deletes-a-label-from-the-termin → main 2 commits fork 2 threads · 2 unresolved
3 files
changed +99
−12
docs/APPS.md +1 −1
| @@ -149,7 +149,7 @@ Under the box, four starter sentences are plain links (no script): | ||
| 149 | 149 | one opens the run's log; a run on work you may not see is left out), |
| 150 | 150 | *Tell the crew*, *Report a problem* (one text, an issue |
| 151 | 151 | titled by its first line, through `openIssueThrough`) and *Make it yours* |
| 152 | (for those who cannot write the repository: `Y` on the repository screen, or the palette row, asks for a name, defaulting to the original's, and a visibility, `tab` to switch, then shows the plan and copies only after a confirm, and the copy opens), *Labels* (its labels with their hex; an admin presses `n`, types a name and six hex digits, `bug d73a4a`, the `#` optional, and the list shows the new label at once — a reader is not offered `n`), and *Vision* | |
| 152 | (for those who cannot write the repository: `Y` on the repository screen, or the palette row, asks for a name, defaulting to the original's, and a visibility, `tab` to switch, then shows the plan and copies only after a confirm, and the copy opens), *Labels* (its labels with their hex; an admin presses `n`, types a name and six hex digits, `bug d73a4a`, the `#` optional, and the list shows the new label at once — `d` on a label asks first, naming it, with Keep the default, then deletes it; a reader is offered neither `n` nor `d`), and *Vision* | |
| 153 | 153 | (docs/VISION.md on the default branch: the purpose, each pillar numbered, |
| 154 | 154 | the rules, through the one read MCP `vision` makes; when there is none the |
| 155 | 155 | terminal says so in one line of its own, where MCP answers an empty `order`; |
src/tui/screens/labels.ts +36 −9
| @@ -7,14 +7,14 @@ | ||
| 7 | 7 | |
| 8 | 8 | import { LABEL_WORDS } from "../../actions/label_words.ts"; |
| 9 | 9 | import { requireRepoAdmin } from "../../actions/access.ts"; |
| 10 | import { createLabel } from "../../actions/repos.ts"; | |
| 10 | import { createLabel, deleteLabel } from "../../actions/repos.ts"; | |
| 11 | 11 | import { canRead, type Repo } from "../../core/repos.ts"; |
| 12 | 12 | import { listLabels, type Label } from "../../core/issues.ts"; |
| 13 | 13 | import type { Ctx, Screen } from "../app/app.ts"; |
| 14 | 14 | import { binding } from "../app/keys.ts"; |
| 15 | 15 | import type { Command } from "../app/palette.ts"; |
| 16 | 16 | import { style } from "../term/style.ts"; |
| 17 | import { ListState, promptScreen } from "./common.ts"; | |
| 17 | import { ListState, confirmScreen, promptScreen } from "./common.ts"; | |
| 18 | 18 | import { labelChip } from "../ui/widgets.ts"; |
| 19 | 19 | import type { Session } from "../session.ts"; |
| 20 | 20 | |
| @@ -22,11 +22,14 @@ import type { Session } from "../session.ts"; | ||
| 22 | 22 | export const NOT_FOUND_LINE = "No such repository"; |
| 23 | 23 | export const NOT_ALLOWED_LINE = "Only a repository admin can make a label."; |
| 24 | 24 | export const NO_LABELS_LINE = "No labels yet."; |
| 25 | export const DELETED_LINE = "Deleted that label."; | |
| 26 | export const GONE_LINE = "That label is already gone, or you can no longer delete it."; | |
| 25 | 27 | |
| 26 | 28 | export function labelsScreen(f: Session, ctx: Ctx, repo: Repo): Screen { |
| 27 | 29 | let labels: Label[] | undefined; |
| 28 | 30 | let hidden = false; |
| 29 | 31 | const state = new ListState(); |
| 32 | let note = ""; | |
| 30 | 33 | // Only whoever `requireRepoAdmin` admits is offered `n`. |
| 31 | 34 | const admin = (): boolean => requireRepoAdmin(f.app, f.user, repo).ok; |
| 32 | 35 | |
| @@ -42,11 +45,27 @@ export function labelsScreen(f: Session, ctx: Ctx, repo: Repo): Screen { | ||
| 42 | 45 | ctx.invalidate(); |
| 43 | 46 | return undefined; |
| 44 | 47 | }; |
| 48 | const ask = (picked: Label): void => { | |
| 49 | ctx.push( | |
| 50 | confirmScreen(ctx, screen, { | |
| 51 | title: "Delete a label", | |
| 52 | body: [`Delete the label ${picked.name}? Issues carrying it lose it.`], | |
| 53 | actions: [{ label: "Keep", variant: "ghost" }, { label: "Delete", variant: "danger" }], | |
| 54 | onChoose: (i) => { | |
| 55 | if (i < 1) return; | |
| 56 | note = deleteLabel(f.app, f.user, repo, picked.id).ok ? DELETED_LINE : GONE_LINE; | |
| 57 | labels = listLabels(f.db, repo.id); | |
| 58 | state.clamp(labels.length); | |
| 59 | ctx.invalidate(); | |
| 60 | }, | |
| 61 | }), | |
| 62 | ); | |
| 63 | }; | |
| 45 | 64 | const screen: Screen = { |
| 46 | 65 | title: "Labels", |
| 47 | 66 | // Read each time: a demoted admin stops being offered `n`. |
| 48 | 67 | get keys() { |
| 49 | return [binding(["j", "k"], "move"), ...(admin() ? [binding(["n"], "new label")] : []), binding(["esc", "q"], "back")]; | |
| 68 | return [binding(["j", "k"], "move"), ...(admin() ? [binding(["n"], "new label"), binding(["d"], "delete label")] : []), binding(["esc", "q"], "back")]; | |
| 50 | 69 | }, |
| 51 | 70 | async load() { |
| 52 | 71 | // Gated here as well as on the command: the screen is exported. |
| @@ -60,9 +79,15 @@ export function labelsScreen(f: Session, ctx: Ctx, repo: Repo): Screen { | ||
| 60 | 79 | const line = (y: number, text: string) => s.text(area.x + 1, area.y + y, text, style(c.fg, c.bg), right); |
| 61 | 80 | if (hidden) return void line(0, NOT_FOUND_LINE); |
| 62 | 81 | if (labels === undefined) return void line(0, "Reading…"); |
| 63 | if (labels.length === 0) return void line(0, NO_LABELS_LINE); | |
| 64 | const from = state.window(area.h, labels.length); | |
| 65 | for (let i = 0; i < area.h; i++) { | |
| 82 | if (labels.length === 0) { | |
| 83 | line(0, NO_LABELS_LINE); | |
| 84 | if (note !== "") line(area.h - 1, note); | |
| 85 | return; | |
| 86 | } | |
| 87 | const rows = note === "" ? area.h : Math.max(1, area.h - 1); | |
| 88 | if (note !== "") line(area.h - 1, note); | |
| 89 | const from = state.window(rows, labels.length); | |
| 90 | for (let i = 0; i < rows; i++) { | |
| 66 | 91 | const l = labels[from + i]; |
| 67 | 92 | if (l === undefined) break; |
| 68 | 93 | const picked = from + i === state.index; |
| @@ -72,10 +97,13 @@ export function labelsScreen(f: Session, ctx: Ctx, repo: Repo): Screen { | ||
| 72 | 97 | } |
| 73 | 98 | }, |
| 74 | 99 | key(e) { |
| 100 | note = ""; | |
| 75 | 101 | if (e.key === "j" || e.key === "down") state.move(1, labels?.length ?? 0); |
| 76 | 102 | else if (e.key === "k" || e.key === "up") state.move(-1, labels?.length ?? 0); |
| 77 | 103 | else if (e.key === "esc" || e.key === "q") ctx.pop(); |
| 78 | else if (e.key === "n" && !e.ctrl && !e.alt && !hidden && admin()) { | |
| 104 | else if (e.key === "d" && !e.ctrl && !e.alt && !hidden && admin() && labels?.[state.index] !== undefined) { | |
| 105 | ask(labels[state.index]!); | |
| 106 | } else if (e.key === "n" && !e.ctrl && !e.alt && !hidden && admin()) { | |
| 79 | 107 | ctx.push( |
| 80 | 108 | promptScreen(ctx, screen, { |
| 81 | 109 | title: "New label", |
| @@ -83,8 +111,7 @@ export function labelsScreen(f: Session, ctx: Ctx, repo: Repo): Screen { | ||
| 83 | 111 | onSubmit: make, |
| 84 | 112 | }), |
| 85 | 113 | ); |
| 86 | } | |
| 87 | else return false; | |
| 114 | } else return false; | |
| 88 | 115 | ctx.invalidate(); |
| 89 | 116 | return true; |
| 90 | 117 | }, |
test/tui-labels.test.ts +62 −2
| @@ -3,7 +3,7 @@ import { afterAll, beforeAll, expect, test } from "bun:test"; | ||
| 3 | 3 | import { mkdtemp, rm } from "node:fs/promises"; |
| 4 | 4 | import { tmpdir } from "node:os"; |
| 5 | 5 | import { join } from "node:path"; |
| 6 | import { createRepo } from "../src/actions/repos.ts"; | |
| 6 | import { createRepo, deleteLabel } from "../src/actions/repos.ts"; | |
| 7 | 7 | import { createApp, type App } from "../src/app.ts"; |
| 8 | 8 | import { LABEL_WORDS } from "../src/actions/label_words.ts"; |
| 9 | 9 | import { createLabel, listLabels } from "../src/core/issues.ts"; |
| @@ -11,7 +11,7 @@ import type { Repo } from "../src/core/repos.ts"; | ||
| 11 | 11 | import { createUser, type User } from "../src/core/users.ts"; |
| 12 | 12 | import { createForgeApp, Session } from "../src/tui/forge.ts"; |
| 13 | 13 | import type { Ctx } from "../src/tui/app/app.ts"; |
| 14 | import { labelsCommands, labelsScreen, NOT_ALLOWED_LINE, NOT_FOUND_LINE, NO_LABELS_LINE } from "../src/tui/screens/labels.ts"; | |
| 14 | import { DELETED_LINE, GONE_LINE, labelsCommands, labelsScreen, NOT_ALLOWED_LINE, NOT_FOUND_LINE, NO_LABELS_LINE } from "../src/tui/screens/labels.ts"; | |
outdated
Stray blank line in the imports; remove it. |
||
| 15 | 15 | import { repoScreen } from "../src/tui/screens/repo.ts"; |
| 16 | 16 | import { local } from "../src/tui/screens/where.ts"; |
| 17 | 17 | import { headlessTerminal } from "../src/tui/term/terminal.ts"; |
| @@ -220,3 +220,63 @@ test("a # before the colour and a name of several words are accepted", async () | ||
| 220 | 220 | expect(text).toContain("#7057ff"); |
| 221 | 221 | expect(listLabels(app.db, bare.id).map((l) => l.name)).toContain("good first issue"); |
| 222 | 222 | }); |
| 223 | ||
| 224 | async function deleting(repo: Repo, user: User | undefined, keys: string[], beforeLast?: () => void) { | |
| 225 | const terminal = headlessTerminal(100, 30); | |
| 226 | const { app: tui, session } = createForgeApp(app, user ? { ...guest, user } : guest, { terminal, exitHooks: false }); | |
| 227 | tui.start(); | |
| 228 | tui.ctx.push(labelsScreen(session, tui.ctx, repo)); | |
| 229 | await settleApp(tui); | |
| 230 | const press = (key: string) => tui.dispatch({ type: "key", key, ctrl: false, alt: false, shift: false }); | |
| 231 | keys.forEach((k, i) => { | |
| 232 | if (i === keys.length - 1) beforeLast?.(); | |
| 233 | press(k); | |
| 234 | }); | |
| 235 | await settleApp(tui); | |
| 236 | const text = tui.screen.toText(); | |
| 237 | tui.stop?.(); | |
| 238 | return text; | |
| 239 | } | |
| 240 | ||
| 241 | test("d asks naming the label; cancel is the default and keeps it", async () => { | |
| 242 | const mid = await deleting(many, alice, ["d"]); | |
| 243 | expect(mid).toContain("l00"); | |
| 244 | expect(mid).toContain("Delete the label l00?"); | |
| 245 | const text = await deleting(many, alice, ["d", "enter"]); | |
| 246 | expect(text).not.toContain(DELETED_LINE); | |
| 247 | expect(listLabels(app.db, many.id).map((l) => l.name)).toContain("l00"); | |
| 248 | }); | |
| 249 | ||
| 250 | test("d then Delete removes the label and the list drops it", async () => { | |
| 251 | createLabel(app.db, open.id, "aaa-doomed", "123456"); | |
| 252 | const text = await deleting(open, alice, ["d", "right", "enter"]); | |
| 253 | expect(text).toContain(DELETED_LINE); | |
| 254 | expect(text).not.toContain("aaa-doomed"); | |
| 255 | expect(listLabels(app.db, open.id).map((l) => l.name)).not.toContain("aaa-doomed"); | |
| 256 | }); | |
| 257 | ||
| 258 | test("the result line clears on the next key", async () => { | |
| 259 | createLabel(app.db, open.id, "aac-doomed", "123456"); | |
| 260 | const text = await deleting(open, alice, ["d", "right", "enter", "j"]); | |
| 261 | expect(text).not.toContain(DELETED_LINE); | |
| 262 | }); | |
| 263 | ||
| 264 | test("a delete that fails says so and changes nothing", async () => { | |
| 265 | const doomed = createLabel(app.db, open.id, "aab-gone", "123456"); | |
| 266 | if (!doomed.ok) throw new Error("label"); | |
| 267 | const text = await deleting(open, alice, ["d", "right", "enter"], () => { | |
| 268 | deleteLabel(app, alice, open, doomed.value.id); | |
| 269 | }); | |
| 270 | expect(text).toContain(GONE_LINE); | |
| 271 | expect(text).not.toContain(DELETED_LINE); | |
| 272 | }); | |
| 273 | ||
| 274 | test("a reader is not offered d and d deletes nothing", async () => { | |
| 275 | const stranger = new Session(app, guest); | |
| 276 | const ctx = { invalidate() {}, push() { throw new Error("pushed"); } } as unknown as Ctx; | |
| 277 | const screen = labelsScreen(stranger, ctx, open); | |
| 278 | expect(screen.keys.some((k) => k.keys.includes("d"))).toBe(false); | |
| 279 | await screen.load!(new AbortController().signal); | |
| 280 | expect(screen.key!({ type: "key", key: "d", ctrl: false, alt: false, shift: false } as never)).toBe(false); | |
| 281 | expect(labelsScreen(new Session(app, { ...guest, user: alice }), ctx, open).keys.some((k) => k.keys.includes("d"))).toBe(true); | |
| 282 | }); |
new above
offline
Clear
notefor every key, not only here. Right now it survives j/k andn, and keeps the list a row short.