diff --git a/README.md b/README.md index bc49b8c..68d4967 100644 --- a/README.md +++ b/README.md @@ -73,8 +73,10 @@ then repair descendants automatically after the root lands. - Refreshes stack blocks in descriptions. - Saves `.git/stack/undo.json` before mutations. -GitHub stack blocks use compact `#101` references. GitLab blocks use `!101` -references plus titles because bare GitLab MR links only show titles on hover. +Stack blocks render open changes as a nested list where indentation shows the +PR/MR target-branch topology. GitHub stack blocks use compact `#101` +references. GitLab blocks use `!101` references plus titles because bare GitLab +MR links only show titles on hover. If a repair fails, run: diff --git a/skills/stack/SKILL.md b/skills/stack/SKILL.md index 8e49b9d..36d76b6 100644 --- a/skills/stack/SKILL.md +++ b/skills/stack/SKILL.md @@ -87,19 +87,21 @@ work. Repeat after any parent branch changes or a squash merge lands. `stack sync --apply` and `stack merge --apply/--auto` refresh a deterministic block in each open change description: -```md +```text ### [Stack](https://github.com/kitlangton/stack) -1. #101 -2. #102 -3. **#103** 👈 current +- #101 `stack-a` + - #102 `stack-b` + - **#103** 👈 current `stack-c` ``` -Earlier entries are landed history. The current change is bold with `👈 current`. -GitHub uses `#123`; GitLab uses `!123 - Title`. +Earlier top-level entries are landed history. Open changes are rendered as a +nested list where indentation shows lineage and siblings share the same parent. +The current change is bold with `👈 current`. GitHub uses `#123`; GitLab uses +`!123 - Title`. ## Safety Rules diff --git a/src/services/Stack.ts b/src/services/Stack.ts index e5e2dc0..f0b4197 100644 --- a/src/services/Stack.ts +++ b/src/services/Stack.ts @@ -1419,7 +1419,7 @@ ${note}`; StackBlock.render({ pulls: selectedPulls, metas, - chain: graph.displayChainFor(String(pull.head)), + tree: graph.displayTreeFor(String(pull.head)), completed, branch: String(pull.head), previous: meta.body, diff --git a/src/stackBlock.ts b/src/stackBlock.ts index 21a14a6..42444cb 100644 --- a/src/stackBlock.ts +++ b/src/stackBlock.ts @@ -1,4 +1,5 @@ import { PullMeta, PullRef } from "./domain/model.ts"; +import type { DisplayTree } from "./stackGraph.ts"; const start = ""; const end = ""; @@ -41,7 +42,7 @@ const completedLines = ( return prior .split("\n") .map((line) => line.trim()) - .filter((line) => line.startsWith("- [") || /^\d+\.\s+/.test(line)) + .filter((line) => line.startsWith("- ") || /^\d+\.\s+/.test(line)) .flatMap((line) => { const checked = line.startsWith("- [x]"); const numbered = /^\d+\.\s+/.test(line); @@ -49,17 +50,21 @@ const completedLines = ( const pr = line.match(/[#!]\d+/)?.[0] ?? null; const key = branch ?? pr; if (!key || liveKeys.has(key)) return []; - if (completedKeys.size > 0 && !numbered && !checked && !completedKeys.has(key)) { + const checkbox = line.startsWith("- ["); + const completedHistory = checked || numbered || (!checkbox && branch === null); + if (!completedHistory && !completedKeys.has(key)) { return []; } - if (completedKeys.size === 0 && line.startsWith("- [") && !checked) { + if (checkbox && !checked && !completedKeys.has(key)) { return []; } const cleaned = line .replace(/^- \[[ x]\]\s+/, "") + .replace(/^-\s+/, "") .replace(/^\d+\.\s+/, "") .replaceAll("**", "") .replace(/([#!]\d+)\s+`[^`]+`/g, "$1") + .replace(/\s+`[^`]+`/g, "") .replace(/\s*(?:←|👈) current$/, ""); const number = Number(pr?.slice(1)); const title = Number.isInteger(number) ? completedTitles.get(number) : undefined; @@ -67,6 +72,11 @@ const completedLines = ( }); }; +const walkTree = (tree: DisplayTree): ReadonlyArray => [ + tree.branch, + ...tree.children.flatMap((child) => walkTree(child)), +]; + export const references = (body: string) => { const prior = body.match(new RegExp(`${start}([\\s\\S]*?)${end}`))?.[1]; if (!prior) return []; @@ -78,7 +88,7 @@ export const references = (body: string) => { export const render = (opts: { readonly pulls: ReadonlyArray; readonly metas: ReadonlyMap; - readonly chain: ReadonlyArray; + readonly tree: DisplayTree | null; readonly completed?: ReadonlySet; readonly branch: string; readonly previous: string; @@ -91,17 +101,25 @@ export const render = (opts: { const showTitles = opts.showTitles ?? false; const heading = (opts.blockLink ?? true) ? linkedHeading : plainHeading; const prs = new Map(opts.pulls.map((pull) => [String(pull.head), pull])); - const chain = opts.chain; + const treeBranches = opts.tree ? walkTree(opts.tree) : []; const liveKeys = new Set( - chain.flatMap((branch) => { + [...opts.pulls.map((pull) => String(pull.head)), ...treeBranches].flatMap((branch) => { const pr = prs.get(branch) ?? opts.metas.get(branch) ?? null; return pr ? [branch, `#${pr.number}`, `!${pr.number}`] : [branch]; }), ); const line = (name: string) => { const head = format(name, prs, opts.metas, reference, showTitles); - if (name === opts.branch) return `**${head}** 👈 current`; - return head; + const label = name === opts.branch ? `**${head}** 👈 current` : head; + if (head === `\`${name}\``) return label; + return `${label} \`${name}\``; + }; + const treeLines = (tree: DisplayTree, depth = 0): ReadonlyArray => { + const prefix = `${" ".repeat(depth)}- `; + return [ + prefix + line(tree.branch), + ...tree.children.flatMap((child) => treeLines(child, depth + 1)), + ]; }; const items = [ ...completedLines( @@ -110,12 +128,18 @@ export const render = (opts: { opts.completed ?? new Set(), showTitles ? (opts.completedTitles ?? new Map()) : new Map(), ), - ...chain.map(line), + ...(opts.tree ? treeLines(opts.tree) : []), ]; - return [start, heading, "", ...items.map((item, index) => `${index + 1}. ${item}`), end].join( - "\n", - ); + return [ + start, + heading, + "", + ...items.map((entry) => + entry.startsWith("- ") || entry.startsWith(" ") ? entry : `- ${entry}`, + ), + end, + ].join("\n"); }; export const splice = (body: string, next: string) => { diff --git a/src/stackGraph.ts b/src/stackGraph.ts index 0dcd021..cf0d9a9 100644 --- a/src/stackGraph.ts +++ b/src/stackGraph.ts @@ -26,11 +26,17 @@ export interface StatusTree { readonly children: ReadonlyMap>; } +export interface DisplayTree { + readonly branch: string; + readonly children: ReadonlyArray; +} + export interface StackGraph { readonly report: StatusReport; readonly tree: StatusTree; readonly pathTo: (branch: string) => ReadonlyArray; readonly displayChainFor: (branch: string) => ReadonlyArray; + readonly displayTreeFor: (branch: string) => DisplayTree; readonly rank: (branch: string) => number; readonly rootOf: (branch: string) => string; readonly wouldCreateCycle: (branch: string, parent: string) => boolean; @@ -177,6 +183,7 @@ export const make = (input: StackGraphInput): StackGraph => { }; const explicitChildren = new Map>(); + const liveBranches = new Set(input.pulls.map((pull) => String(pull.head))); for (const link of input.state.links) { const parent = String(link.parent); const list = explicitChildren.get(parent) ?? []; @@ -203,6 +210,21 @@ export const make = (input: StackGraphInput): StackGraph => { return chain; }; + const displayTreeFor = (branch: string): DisplayTree => { + const root = pathTo(branch).find((name) => liveBranches.has(name)) ?? branch; + const build = (name: string, seen = new Set()): DisplayTree => { + if (seen.has(name)) return { branch: name, children: [] }; + const nextSeen = new Set(seen); + nextSeen.add(name); + const children = (explicitChildren.get(name) ?? []) + .filter((child) => liveBranches.has(child)) + .map((child) => build(child, nextSeen)); + return { branch: name, children }; + }; + + return build(root); + }; + const rank = (branch: string, seen = new Set()): number => { if (seen.has(branch)) return 0; const link = links.get(branch); @@ -229,6 +251,7 @@ export const make = (input: StackGraphInput): StackGraph => { tree, pathTo, displayChainFor, + displayTreeFor, rank, rootOf, wouldCreateCycle: (branch: string, parent: string) => diff --git a/tests/stack.test.ts b/tests/stack.test.ts index 2fc0e4f..17ea296 100644 --- a/tests/stack.test.ts +++ b/tests/stack.test.ts @@ -74,6 +74,8 @@ const metaFor = (pull: ReturnType, body = "body") => labels: [], }); +const expectLine = (value: string, line: string) => expect(value.split("\n")).toContain(line); + const gitAndCodeHost = (service: Partial) => { const unused = (tool: string, args: ReadonlyArray) => new ExecError(tool, args, 1, "unused test service"); @@ -1288,8 +1290,36 @@ describe("StackGraph", () => { expect(graph.pathTo("stack-c")).toEqual(["stack-a", "stack-c"]); expect(graph.displayChainFor("stack-b")).toEqual(["stack-a", "stack-b"]); expect(graph.displayChainFor("stack-c")).toEqual(["stack-a", "stack-c"]); + expect(graph.displayTreeFor("stack-b")).toEqual({ + branch: "stack-a", + children: [ + { branch: "stack-b", children: [] }, + { branch: "stack-c", children: [] }, + ], + }); expect(graph.wouldCreateCycle("stack-a", "stack-b")).toBe(true); }); + + it("starts display trees at the highest live ancestor", () => { + const state = stackState([ + stackLink({ branch: "stack-a", parent: "dev", anchor: "dev", pr: 1 }), + stackLink({ branch: "stack-b", parent: "stack-a", anchor: "a", pr: 2 }), + stackLink({ branch: "stack-c", parent: "stack-b", anchor: "b", pr: 3 }), + ]); + const graph = StackGraph.make({ + state, + refs: [ref("dev"), ref("stack-a", "a"), ref("stack-b", "b"), ref("stack-c", "c")], + pulls: [pr(2, "stack-b", "stack-a"), pr(3, "stack-c", "stack-b")], + trunks: ["dev"], + current: "stack-c", + }); + + expect(graph.rootOf("stack-c")).toBe("stack-a"); + expect(graph.displayTreeFor("stack-c")).toEqual({ + branch: "stack-b", + children: [{ branch: "stack-c", children: [] }], + }); + }); }); describe("Git", () => { @@ -3187,7 +3217,7 @@ describe("Stack", () => { }).pipe(Effect.provide(test.layer)); }); - it.effect("links render the stack as chronological GitHub checkboxes", () => { + it.effect("links render the stack as nested GitHub bullets", () => { const test = makeSync(); return Effect.gen(function* () { @@ -3199,9 +3229,8 @@ describe("Stack", () => { expect(body).not.toContain("Earlier in Stack"); expect(body).not.toContain("Current / Remaining"); expect(body).not.toContain("\nMerged\n"); - expect(body).toContain("1. #4"); - expect(body).toContain("2. #5"); - expect(body).toContain("3. **#3** 👈 current"); + expectLine(body, "- #5 `stack-b`"); + expectLine(body, " - **#3** 👈 current `stack-c`"); }).pipe(Effect.provide(test.layer)); }); @@ -3245,9 +3274,9 @@ describe("Stack", () => { yield* stack.links(true); const body = bodies.get(1) ?? ""; - expect(body).toContain("1. **#1** 👈 current"); - expect(body).toContain("2. #2"); - expect(body).toContain("3. #3"); + expectLine(body, "- **#1** 👈 current `stack-a`"); + expectLine(body, " - #2 `stack-b`"); + expectLine(body, " - #3 `stack-c`"); }).pipe(Effect.provide(layer)); }); @@ -3264,14 +3293,13 @@ describe("Stack", () => { yield* stack.links(true); const body = test.bodies.get(3) ?? ""; - expect(body).toContain("1. !4 - stack-a"); - expect(body).toContain("2. !5 - fix+refactor(vcs): old title"); - expect(body).toContain("3. **!3 - stack-c** 👈 current"); + expectLine(body, "- !5 - fix+refactor(vcs): old title `stack-b`"); + expectLine(body, " - **!3 - stack-c** 👈 current `stack-c`"); expect(body).not.toContain("#3"); }).pipe(Effect.provide(test.layer)); }); - it.effect("links render the current path through a forked stack", () => { + it.effect("links render sibling PRs in a forked stack", () => { const bodies = new Map(); const pulls = [ pullRef({ number: 1, head: "stack-a", base: "dev", url: "u1", draft: false }), @@ -3348,9 +3376,9 @@ describe("Stack", () => { const stack = yield* Stack; yield* stack.links(true); const body = bodies.get(2) ?? ""; - expect(body).toContain("1. #1"); - expect(body).toContain("2. **#2** 👈 current"); - expect(body).not.toContain("stack-c"); + expectLine(body, "- #1 `stack-a`"); + expectLine(body, " - **#2** 👈 current `stack-b`"); + expectLine(body, " - #3 `stack-c`"); }).pipe(Effect.provide(layer)); }); @@ -3389,9 +3417,9 @@ describe("Stack", () => { yield* stack.links(true); const body = bodies.get(3) ?? ""; - expect(body).toContain("1. #1"); - expect(body).toContain("2. #2"); - expect(body).toContain("3. **#3** 👈 current"); + expect(body).toContain("- #1"); + expectLine(body, "- #2 `stack-b`"); + expectLine(body, " - **#3** 👈 current `stack-c`"); }).pipe(Effect.provide(layer)); }); @@ -3425,10 +3453,10 @@ describe("Stack", () => { yield* stack.links(true); const body = bodies.get(4) ?? ""; - expect(body).toContain("1. #1"); + expect(body).toContain("- #1"); expect(body).not.toContain("#2"); expect(body).not.toContain("#3"); - expect(body).toContain("2. **#4** 👈 current"); + expectLine(body, "- **#4** 👈 current `stack-b`"); }).pipe(Effect.provide(layer)); }); @@ -3462,10 +3490,10 @@ describe("Stack", () => { yield* stack.links(true); const body = bodies.get(4) ?? ""; - expect(body).toContain("1. #1"); - expect(body).toContain("2. #2"); - expect(body).toContain("3. #3"); - expect(body).toContain("4. **#4** 👈 current"); + expect(body).toContain("- #1"); + expect(body).toContain("- #2"); + expect(body).toContain("- #3"); + expectLine(body, "- **#4** 👈 current `stack-b`"); }).pipe(Effect.provide(layer)); }); @@ -5201,18 +5229,27 @@ describe("StackBlock", () => { draft: false, }), ]; + const tree = { + branch: "feat/a", + children: [ + { + branch: "feat/b", + children: [{ branch: "feat/c", children: [] }], + }, + ], + }; - it("renders GitHub PR references with the # prefix by default", () => { + it("renders GitHub PR references as a nested topology", () => { const block = StackBlock.render({ pulls, metas: new Map(), - chain: ["feat/a", "feat/b", "feat/c"], + tree, branch: "feat/b", previous: "", }); - expect(block).toContain("1. #1"); - expect(block).toContain("2. **#2** 👈 current"); - expect(block).toContain("3. #3"); + expectLine(block, "- #1 `feat/a`"); + expectLine(block, " - **#2** 👈 current `feat/b`"); + expectLine(block, " - #3 `feat/c`"); expect(block).not.toContain("Feature A"); }); @@ -5220,7 +5257,7 @@ describe("StackBlock", () => { const block = StackBlock.render({ pulls, metas: new Map(), - chain: ["feat/a", "feat/b", "feat/c"], + tree, branch: "feat/b", previous: "", }); @@ -5231,7 +5268,7 @@ describe("StackBlock", () => { const block = StackBlock.render({ pulls, metas: new Map(), - chain: ["feat/a", "feat/b", "feat/c"], + tree, branch: "feat/b", previous: "", blockLink: false, @@ -5239,21 +5276,21 @@ describe("StackBlock", () => { expect(block).toContain("### Stack"); expect(block).not.toContain("[Stack]"); expect(block).not.toContain("https://github.com/kitlangton/stack"); - expect(block).toContain("2. **#2** 👈 current"); + expectLine(block, " - **#2** 👈 current `feat/b`"); }); it("renders GitLab MR references using the code host reference formatter", () => { const block = StackBlock.render({ pulls, metas: new Map(), - chain: ["feat/a", "feat/b", "feat/c"], + tree, branch: "feat/c", previous: "", reference: (number) => `!${number}`, }); - expect(block).toContain("1. !1"); - expect(block).toContain("2. !2"); - expect(block).toContain("3. **!3** 👈 current"); + expectLine(block, "- !1 `feat/a`"); + expectLine(block, " - !2 `feat/b`"); + expectLine(block, " - **!3** 👈 current `feat/c`"); expect(block).not.toContain("#1"); expect(block).not.toContain("Feature A"); }); @@ -5262,15 +5299,15 @@ describe("StackBlock", () => { const block = StackBlock.render({ pulls, metas: new Map(), - chain: ["feat/a", "feat/b", "feat/c"], + tree, branch: "feat/c", previous: "", reference: (number) => `!${number}`, showTitles: true, }); - expect(block).toContain("1. !1 - Feature A"); - expect(block).toContain("2. !2 - Feature B"); - expect(block).toContain("3. **!3 - Feature C** 👈 current"); + expectLine(block, "- !1 - Feature A `feat/a`"); + expectLine(block, " - !2 - Feature B `feat/b`"); + expectLine(block, " - **!3 - Feature C** 👈 current `feat/c`"); }); it("can enrich completed GitLab history with MR titles", () => { @@ -5286,7 +5323,7 @@ describe("StackBlock", () => { const block = StackBlock.render({ pulls: [], metas: new Map(), - chain: [], + tree: null, branch: "feat/d", previous, reference: (number) => `!${number}`, @@ -5297,9 +5334,9 @@ describe("StackBlock", () => { [3, "Feature C"], ]), }); - expect(block).toContain("1. !1 - Feature A"); - expect(block).toContain("2. !2 - Already titled"); - expect(block).toContain("3. !3 - Feature C"); + expect(block).toContain("- !1 - Feature A"); + expect(block).toContain("- !2 - Already titled"); + expect(block).toContain("- !3 - Feature C"); }); it("parses both # and ! prefixed entries from a previous block", () => { @@ -5315,12 +5352,12 @@ describe("StackBlock", () => { const block = StackBlock.render({ pulls: [pulls[2]!], metas: new Map(), - chain: ["feat/c"], + tree: { branch: "feat/c", children: [] }, branch: "feat/c", previous, reference: (number) => `!${number}`, }); - expect(block).toContain("**!3** 👈 current"); + expectLine(block, "- **!3** 👈 current `feat/c`"); }); it("does not duplicate live entries when prefix migrates between syncs", () => { @@ -5336,7 +5373,7 @@ describe("StackBlock", () => { const block = StackBlock.render({ pulls, metas: new Map(), - chain: ["feat/a", "feat/b", "feat/c"], + tree, branch: "feat/c", previous, reference: (number) => `!${number}`, @@ -5344,9 +5381,91 @@ describe("StackBlock", () => { expect(block).not.toContain("#1"); expect(block).not.toContain("#2"); expect(block).not.toContain("#3"); - expect(block).toContain("1. !1"); - expect(block).toContain("2. !2"); - expect(block).toContain("3. **!3** 👈 current"); + expectLine(block, "- !1 `feat/a`"); + expectLine(block, " - !2 `feat/b`"); + expectLine(block, " - **!3** 👈 current `feat/c`"); + }); + + it("preserves prior bullet history across consecutive merges", () => { + const previous = `body before + + +### [Stack](https://github.com/aryasaatvik/stack) + +- #1 +- #2 \`feat/b\` + - **#3** 👈 current \`feat/c\` +`; + const block = StackBlock.render({ + pulls: [pulls[2]!], + metas: new Map(), + tree: { branch: "feat/c", children: [] }, + branch: "feat/c", + previous, + completed: new Set(["#2", "feat/b"]), + }); + + expectLine(block, "- #1"); + expectLine(block, "- #2"); + expectLine(block, "- **#3** 👈 current `feat/c`"); + expect(block).not.toContain("`feat/b`"); + }); + + it("does not preserve abandoned siblings from a previous nested block as history", () => { + const previous = `body before + + +### [Stack](https://github.com/aryasaatvik/stack) + +- #1 \`feat/a\` + - **#2** 👈 current \`feat/b\` + - #3 \`feat/c\` +`; + const fork = { + branch: "feat/a", + children: [{ branch: "feat/b", children: [] }], + }; + const block = StackBlock.render({ + pulls: [pulls[0]!, pulls[1]!], + metas: new Map(), + tree: fork, + branch: "feat/b", + previous, + }); + + expect(block).not.toContain("#3"); + expectLine(block, "- #1 `feat/a`"); + expectLine(block, " - **#2** 👈 current `feat/b`"); + }); + + it("does not preserve open siblings from a stale flat block as history", () => { + const previous = `body before + + +### [Stack](https://github.com/kitlangton/stack) + +1. #1 +2. #3 +3. **#2** 👈 current +`; + const fork = { + branch: "feat/a", + children: [ + { branch: "feat/b", children: [] }, + { branch: "feat/c", children: [] }, + ], + }; + const block = StackBlock.render({ + pulls, + metas: new Map(), + tree: fork, + branch: "feat/b", + previous, + }); + + expect(block.match(/#3/g)).toHaveLength(1); + expectLine(block, " - #3 `feat/c`"); + expect(block).not.toContain("- #3\n"); }); });