diff --git a/README.md b/README.md index 0e11c92..c8aafc2 100644 --- a/README.md +++ b/README.md @@ -69,6 +69,10 @@ checklist), so the UI stays responsive while remote pushes complete. In the diff view: `↑`/`↓` scroll, `space`/`ctrl+d` page down, `ctrl+u`/`pgup` page up, `g`/`G` jump to top/bottom, and `q`/`esc`/`v` return to the list. +The confirmation, force-delete, progress and results screens scroll with the same keys, so a +wide selection never pushes the prompt off the bottom of the terminal. The answer keys take +precedence, so `y`, `R` and `n` still work while a list is scrolled. + ## Row format ![The branch list: selection, track and merge columns, dates, hashes and subjects](assets/branches.png) diff --git a/docs/improvements.md b/docs/improvements.md index 214b2c4..e189684 100644 --- a/docs/improvements.md +++ b/docs/improvements.md @@ -78,6 +78,124 @@ to fail against the old code (`date column at cell 31, want 33`) before being ke silently vanished — no error, the safety signal simply was not there. `remotes()` now tries every configured remote with `origin` ordered first. +### Tier 2 + +**7. The tested delete path was not the one users run.** `performDeletions` was test-only by its +own comment, so the live async path's completion logic — `branchDeletedMsg` → `deletesDone` → the +`stateForcePrompt` / `stateResult` transition — was never fed through `Update`. The riskiest state +machine in the program was the untested one. + +All 18 delete call sites now drive the real path through a `drainDeletions` helper, which runs the +`tea.Batch` cmds concurrently (as the runtime does) and feeds every `branchDeletedMsg` back through +`Update`. The force prompt is answered with a keystroke rather than by calling +`forceDeleteUnmerged` directly, and the four tests that hand-set `state = stateForcePrompt` now +assert the machine put them there. `performDeletions` has been deleted. + +Five tests cover what only the async path can get wrong; each was verified to fail against a +deliberately broken `Update` before being kept: + +| test | mutation it catches | +| ---- | ------------------- | +| `TestDeletingWaitsForEveryResult` | completing the run on the first result (`> 0` instead of `>= len(m.results)`) — and reloading the branch list mid-run, which would renumber the indices outstanding messages still write to | +| `TestStrayDeleteResultIsIgnored` | dropping the `msg.idx` bounds check — panics with `index out of range [7]` | +| `TestSpinnerTickStopsAfterDeleting` | dropping the `state == stateDeleting` guard, leaving the tick re-arming forever | +| `TestForcePromptDeclineKeepsBranch` | `n` at the prompt discarding the branch it exists to spare | +| `TestDeletingIgnoresKeysExceptCtrlC` | a stray keystroke dismissing a run whose results have not landed | + +**14. Startup and refresh spent over a second measuring risk, one branch at a time.** Every +`git cherry` is its own subprocess, and `refreshMergeInfo` ran one per gone branch in sequence — +on the tool's headline case, a repo full of gone branches, that is the whole list on the clock +before the first paint, again after every fetch, and again after every prune. + +Measured on a 101-branch repo where 100 branches are gone: + +| path | before | after | +| ---- | ------ | ----- | +| `initialModel` (startup) | 1.14s | 0.56s | +| `refreshMergeInfo` (fetch / reload) | 1.12s | 0.33s | +| `measureSelectedRisk` (pressing `d`)| 1.09s | 0.25s | + +The cost is process spawn, not git work: 100 × `git rev-parse HEAD` takes 0.63s against 100 × +`git cherry` at 0.74s, so ~6ms per subprocess is the floor and no cheaper git command would have +helped. `measureRisk` now takes a predicate and runs the calls concurrently, one goroutine per +branch writing its own slice element. A cap sweep on the same repo put the knee at 4–8 workers +(1 → 1056ms, 4 → 364ms, 8 → 330ms, 32 → 331ms), so `maxLocalGit` is 8; past that the limit is +elsewhere. + +**15. A wide prune opened one connection per branch.** `tea.Batch` gives every selected branch its +own goroutine (`bubbletea/tea.go:545`), so arming remotes on 100 branches meant 100 simultaneous +`git push --delete` — 100 connections a remote would throttle or refuse. A failed push there is +the worst shape the tool has: the local branch is already gone, and the copy that was meant to +outlive it is still sitting on the remote. + +Local and network work are now capped separately — `maxLocalGit = 8`, `maxRemotePush = 3` — so a +wide local fan-out never widens the network one. The cap sits inside `runGit`, the single door +every git invocation already passes through, and `networkBound(args)` picks which one applies. Put +at the call sites instead it held only for the callers that remembered to ask: `fetchPruneCmd` and +`forceDeleteUnmerged` both escaped it. + +The cap is measured on the subprocesses themselves, not on the limiter: `runGit` feeds two +`gauge`s, one for all git processes and one for the network-bound ones. Instrumenting the limiter +would have stopped reporting along with it — verified by deleting the limiter, which took the +observed peak from 3 to 17 and failed `TestWideDeleteStaysWithinCaps`. The test asserts literal +bounds rather than the constants, so raising a cap breaks it instead of moving with it. + +The earlier "60 parallel deletes succeeded" note below still stands, but it only ever covered +local ref locking. It says nothing about the network. + +**16. A `git cherry` subprocess per branch, for an answer one query already held.** `measureRisk` +ran `git cherry ` for every gone branch. A branch whose tip is already an ancestor +of the base has an empty `base..branch` range, so `git cherry` can only report nothing — and on +the tool's headline case, a repo full of branches that were merged and then pruned, that is nearly +every one of them. + +`refreshMergeInfo` now runs one `git branch --merged ` and caches the result in +`m.baseMerged`; `measureRisk` reads the set instead of starting a process. Measured on the same +101-branch fixture (100 gone, all merged): startup **529ms → 45ms**, peak concurrent git +processes 8 → 2. One query at 9ms replaced 30 `git cherry` calls at 330ms on a 30-branch probe. + +`TestMergedBranchesCostNoSubprocess` asserts a literal process bound rather than a time, and fails +at 39 processes when the shortcut is removed. + +**17. The confirm screen rendered its whole body, pushing its own question off the terminal.** +`confirmView` wrote every selected branch in full. With 40 selected that is 206 rows into a 24-row +terminal, with `Delete these branches?` on row 205 — the user answered a prompt they could not +read. `forcePromptView`, `resultView` and `deletingView` had the same shape. + +All four now go through `page(header, body, footer)`, which renders the header, a window into the +body, and the footer, plus a position line when the body does not fit. `scrollKeys` adds ↑/↓, +space, ctrl+d/u, pgup/pgdn and g/G, and the mouse wheel scrolls them; each screen answers its own +keys first, so `y`, `R` and `n` are never swallowed. Rendering only the visible rows also takes +the per-frame cost off the length of the list. + +**18. Startup was six git subprocesses deep, and the depth was the whole cost.** On this repo +(3 branches) startup was 34ms, all of it process-start latency — one `git` costs ~6.5ms to spawn, +and the chain ran `rev-parse` → `for-each-ref` → `remote` → `symbolic-ref` → `branch -r --merged` +→ `branch --merged ` in sequence. Only `branch --merged HEAD` overlapped anything. + +Three changes, together taking the chain to two rounds: + +- `loadRemoteRefs` reads `refs/remotes` once with `--format='%(refname)%00%(symref)'`. That single + call replaces `git remote`, a `symbolic-ref` per remote and a `rev-parse --verify` per candidate. + `%(symref)` yields the full ref, so it keeps the tag-shadowing guarantee `symbolic-ref --short` + would break. Remote names now come out of the refs: a remote with no fetched refs holds nothing + a default branch could resolve to, so nothing is lost. +- `localDefaultBranch` takes a `has(name)` predicate. `refreshMergeInfo` passes a lookup into the + branch list it already holds; only `baseBranch`, which has no list, still pays `gitHasBranch`. +- `loadRepo` starts `loadBranches`, `localMergedSet` and `loadRemoteRefs` in one round, and + `initialModel` runs the repo check beside them rather than ahead of them. `refreshMergeInfo` + then runs `branch -r --merged` and `branch --merged ` as a second round. + +| repo | before | after | +| ---- | ------ | ----- | +| this one (3 branches) | 34ms | 19ms | +| 101 branches, 100 gone (with item 16)| 529ms | 17ms | +| 501 branches | 90ms | 77ms | + +The 501-branch case is `for-each-ref`'s own work (42ms of the 77ms), not chain depth — measured at +15ms for `%(refname)` alone against 42ms for the full format, so the commit-object reads behind +`%(committerdate)` are the floor there. + ### Also completed - `LICENSE` (MIT) @@ -90,27 +208,33 @@ configured remote with `origin` ordered first. ### Tier 2 — robustness -**5. Blocking git calls inside `Update`.** `loadDiff` (`v`), `refreshMergeInfo`, and -`reloadBranches` run synchronously in the update loop. `git branch -r --merged` is -O(remote refs × history) and runs on *every* reload; on a repo with thousands of remote branches -the UI freezes. The `tea.Cmd` pattern already works for fetch — reuse it. Note that -`refreshMergeInfo` now also issues one `git cherry` per gone branch, which raises the stakes. +**5. Blocking git calls inside `Update`.** `loadDiff` (`v`), `refreshMergeInfo` and +`reloadBranches` still run synchronously in the update loop, so what remains of their cost is +still a freeze. Items 14, 16 and 18 under Completed took the residue to ~16ms on a +100-gone-branch repo, so this is no longer a visible freeze — what it would still buy is the +first paint, which currently waits on the whole two-round load. Moving `refreshMergeInfo` onto +the `tea.Cmd` pattern already used for fetch would put the branch list on screen after one +subprocess (~7ms) and fill the ✓ and risk columns in behind it. + +Do not reach for `git branch -r --merged` first: it was measured at 100ms with 2000 remote +refs, roughly a tenth of what the `git cherry` loop beside it cost. **6. `runGit` has no timeout and does not disable terminal prompts.** `fetch --all --prune` and `push --delete` are network-bound; a credential or SSH prompt hangs the TUI with no recovery. Set `GIT_TERMINAL_PROMPT=0` and attach a `context.WithTimeout` so it fails fast instead. -**7. The tested delete path is not the one users run.** `performDeletions` is test-only by its own -comment; the live async path's completion logic — `branchDeletedMsg` → `deletesDone` → the -`stateForcePrompt` / `stateResult` transition — is never fed through `Update` in any test. The -riskiest state machine in the program is the untested one. Port the tests to the async path and -delete `performDeletions`. - **8. Smaller items.** +- Rows are wider than the terminal whenever `subjectWidth` hits its `max(10, …)` floor, and each + wrapped row eats two screen lines while `visibleRows` still counts it as one — so the list + overruns and the footer scrolls away. Measured threshold: rows wrap below `68 + nameW` columns, + which at a classic 80-column terminal means any branch name of 13 cells or more wraps *every* + row. `renderRow` needs to fit `m.width` rather than assume it. - `listView` runs one line over terminal height when `status` and `err` are both set - (`visibleRows` is `height-5`; actual emission is `height+1`). + (`visibleRows` is `height-5`; actual emission is `height+1`) — confirmed at 25 lines for a + height of 24. - ANSI and control characters in commit subjects and branch names render raw into the terminal. -- `applyBranches` silently discards the user's existing selections on `p`. + Confirmed: a `\x1b[31m` in a subject reaches the row intact (a bare `BEL` is stripped by + `ansi.Truncate`). The text comes from fetched branches, so it is not the author's to trust. - The cursor starts on an arbitrary row. `sortBranches` preserves the cursor by name unconditionally, but at startup `cursor` is 0 and `branches` is still in `for-each-ref` (alphabetical) order, so it pins the cursor to wherever the alphabetically-first branch @@ -144,6 +268,15 @@ naturally with finding 1. ## Investigated and rejected +**A commit subject spanning several lines breaking the `for-each-ref` parse.** `loadBranches` +splits output on newlines and needs 8 NUL-separated fields per branch, so an embedded newline +would silently drop branches from the list. It cannot happen: `%(contents:subject)` folds the +subject's newlines to spaces. Verified against a commit whose subject wraps before the blank line. + +**A duplicate or stray `branchDeletedMsg` ending a run early.** `deletesDone` counts `done` flags +on the results, not messages received, so a repeat cannot over-count; an out-of-range index is +dropped by the bounds check. Both are covered by `TestStrayDeleteResultIsIgnored`. + **Concurrent `git branch -d` racing on `packed-refs.lock`.** `tea.Batch` runs deletions concurrently, which looked like it should collide on the packed-refs lock. Tested with 60 parallel deletes against a freshly packed repo: **all 60 succeeded.** Git's ref-lock retry handles it. No diff --git a/docs/test-coverage-gaps.md b/docs/test-coverage-gaps.md index 9ecba46..d50d499 100644 --- a/docs/test-coverage-gaps.md +++ b/docs/test-coverage-gaps.md @@ -208,7 +208,8 @@ All three tiers are complete (2026-08-17): **14 tests added, 43 total**, passing - Tier 3 pins five everyday operations and corrected one more prediction. - A `/simplify` pass then found **two further live instances of the tag-shadowing bug** that Tier 1 had missed (`localDefaultBranch` and `push --delete`), removed the `qualifyRef` - probe, and brought startup back from 11 git subprocesses to 7 — the pre-change baseline. + probe, and brought startup back from 11 git subprocesses to 7 — the pre-change baseline. It is +6 now, in two concurrent rounds rather than six serial ones (see item 18 in `improvements.md`). Fixtures compose rather than duplicate: `initRepo` (bare init + identity) → `setupLocalRepo` (branch shapes) → `setupRepo` (+ `addOrigin` + a tracking branch), with diff --git a/main.go b/main.go index d7e490e..dd92706 100644 --- a/main.go +++ b/main.go @@ -9,6 +9,8 @@ import ( "sort" "strconv" "strings" + "sync" + "sync/atomic" "time" tea "github.com/charmbracelet/bubbletea" @@ -134,6 +136,10 @@ type model struct { remoteDefault string // resolved remote default branch, e.g. "origin/main" riskBase string // ref that branch.riskCommits is measured against ("" if unresolved) riskBaseRef string // riskBase fully qualified, so a same-named tag cannot shadow it + // baseMerged holds the local branches whose tip is an ancestor of riskBaseRef. + // Their riskCommits is 0 by definition, so one query here removes a `git + // cherry` subprocess per branch (see measureRisk). + baseMerged map[string]bool spinnerFrame int // animation frame for the deleting spinner (deletion counts derive from results) @@ -142,6 +148,8 @@ type model struct { diffLines []string // raw lines of the diff being viewed diffTop int // scroll offset within diffLines + bodyTop int // scroll offset within the confirm/force/result body (see page) + width, height int err string status string // transient info message (e.g. fetch results) @@ -174,7 +182,75 @@ var ( // ---- git I/O ---- +// Concurrency caps. Local git work is subprocess-bound — roughly 6ms of spawn +// cost each — so running it a few at a time is what makes a repo full of gone +// branches load quickly. Remote work is not: every push or fetch opens its own +// connection, and a prune of a hundred armed branches would open a hundred at +// once, which remotes throttle or refuse. The two are capped separately so a +// wide local fan-out never widens the network one. +const ( + maxLocalGit = 8 + maxRemotePush = 3 +) + +var ( + localSlots = make(chan struct{}, maxLocalGit) + remoteSlots = make(chan struct{}, maxRemotePush) +) + +// networkBound reports whether a git invocation opens a connection to a remote. +// It is what picks the cap, so a new network subcommand belongs here rather than +// at its call site. +func networkBound(args []string) bool { + return len(args) > 0 && (args[0] == "push" || args[0] == "fetch") +} + +// gauge records the highest number of concurrent holders it has seen. It sits on +// the subprocesses rather than on the slots, so a test can tell a cap that works +// from one that was removed — instrumenting the cap would stop reporting along +// with it. +// It also counts every holder, which is what tells a fan-out that was removed +// from one that merely runs fast on a small fixture. +type gauge struct{ inFlight, peak, total atomic.Int64 } + +func (g *gauge) enter() { + g.total.Add(1) + n := g.inFlight.Add(1) + for { + peak := g.peak.Load() + if n <= peak || g.peak.CompareAndSwap(peak, n) { + return + } + } +} + +func (g *gauge) leave() { g.inFlight.Add(-1) } + +// gitProcs counts every git subprocess; netProcs counts the ones that talk to a +// remote, which is what a remote host actually feels. +var gitProcs, netProcs gauge + +// runGit is the single door every git invocation passes through, which is what +// makes it the place to bound them: a cap at the call sites would only hold for +// the callers that remembered to ask. func runGit(args ...string) (string, error) { + net := networkBound(args) + slots := localSlots + if net { + slots = remoteSlots + } + slots <- struct{}{} + defer func() { <-slots }() + + // Counted after the slot is held, so the gauges measure what is running + // rather than what is queued. + gitProcs.enter() + defer gitProcs.leave() + if net { + netProcs.enter() + defer netProcs.leave() + } + cmd := exec.Command("git", args...) // Pin the locale: deleteBranch classifies failures by matching git's own // error text, which gettext would otherwise translate. Everything else we @@ -267,59 +343,92 @@ func loadBranches() ([]branch, error) { return branches, nil } -// remotes lists the configured remotes with "origin" first, so the conventional -// remote wins when several exist while repos whose only remote is named -// something else (upstream, fork, …) still resolve a default branch. -func remotes() []string { - out, err := runGit("remote") +// remoteRefs is one read of refs/remotes: which remote-tracking refs exist, and +// what each symbolic one points at. Reading them together is what replaces `git +// remote`, a `symbolic-ref` per remote and a `rev-parse --verify` per candidate +// — five subprocess starts on an ordinary one-remote repo, against this one. +type remoteRefs struct { + names []string // remote names, "origin" first + exists map[string]bool // fully qualified ref -> present + symref map[string]string // fully qualified symbolic ref -> the ref it names +} + +// loadRemoteRefs reads every remote-tracking ref in one call. The remote names +// come out of the refs rather than out of `git remote`: a remote with no fetched +// refs cannot supply a default branch, so it is nothing the caller could use. +func loadRemoteRefs() remoteRefs { + rr := remoteRefs{exists: map[string]bool{}, symref: map[string]string{}} + out, err := runGit("for-each-ref", "--format=%(refname)%00%(symref)", "refs/remotes") if err != nil { - return nil + return rr } - var names []string + seen := map[string]bool{} for _, line := range strings.Split(out, "\n") { + ref, target := "", "" if s := strings.TrimSpace(line); s != "" { - names = append(names, s) + ref, target, _ = strings.Cut(s, "\x00") + } + if ref == "" { + continue + } + rr.exists[ref] = true + if target != "" { + rr.symref[ref] = target + } + // The first segment after the namespace is the remote's name. + if name, _, ok := strings.Cut(shortRef(ref), "/"); ok && !seen[name] { + seen[name] = true + rr.names = append(rr.names, name) } } - sort.SliceStable(names, func(i, j int) bool { return names[i] == "origin" && names[j] != "origin" }) - return names + // origin first, so the conventional remote wins when several exist while a + // repo whose only remote is named something else (upstream, fork, …) still + // resolves a default branch. + sort.SliceStable(rr.names, func(i, j int) bool { return rr.names[i] == "origin" && rr.names[j] != "origin" }) + return rr } +// defaultBranchNames are the branch names treated as a repo's trunk, in order of +// preference. +var defaultBranchNames = []string{"main", "master"} + // localDefaultBranch returns the ref of a local main/master, skipping exclude (a // short branch name) so a branch is never compared against itself. Returns "" -// when neither exists. +// when neither exists. has answers whether a local branch of that name exists; +// a caller already holding the branch list passes a lookup into it rather than +// paying a subprocess per candidate. // // The resolvers below all return fully qualified refs, and the display layer // shortens them with shortRef. Resolving is the only place the namespace is // known for certain, so carrying it forward from here is what keeps a same-named // tag from being measured in place of the branch further down. -func localDefaultBranch(exclude string) string { - for _, c := range []string{"main", "master"} { - if c == exclude { - continue - } - if ref := branchRef(c); refExists(ref) { - return ref +func localDefaultBranch(exclude string, has func(string) bool) string { + for _, c := range defaultBranchNames { + if c != exclude && has(c) { + return branchRef(c) } } return "" } -// remoteDefault resolves the remote's default branch as a remote-tracking ref -// (e.g. "refs/remotes/origin/main"): /HEAD if set, else /main, -// else /master, trying each remote in turn. Returns "" when none can be -// found. -func remoteDefault() string { - for _, r := range remotes() { - // Deliberately not symbolic-ref --short: it shortens to the shortest - // *unambiguous* name, which a same-named tag turns into "remotes/origin/main". - if out, err := runGit("symbolic-ref", "refs/remotes/"+r+"/HEAD"); err == nil { - if s := strings.TrimSpace(out); s != "" { - return s - } +// gitHasBranch is the localDefaultBranch lookup for callers with no branch list +// to hand — it costs a subprocess per candidate. +func gitHasBranch(name string) bool { return refExists(branchRef(name)) } + +// remoteDefaultFrom resolves the remote's default branch as a remote-tracking +// ref (e.g. "refs/remotes/origin/main"): /HEAD if set, else +// /main, else /master, trying each remote in turn. Returns "" +// when none can be found. +func remoteDefaultFrom(rr remoteRefs) string { + for _, r := range rr.names { + // %(symref) yields the full ref, unlike `symbolic-ref --short`, which + // gives the shortest *unambiguous* name — "remotes/origin/main" as soon + // as a tag shares the name. + if t := rr.symref["refs/remotes/"+r+"/HEAD"]; t != "" { + return t } - for _, c := range []string{r + "/main", r + "/master"} { - if ref := "refs/remotes/" + c; refExists(ref) { + for _, c := range defaultBranchNames { + if ref := "refs/remotes/" + r + "/" + c; rr.exists[ref] { return ref } } @@ -327,13 +436,17 @@ func remoteDefault() string { return "" } +// remoteDefault reads the remote-tracking refs and resolves the default branch +// from them. +func remoteDefault() string { return remoteDefaultFrom(loadRemoteRefs()) } + // baseBranch returns the ref to diff a branch against: the remote default branch, // else a local main/master, excluding name itself. func baseBranch(name string) string { if def := remoteDefault(); def != "" { return def } - return localDefaultBranch(name) + return localDefaultBranch(name, gitHasBranch) } // riskCommitCount counts commits on name whose patch is not already present in @@ -392,6 +505,32 @@ func remoteMergedSet(def string) map[string]bool { // git call covers the whole list, so this costs nothing per branch. func localMergedSet() map[string]bool { return mergedSet("branch", "--merged", "HEAD") } +// repoReads holds the reads that do not depend on the branch list, so they can +// run in the same round as it. +type repoReads struct { + headMerged map[string]bool // local branches merged into HEAD + remotes remoteRefs +} + +// loadRepo reads the branch list and everything independent of it in one round. +// Starting a git subprocess costs about 6ms, and none of these three waits on +// another, so the depth of the chain is what the user waits on — not the work +// inside it. +func loadRepo() ([]branch, repoReads, error) { + var ( + branches []branch + err error + reads repoReads + ) + var wg sync.WaitGroup + wg.Add(3) + go func() { defer wg.Done(); branches, err = loadBranches() }() + go func() { defer wg.Done(); reads.headMerged = localMergedSet() }() + go func() { defer wg.Done(); reads.remotes = loadRemoteRefs() }() + wg.Wait() + return branches, reads, err +} + // fetchDoneMsg reports completion of an async `git fetch --all --prune`. type fetchDoneMsg struct{ err error } @@ -447,17 +586,26 @@ func loadDiff(name string) (diff, base string, err error) { // ---- model ---- func initialModel() (model, error) { - if _, err := runGit("rev-parse", "--is-inside-work-tree"); err != nil { + // The repo check runs beside the reads rather than ahead of them. It is here + // to give a clearer message than git's own, not to gate the work, and a + // serial subprocess start is most of what startup costs. + var repoErr error + checked := make(chan struct{}) + go func() { + _, repoErr = runGit("rev-parse", "--is-inside-work-tree") + close(checked) + }() + branches, reads, err := loadRepo() + <-checked + + if repoErr != nil { return model{}, fmt.Errorf("not a git repository (or git is unavailable)") } - branches, err := loadBranches() if err != nil { return model{}, err } - m := model{branches: branches, field: sortDate, ascending: false, height: 24, width: 100} - m.recomputeNameWidth() - m.refreshMergeInfo() - m.sortBranches() + m := model{field: sortDate, ascending: false, height: 24, width: 100} + m.applyBranches(branches, reads) return m, nil } @@ -507,6 +655,9 @@ func (m *model) scroll(delta int) { case stateDiff: m.diffTop += delta * 3 // 3 lines per wheel notch, like a pager m.clampDiff() + case stateConfirm, stateForcePrompt, stateDeleting, stateResult: + m.bodyTop += delta * 3 + m.clampBody() } } @@ -557,10 +708,10 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.status = "" return m, nil } - if branches, err := loadBranches(); err == nil { + if branches, reads, err := loadRepo(); err == nil { // A fetch is non-destructive, so both the cursor (by name, in // sortBranches) and the user's pending marks survive it. - m.applyBranches(carryMarks(m.branches, branches)) + m.applyBranches(carryMarks(m.branches, branches), reads) } // Auto-select only gone branches that carry nothing missing from the // base. Ones holding unique commits are left unselected so discarding @@ -595,6 +746,7 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } if m.deletesDone() >= len(m.results) { m.reloadBranches() + m.bodyTop = 0 if len(m.forceableFailures()) > 0 { m.state = stateForcePrompt } else { @@ -637,6 +789,8 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { switch msg.String() { case "q", "ctrl+c", "enter", "esc": return m, tea.Quit + default: + m.scrollKeys(msg.String()) } case stateHelp: if msg.String() == "ctrl+c" { @@ -721,7 +875,7 @@ func (m model) updateList(msg tea.KeyMsg) (tea.Model, tea.Cmd) { case "d", "enter": if len(m.selectedBranches()) > 0 { m.measureSelectedRisk() // the confirm screen states what each delete costs - m.state = stateConfirm + m.state, m.bodyTop = stateConfirm, 0 } } return m, nil @@ -741,6 +895,8 @@ func (m model) updateConfirm(msg tea.KeyMsg) (tea.Model, tea.Cmd) { m.state = stateList case "ctrl+c": return m, tea.Quit + default: + m.scrollKeys(msg.String()) } return m, nil } @@ -770,7 +926,7 @@ func (m *model) startDeletions(includeRemote bool) tea.Cmd { sel := m.selectedBranches() m.results = make([]deleteResult, len(sel)) m.spinnerFrame = 0 - m.state = stateDeleting + m.state, m.bodyTop = stateDeleting, 0 cmds := []tea.Cmd{spinnerTickCmd()} for i, b := range sel { @@ -808,11 +964,13 @@ func (m model) updateForcePrompt(msg tea.KeyMsg) (tea.Model, tea.Cmd) { switch msg.String() { case "y", "Y": m.forceDeleteUnmerged() - m.state = stateResult + m.state, m.bodyTop = stateResult, 0 case "n", "N", "esc", "q", "enter": - m.state = stateResult + m.state, m.bodyTop = stateResult, 0 case "ctrl+c": return m, tea.Quit + default: + m.scrollKeys(msg.String()) } return m, nil } @@ -857,8 +1015,8 @@ func (b branch) deleteFlag(force bool) string { } // deleteBranch runs one branch's local delete and, when wantRemote is set, its -// remote-branch push --delete. It is the single worker shared by the synchronous -// performDeletions path and the asynchronous deleteBranchCmd path. +// remote-branch push --delete. It is the worker deleteBranchCmd runs off the +// update loop, one cmd per branch. func deleteBranch(b branch, flag string, wantRemote bool) deleteResult { res := deleteResult{br: b, done: true} if _, err := runGit("branch", flag, b.name); err != nil { @@ -898,19 +1056,6 @@ func pushRemoteDelete(res *deleteResult) { } } -// performDeletions deletes the selected branches synchronously. The interactive -// UI uses the async startDeletions path instead; this remains for tests and as -// the straightforward equivalent. -func (m *model) performDeletions() { - m.measureSelectedRisk() - m.results = nil - for _, b := range m.selectedBranches() { - wantRemote := b.deleteRemote && b.upstream != "" - m.results = append(m.results, deleteBranch(b, b.deleteFlag(m.force), wantRemote)) - } - m.reloadBranches() -} - // carryMarks copies the user's pending selections from old onto a freshly loaded // branch set, matching by name. Used on the fetch path, which reloads every // branch struct but changes nothing the marks were made about. An armed remote @@ -934,73 +1079,115 @@ func carryMarks(old, fresh []branch) []branch { // derived from it (name-width, merge info, sort order). Callers set their own // cursor policy around it. This is the single refresh core shared by // reloadBranches and the fetch handler. -func (m *model) applyBranches(branches []branch) { +func (m *model) applyBranches(branches []branch, reads repoReads) { m.branches = branches m.recomputeNameWidth() - m.refreshMergeInfo() + m.refreshMergeInfo(reads) m.sortBranches() } // reloadBranches refreshes the branch list from git and resets the view to the // top — appropriate after a mutation that may have removed the cursor's branch. func (m *model) reloadBranches() { - if branches, err := loadBranches(); err == nil { + if branches, reads, err := loadRepo(); err == nil { m.cursor = 0 m.top = 0 - m.applyBranches(branches) + m.applyBranches(branches, reads) } } +// hasBranch reports whether the loaded list holds a local branch of that name. +func (m model) hasBranch(name string) bool { + for _, b := range m.branches { + if b.name == name { + return true + } + } + return false +} + // refreshMergeInfo caches the remote default branch, marks each branch whose // upstream is merged into it or whose tip is merged into HEAD, and measures what -// deleting it would cost. Call after every branch (re)load. -func (m *model) refreshMergeInfo() { - defRef := remoteDefault() - merged := remoteMergedSet(defRef) - headMerged := localMergedSet() +// deleting it would cost. Call after every branch (re)load, passing the reads +// that came back with it. +func (m *model) refreshMergeInfo(reads repoReads) { + defRef := remoteDefaultFrom(reads.remotes) m.riskBaseRef = defRef if m.riskBaseRef == "" { - m.riskBaseRef = localDefaultBranch("") + // The branch list is already loaded, so this candidate check is a lookup + // rather than a subprocess per name. + m.riskBaseRef = localDefaultBranch("", m.hasBranch) } // The refs drive git; the short forms are what the views print. m.remoteDefault = shortRef(defRef) m.riskBase = shortRef(m.riskBaseRef) + // Round two. Neither query needs the other's answer, and each is a + // subprocess start, so they go together. + var merged map[string]bool + remote := make(chan struct{}) + go func() { + merged = remoteMergedSet(defRef) + close(remote) + }() + // One query answers "is this branch's tip already in the base?" for the whole + // list, which is the answer for most branches a prune touches. + m.baseMerged = nil + if m.riskBaseRef != "" { + m.baseMerged = mergedSet("branch", "--merged", m.riskBaseRef) + } + <-remote + for i := range m.branches { b := &m.branches[i] b.remoteMerged = b.upstream != "" && merged[b.upstream] - b.headMerged = headMerged[b.name] + b.headMerged = reads.headMerged[b.name] b.riskMeasured = false // the branch was just reloaded; any old count is stale - // Only gone branches are measured up front, because `p` consults the count - // to decide what it may auto-select. The rest wait for measureSelectedRisk: - // riskCommitCount is a subprocess per branch, and running it for every - // unmergeable branch here cost a second of startup on a repo with dozens. - if b.gone { - m.measureRisk(b) - } } -} - -// measureRisk fills in b's cost-of-deletion count, once per (re)load. -func (m *model) measureRisk(b *branch) { - if b.riskMeasured { - return + // Only gone branches are measured up front, because `p` consults the count to + // decide what it may auto-select. The rest wait for measureSelectedRisk: + // riskCommitCount is a subprocess per branch, and measuring every unmergeable + // branch here would put a network-free repo's whole branch list on the clock. + m.measureRisk(func(b branch) bool { return b.gone }) +} + +// measureRisk fills in the cost-of-deletion count for every not-yet-measured +// branch that want accepts. The counts are independent `git cherry` subprocesses +// whose cost is dominated by process spawn, so they run concurrently: measured +// one at a time, a repo with a hundred gone branches spent 1.1s here on every +// load, fetch and prune. +func (m *model) measureRisk(want func(branch) bool) { + base := m.riskBaseRef // read once: the goroutines must not touch the model + var wg sync.WaitGroup + for i := range m.branches { + if m.branches[i].riskMeasured || !want(m.branches[i]) { + continue + } + // A branch already contained in the base has an empty base..branch range, + // so `git cherry` would report nothing. Answer from the set instead of + // spawning the subprocess. + if m.baseMerged[m.branches[i].name] { + m.branches[i].riskCommits = 0 + m.branches[i].riskMeasured = true + continue + } + wg.Add(1) + go func() { + defer wg.Done() + // Each goroutine owns one slice element, so no two write the same branch. + m.branches[i].riskCommits = riskCommitCount(m.branches[i].name, base) + m.branches[i].riskMeasured = true + }() } - b.riskCommits = riskCommitCount(b.name, m.riskBaseRef) - b.riskMeasured = true + wg.Wait() } // measureSelectedRisk measures what deleting each selected branch would cost. // Call before any view that reports the cost: only branches a safe delete would // refuse are measured, since those are the ones deleted with -D. func (m *model) measureSelectedRisk() { - for i := range m.branches { - b := &m.branches[i] - if b.selected && (b.gone || !b.safeDeletable()) { - m.measureRisk(b) - } - } + m.measureRisk(func(b branch) bool { return b.selected && (b.gone || !b.safeDeletable()) }) } // forceDeleteUnmerged re-runs the deletions that a safe (-d) delete refused, @@ -1048,6 +1235,99 @@ func (m model) View() string { } } +// page renders a screen as fixed header lines, a window into body, and fixed +// footer lines. The prompt on these screens is the whole point of them, and it +// lives in the footer: without a window, a wide selection pushes the question +// past the last row of the terminal, where the user cannot read what they are +// answering. Rendering only the visible rows is also what keeps a long list off +// the cost of every frame. +func (m model) page(header, body, footer []string) string { + rows := m.bodyRows(len(header), len(footer), len(body)) + top := max(0, min(m.bodyTop, len(body)-rows)) + end := min(top+rows, len(body)) + + var b strings.Builder + for _, l := range header { + b.WriteString(l + "\n") + } + for _, l := range body[top:end] { + b.WriteString(l + "\n") + } + if len(body) > rows { + b.WriteString(dimStyle.Render(fmt.Sprintf("[%d-%d / %d] ↑/↓ scroll · space/ctrl+d page · g/G top/bottom", + top+1, end, len(body))) + "\n") + } + for _, l := range footer { + b.WriteString(l + "\n") + } + return b.String() +} + +// bodyRows is how many body lines fit between header and footer. A body that +// does not fit gives up one more row to the position line, so that line never +// pushes the footer off in its turn. +func (m model) bodyRows(header, footer, body int) int { + rows := max(1, m.height-header-footer) + if body > rows { + rows = max(1, rows-1) + } + return rows +} + +// pageParts returns the current state's screen as header, body and footer. The +// scroll keys measure against it too, so the window and the clamp can never +// disagree about how far down the body goes. +func (m model) pageParts() (header, body, footer []string) { + switch m.state { + case stateConfirm: + return m.confirmParts() + case stateForcePrompt: + return m.forcePromptParts() + case stateDeleting: + return m.deletingParts() + case stateResult: + return m.resultParts() + } + return nil, nil, nil +} + +// bodyWindow reports the visible row count and the total body length for the +// current state. +func (m model) bodyWindow() (rows, total int) { + header, body, footer := m.pageParts() + return m.bodyRows(len(header), len(footer), len(body)), len(body) +} + +func (m *model) clampBody() { + rows, total := m.bodyWindow() + m.bodyTop = max(0, min(m.bodyTop, total-rows)) +} + +// scrollKeys applies the shared paging keys to a windowed screen. It reports +// whether the key was one of them, so each screen's own keys stay in charge: +// callers must offer their answers first. +func (m *model) scrollKeys(s string) bool { + rows, total := m.bodyWindow() + switch s { + case "up", "k": + m.bodyTop-- + case "down", "j": + m.bodyTop++ + case "ctrl+u", "pgup": + m.bodyTop -= max(1, rows/2) + case "ctrl+d", "pgdown", " ": + m.bodyTop += max(1, rows/2) + case "g", "home": + m.bodyTop = 0 + case "G", "end": + m.bodyTop = total + default: + return false + } + m.clampBody() + return true +} + func colorizeDiffLine(line string) string { switch { case strings.HasPrefix(line, "+++"), strings.HasPrefix(line, "---"): @@ -1272,6 +1552,7 @@ func (m model) helpView() string { {"d, enter", "delete selected branches (local)"}, {"", "on confirm: y = local only · R = local + remote"}, {"", "(unmerged -d failures prompt to retry with -D)"}, + {"", "long lists scroll: ↑/↓ · space/ctrl+d · g/G"}, {"?", "toggle this help screen"}, {"q, ctrl+c", "quit"}, }) @@ -1299,65 +1580,67 @@ func (m model) helpView() string { return b.String() } -func (m model) confirmView() string { - var b strings.Builder +func (m model) confirmParts() (header, body, footer []string) { sel := m.selectedBranches() - - b.WriteString(headerStyle.Render("Confirm deletion")) - b.WriteString("\n\n") flag := "-d (safe)" if m.force { flag = "-D (force)" } remoteCount := countArmedRemotes(sel) - b.WriteString(fmt.Sprintf("Local delete mode: %s\n", flag)) - b.WriteString(fmt.Sprintf("Deleting %d local branch(es), %d remote branch(es).\n\n", len(sel), remoteCount)) + header = []string{ + headerStyle.Render("Confirm deletion"), + "", + fmt.Sprintf("Local delete mode: %s", flag), + fmt.Sprintf("Deleting %d local branch(es), %d remote branch(es).", len(sel), remoteCount), + "", + } for _, br := range sel { - b.WriteString(" " + cursorStyle.Render("• "+br.name) + "\n") + body = append(body, " "+cursorStyle.Render("• "+br.name)) date := br.committed.Format("2006-Jan-02") if br.committedRel != "" { date += " (" + br.committedRel + ")" } - b.WriteString(" " + dimStyle.Render(fmt.Sprintf("%s %s %s", br.hash, date, truncate(br.subject, 50))) + "\n") + body = append(body, " "+dimStyle.Render(fmt.Sprintf("%s %s %s", br.hash, date, truncate(br.subject, 50)))) switch { case br.gone: - b.WriteString(" " + goneStyle.Render("upstream gone: "+br.upstream+" — will prune with -D (force)") + "\n") + body = append(body, " "+goneStyle.Render("upstream gone: "+br.upstream+" — will prune with -D (force)")) case br.upstream != "": - b.WriteString(" " + dimStyle.Render("upstream: "+br.upstream) + " " + m.trackStr(br) + "\n") + body = append(body, " "+dimStyle.Render("upstream: "+br.upstream)+" "+m.trackStr(br)) default: - b.WriteString(" " + dimStyle.Render("no upstream") + "\n") + body = append(body, " "+dimStyle.Render("no upstream")) } if br.upstream != "" && m.remoteDefault != "" { if br.remoteMerged { - b.WriteString(" " + okStyle.Render("✓ merged into "+m.remoteDefault) + "\n") + body = append(body, " "+okStyle.Render("✓ merged into "+m.remoteDefault)) } else { - b.WriteString(" " + goneStyle.Render("⚠ not merged into "+m.remoteDefault) + "\n") + body = append(body, " "+goneStyle.Render("⚠ not merged into "+m.remoteDefault)) } } if br.deleteRemote && br.upstream != "" { - b.WriteString(" " + errStyle.Render(fmt.Sprintf("+ delete remote %s/%s", br.remoteName(), br.remoteBranch())) + "\n") + body = append(body, " "+errStyle.Render(fmt.Sprintf("+ delete remote %s/%s", br.remoteName(), br.remoteBranch()))) } if w := m.riskWarning(br); w != "" { - b.WriteString(" " + errStyle.Render(w) + "\n") + body = append(body, " "+errStyle.Render(w)) } - b.WriteString("\n") + body = append(body, "") } - b.WriteString(headerStyle.Render("Delete these branches? ")) + prompt := headerStyle.Render("Delete these branches? ") if remoteCount > 0 { - b.WriteString(dimStyle.Render(fmt.Sprintf("(y = local only · R = local + remote (%d) · n/esc = cancel)", remoteCount))) + prompt += dimStyle.Render(fmt.Sprintf("(y = local only · R = local + remote (%d) · n/esc = cancel)", remoteCount)) } else { - b.WriteString(dimStyle.Render("(y = yes · n/esc = cancel)")) + prompt += dimStyle.Render("(y = yes · n/esc = cancel)") } - b.WriteString("\n") - return b.String() + return header, body, []string{prompt} } +func (m model) confirmView() string { return m.page(m.confirmParts()) } + // riskWarning states the cost of deleting br, or "" when the delete is clean. // It covers every branch git's safe delete would refuse plus gone branches, // which take the -D path regardless: under -D the unmerged commits are @@ -1382,92 +1665,92 @@ func (m model) riskWarning(br branch) string { return "⚠ not fully merged — safe delete (-d) will fail; use force (f)" } -func (m model) forcePromptView() string { - var b strings.Builder +func (m model) forcePromptParts() (header, body, footer []string) { failures := m.forceableFailures() - b.WriteString(headerStyle.Render("Force delete unmerged branches?")) - b.WriteString("\n\n") - b.WriteString(fmt.Sprintf("%d branch(es) were refused by safe delete (-d) because they are not\n", len(failures))) - b.WriteString("fully merged. Force deleting (-D) will ") - b.WriteString(errStyle.Render("permanently discard their unmerged commits")) - b.WriteString(".\n\n") + header = []string{ + headerStyle.Render("Force delete unmerged branches?"), + "", + fmt.Sprintf("%d branch(es) were refused by safe delete (-d) because they are not", len(failures)), + "fully merged. Force deleting (-D) will " + errStyle.Render("permanently discard their unmerged commits") + ".", + "", + } for _, r := range failures { - b.WriteString(" " + cursorStyle.Render("• "+r.br.name) + "\n") + body = append(body, " "+cursorStyle.Render("• "+r.br.name)) // Every branch here failed -d, so its risk was measured before the delete // ran: riskCommits == 0 means either nothing is missing from the base or // there was no base to measure against. switch { case r.br.riskCommits > 0: - b.WriteString(" " + errStyle.Render(fmt.Sprintf("⚠ %d commit(s) not in %s will be lost", r.br.riskCommits, m.riskBase)) + "\n") + body = append(body, " "+errStyle.Render(fmt.Sprintf("⚠ %d commit(s) not in %s will be lost", r.br.riskCommits, m.riskBase))) case m.riskBase == "": - b.WriteString(" " + errStyle.Render("⚠ no base branch to compare against — unmerged commits may be lost") + "\n") + body = append(body, " "+errStyle.Render("⚠ no base branch to compare against — unmerged commits may be lost")) default: - b.WriteString(" " + dimStyle.Render("no commits missing from "+m.riskBase) + "\n") + body = append(body, " "+dimStyle.Render("no commits missing from "+m.riskBase)) } if r.remoteSkipped { - b.WriteString(" " + errStyle.Render(fmt.Sprintf("+ remote %s/%s will be deleted once the branch is gone", r.br.remoteName(), r.br.remoteBranch())) + "\n") + body = append(body, " "+errStyle.Render(fmt.Sprintf("+ remote %s/%s will be deleted once the branch is gone", r.br.remoteName(), r.br.remoteBranch()))) } } - b.WriteString("\n") - b.WriteString(headerStyle.Render("Force delete (-D) these branches? ")) - b.WriteString(dimStyle.Render("(y = yes, discard · n/esc = keep them)")) - b.WriteString("\n") - return b.String() + footer = []string{ + "", + headerStyle.Render("Force delete (-D) these branches? ") + dimStyle.Render("(y = yes, discard · n/esc = keep them)"), + } + return header, body, footer } -// writeResultLines renders one completed deletion result (local, then remote if -// tried) into b. Shared by the results screen and the live deleting screen. -func writeResultLines(b *strings.Builder, r deleteResult) { +func (m model) forcePromptView() string { return m.page(m.forcePromptParts()) } + +// appendResultLines adds one completed deletion result (local, then remote if +// tried) to dst. Shared by the results screen and the live deleting screen. +func appendResultLines(dst []string, r deleteResult) []string { if r.localOK { - b.WriteString(okStyle.Render(" ✓ ") + "deleted local " + r.br.name + "\n") + dst = append(dst, okStyle.Render(" ✓ ")+"deleted local "+r.br.name) } else { - b.WriteString(errStyle.Render(" ✗ ") + "local " + r.br.name + ": " + r.localErr + "\n") + dst = append(dst, errStyle.Render(" ✗ ")+"local "+r.br.name+": "+r.localErr) } switch { case r.remoteSkipped: // Say why the armed remote survived, or it reads as a silent failure. - b.WriteString(errStyle.Render(" ! ") + "kept remote " + r.br.remoteName() + "/" + r.br.remoteBranch() + ": local delete failed\n") + dst = append(dst, errStyle.Render(" ! ")+"kept remote "+r.br.remoteName()+"/"+r.br.remoteBranch()+": local delete failed") case r.remoteTried && r.remoteOK: - b.WriteString(okStyle.Render(" ✓ ") + "deleted remote " + r.br.name + "\n") + dst = append(dst, okStyle.Render(" ✓ ")+"deleted remote "+r.br.name) case r.remoteTried: - b.WriteString(errStyle.Render(" ✗ ") + "remote " + r.br.name + ": " + r.remoteErr + "\n") + dst = append(dst, errStyle.Render(" ✗ ")+"remote "+r.br.name+": "+r.remoteErr) } + return dst } -func (m model) deletingView() string { - var b strings.Builder +func (m model) deletingParts() (header, body, footer []string) { spin := spinnerFrames[m.spinnerFrame%len(spinnerFrames)] - b.WriteString(headerStyle.Render(fmt.Sprintf("%s Deleting… (%d/%d)", spin, m.deletesDone(), len(m.results)))) - b.WriteString("\n\n") + header = []string{ + headerStyle.Render(fmt.Sprintf("%s Deleting… (%d/%d)", spin, m.deletesDone(), len(m.results))), + "", + } for _, r := range m.results { if r.done { - writeResultLines(&b, r) + body = appendResultLines(body, r) } else { - b.WriteString(dimStyle.Render(" "+spin+" deleting "+r.br.name+"…") + "\n") + body = append(body, dimStyle.Render(" "+spin+" deleting "+r.br.name+"…")) } } - b.WriteString("\n") - b.WriteString(dimStyle.Render("working — ctrl+c to abort")) - b.WriteString("\n") - return b.String() + return header, body, []string{"", dimStyle.Render("working — ctrl+c to abort")} } -func (m model) resultView() string { - var b strings.Builder - b.WriteString(headerStyle.Render("Results")) - b.WriteString("\n\n") +func (m model) deletingView() string { return m.page(m.deletingParts()) } + +func (m model) resultParts() (header, body, footer []string) { + header = []string{headerStyle.Render("Results"), ""} for _, r := range m.results { - writeResultLines(&b, r) + body = appendResultLines(body, r) } - b.WriteString("\n") - b.WriteString(dimStyle.Render("press q/enter to quit")) - b.WriteString("\n") - return b.String() + return header, body, []string{"", dimStyle.Render("press q/enter to quit")} } +func (m model) resultView() string { return m.page(m.resultParts()) } + // versionString reports the build's commit and date using Go's automatic VCS // stamping (populated when built with `go build` inside the repo). Fields fall // back to "unknown" when build info is unavailable (e.g. `go run`). diff --git a/main_test.go b/main_test.go index f72dace..d80b00c 100644 --- a/main_test.go +++ b/main_test.go @@ -4,9 +4,13 @@ import ( "fmt" "os" "os/exec" + "path/filepath" "regexp" + "strconv" "strings" + "sync" "testing" + "time" "unicode/utf8" tea "github.com/charmbracelet/bubbletea" @@ -16,18 +20,81 @@ import ( // key builds a rune KeyMsg (e.g. "y", "R") for driving update handlers in tests. func key(s string) tea.KeyMsg { return tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune(s)} } -// runCmd executes a tea.Cmd to completion, recursing into batched cmds, so tests -// can drive the async deletion path synchronously. -func runCmd(t *testing.T, cmd tea.Cmd) { +// deleteMsgs runs every cmd in a deletion batch concurrently — as the tea +// runtime does — and returns the branchDeletedMsgs they produce. The batch is +// one cmd per branch plus the spinner tick, which is dropped: it only animates, +// and waiting on its 120ms timer would slow every caller. +func deleteMsgs(t *testing.T, cmd tea.Cmd) []branchDeletedMsg { t.Helper() if cmd == nil { - return + t.Fatal("want a deletion batch cmd, got nil") } - if batch, ok := cmd().(tea.BatchMsg); ok { - for _, c := range batch { - runCmd(t, c) + msg := cmd() + batch, ok := msg.(tea.BatchMsg) + if !ok { + t.Fatalf("want a tea.BatchMsg, got %T", msg) + } + msgs := make(chan tea.Msg, len(batch)) + for _, c := range batch { + go func() { msgs <- c() }() + } + want := len(batch) - 1 + timeout := time.NewTimer(30 * time.Second) + defer timeout.Stop() + var out []branchDeletedMsg + for len(out) < want { + select { + case msg := <-msgs: + if dm, ok := msg.(branchDeletedMsg); ok { + out = append(out, dm) + } + case <-timeout.C: + t.Fatalf("timed out after %d of %d deletions reported", len(out), want) } } + return out +} + +// drainDeletions runs a deletion batch and feeds each result back through +// Update, so tests exercise the path users actually run: the completion logic +// (reload, then force-prompt vs result) decides the end state rather than the +// test asserting it into place. +func drainDeletions(t *testing.T, m model, cmd tea.Cmd) model { + t.Helper() + for _, dm := range deleteMsgs(t, cmd) { + nm, _ := m.Update(dm) + m = nm.(model) + } + return m +} + +// startAndDrain deletes m's selected branches and drives the run to completion. +// Starting the batch inside the call keeps m's copy and startDeletions' writes +// to it from being operands of one expression, where Go does not define which +// happens first. +func startAndDrain(t *testing.T, m model, includeRemote bool) model { + t.Helper() + return drainDeletions(t, m, m.startDeletions(includeRemote)) +} + +// selectAll marks every branch but the current one, arming the remote delete too +// when arm is set. +func selectAll(m *model, arm bool) { + for i := range m.branches { + if m.branches[i].isCurrent { + continue + } + m.branches[i].selected = true + m.branches[i].deleteRemote = arm + } +} + +// wantState asserts the state the machine landed in, with why it had to. +func wantState(t *testing.T, m model, want viewState, why string) { + t.Helper() + if m.state != want { + t.Fatalf("%s: state is %v, want %v", why, m.state, want) + } } // remoteHasBranch reports whether origin still has the named branch. @@ -95,6 +162,23 @@ func setupRepo(t *testing.T) string { return tmp } +// setupManyTracked builds a repo whose n feature branches each track origin and +// hold a commit of their own, so a prune of the lot fans out widely. +func setupManyTracked(t *testing.T, n int) string { + t.Helper() + tmp := initRepo(t, "main") + commitFile(t, tmp, "a", "a") + addOrigin(t, tmp, "main") + for i := 0; i < n; i++ { + name := fmt.Sprintf("feature/%d", i) + git(t, tmp, "checkout", "-q", "-b", name, "main") + commitFile(t, tmp, fmt.Sprintf("f%d", i), name) + } + git(t, tmp, "checkout", "-q", "main") + git(t, tmp, "push", "-q", "-u", "origin", "--all") // one connection, not n + return tmp +} + func chdir(t *testing.T, dir string) { t.Helper() old, _ := os.Getwd() @@ -204,7 +288,7 @@ func TestPruneGoneBranch(t *testing.T) { } find(m.branches, "feature/tracked").selected = true m.force = false - m.performDeletions() + m = startAndDrain(t, m, true) if len(m.results) != 1 || !m.results[0].localOK { t.Fatalf("gone branch should force-delete: %+v", m.results) @@ -239,7 +323,7 @@ func TestSafeDeleteRefusesUnmerged(t *testing.T) { find(m.branches, "feature/merged").selected = true find(m.branches, "feature/unmerged").selected = true m.force = false // safe -d - m.performDeletions() + m = startAndDrain(t, m, true) var merged, unmerged *deleteResult for i := range m.results { @@ -275,10 +359,11 @@ func TestForceDeleteUnmergedRetry(t *testing.T) { } find(m.branches, "feature/unmerged").selected = true m.force = false // safe -d, which will be refused - m.performDeletions() + m = startAndDrain(t, m, true) // The refused unmerged branch should be surfaced for a force prompt, with // its ahead count copied onto the result (0 here: it has no upstream). + wantState(t, m, stateForcePrompt, "a refused -d must raise the force prompt") failures := m.forceableFailures() if len(failures) != 1 || failures[0].br.name != "feature/unmerged" { t.Fatalf("want feature/unmerged in forceableFailures, got %+v", failures) @@ -290,8 +375,10 @@ func TestForceDeleteUnmergedRetry(t *testing.T) { t.Fatal("feature/unmerged should still exist before force retry") } - // Answering yes retries with -D and clears the branch. - m.forceDeleteUnmerged() + // Answering yes at the prompt retries with -D and clears the branch. + nm, _ := m.Update(key("y")) + m = nm.(model) + wantState(t, m, stateResult, "answering the prompt must land on the results screen") if len(m.forceableFailures()) != 0 { t.Fatalf("no failures should remain after force retry: %+v", m.results) } @@ -317,7 +404,7 @@ func TestNoForcePromptForCleanDelete(t *testing.T) { } find(m.branches, "feature/merged").selected = true m.force = false - m.performDeletions() + m = startAndDrain(t, m, true) if len(m.results) != 1 || !m.results[0].localOK { t.Fatalf("merged branch should delete cleanly: %+v", m.results) @@ -349,7 +436,7 @@ func TestGoneFailureNotForceable(t *testing.T) { } tb.selected = true m.force = false // gone branches still use -D - m.performDeletions() + m = startAndDrain(t, m, true) if len(m.results) != 1 || !m.results[0].localOK { t.Fatalf("gone branch should force-delete: %+v", m.results) @@ -371,8 +458,9 @@ func TestForceDeleteUnmergedNoop(t *testing.T) { } find(m.branches, "feature/merged").selected = true m.force = false - m.performDeletions() + m = startAndDrain(t, m, true) before := len(m.branches) + wantState(t, m, stateResult, "a clean delete must skip the force prompt") m.forceDeleteUnmerged() // no forceable failures — should change nothing if len(m.forceableFailures()) != 0 { @@ -398,7 +486,7 @@ func TestForceDeleteAndRemote(t *testing.T) { tb.selected = true tb.deleteRemote = true m.force = true // -D, also needed since tracked has its own commit - m.performDeletions() + m = startAndDrain(t, m, true) if len(m.results) != 1 { t.Fatalf("want 1 result, got %d", len(m.results)) @@ -447,6 +535,254 @@ func TestDeleteBranchCmd(t *testing.T) { } } +// runGit is where the cap lives, so callers cannot escape it however many pile +// in at once — and it still runs them in parallel rather than one at a time. +func TestGitCallsAreCapped(t *testing.T) { + chdir(t, setupLocalRepo(t)) + + const callers = 40 + gitProcs.peak.Store(0) + var wg sync.WaitGroup + for i := 0; i < callers; i++ { + wg.Add(1) + go func() { + defer wg.Done() + runGit("rev-parse", "--git-dir") + }() + } + wg.Wait() + + peak := gitProcs.peak.Load() + if peak > maxLocalGit { + t.Fatalf("%d callers ran %d git processes at once, cap is %d", callers, peak, maxLocalGit) + } + if peak < 2 { + t.Fatalf("the cap serialized everything: peak %d", peak) + } + if n := gitProcs.inFlight.Load(); n != 0 { + t.Fatalf("every slot should be released, %d still held", n) + } +} + +// Pruning a wide selection must not open a connection per branch: remotes +// throttle or refuse a burst, and a failed push is a branch deleted locally +// whose only other copy is still out there. +func TestWideDeleteStaysWithinCaps(t *testing.T) { + const n = 20 + repo := setupManyTracked(t, n) + chdir(t, repo) + + m, err := initialModel() + if err != nil { + t.Fatal(err) + } + selectAll(&m, true) + m.force = true + + gitProcs.peak.Store(0) + netProcs.peak.Store(0) + m = startAndDrain(t, m, true) + + // Literal bounds, not the constants under test: raising a cap must break this + // test rather than move the goalposts with it. + const fewConnections, fewProcesses = 4, 12 + if maxRemotePush > fewConnections { + t.Fatalf("maxRemotePush is %d; a prune should never ask a remote for more than %d connections", maxRemotePush, fewConnections) + } + if got := netProcs.peak.Load(); got > fewConnections { + t.Fatalf("%d branches opened %d simultaneous connections to the remote", n, got) + } + if got := gitProcs.peak.Load(); got > fewProcesses { + t.Fatalf("%d branches ran %d git processes at once", n, got) + } + if netProcs.peak.Load() == 0 { + t.Fatal("no pushes were observed; the test is not measuring the remote path") + } + // The cap must not cost any of them their deletion. + for _, r := range m.results { + if !r.localOK || !r.remoteOK { + t.Fatalf("throttled delete did not complete: %+v", r) + } + } + if len(m.branches) != 1 { + t.Fatalf("only the current branch should remain, got %v", branchNames(m.branches)) + } +} + +// Risk is measured concurrently, one goroutine per branch writing its own slice +// element. Each branch must end up with its own count, not a neighbour's. +func TestConcurrentRiskMeasurementKeepsCountsWithBranch(t *testing.T) { + repo := initRepo(t, "main") + chdir(t, repo) + commitFile(t, repo, "a", "a") + + // feature/i carries i commits of its own. Branching off one chain costs n + // commits for the n distinct counts; a fresh branch each time costs n(n+1)/2. + const n = 12 + git(t, repo, "checkout", "-q", "-b", "chain", "main") + for i := 1; i <= n; i++ { + commitFile(t, repo, fmt.Sprintf("f%d", i), "x") + git(t, repo, "branch", fmt.Sprintf("feature/%d", i)) + } + git(t, repo, "checkout", "-q", "main") + git(t, repo, "branch", "-D", "chain") + + m, err := initialModel() + if err != nil { + t.Fatal(err) + } + selectAll(&m, false) + m.measureSelectedRisk() + + for i := 1; i <= n; i++ { + name := fmt.Sprintf("feature/%d", i) + b := find(m.branches, name) + if b == nil || !b.riskMeasured { + t.Fatalf("%s was not measured: %+v", name, b) + } + if b.riskCommits != i { + t.Fatalf("%s should hold %d commits, measured %d", name, i, b.riskCommits) + } + } +} + +// The live path only leaves stateDeleting once every branch has reported, and +// the branch list is not reloaded before then — a mid-run reload would renumber +// the results the outstanding messages are still indexing into. +func TestDeletingWaitsForEveryResult(t *testing.T) { + repo := setupRepo(t) + chdir(t, repo) + + m, err := initialModel() + if err != nil { + t.Fatal(err) + } + find(m.branches, "feature/merged").selected = true // -d succeeds + find(m.branches, "feature/unmerged").selected = true // -d is refused + m.force = false + + msgs := deleteMsgs(t, m.startDeletions(false)) + if len(msgs) != 2 { + t.Fatalf("want 2 results, got %d", len(msgs)) + } + for i, dm := range msgs { + nm, _ := m.Update(dm) + m = nm.(model) + if !m.results[dm.idx].done { + t.Fatalf("result %d should be marked done: %+v", dm.idx, m.results[dm.idx]) + } + if i == len(msgs)-1 { + break + } + wantState(t, m, stateDeleting, "state must hold at stateDeleting until the last result") + if find(m.branches, "feature/merged") == nil { + t.Fatal("the branch list must not be reloaded mid-run") + } + } + wantState(t, m, stateForcePrompt, "the refused -d should raise the force prompt") + if find(m.branches, "feature/merged") != nil { + t.Fatal("the final result should have reloaded the branch list") + } +} + +// A result carrying an index outside the current run must be dropped rather +// than panic: the async cmds outlive nothing here, but the bounds check is the +// only thing standing between a stale message and an out-of-range write. +func TestStrayDeleteResultIsIgnored(t *testing.T) { + repo := setupRepo(t) + chdir(t, repo) + + m, err := initialModel() + if err != nil { + t.Fatal(err) + } + find(m.branches, "feature/merged").selected = true + m.startDeletions(false) + + nm, _ := m.Update(branchDeletedMsg{idx: 7, res: deleteResult{done: true}}) + m = nm.(model) + wantState(t, m, stateDeleting, "a stray result must not complete the run") + if m.deletesDone() != 0 { + t.Fatalf("a stray result must not be counted, got %d", m.deletesDone()) + } +} + +// Declining the force prompt leaves the refused branch — and its commits — in +// place. This is the escape hatch the -d/-D split exists for. +func TestForcePromptDeclineKeepsBranch(t *testing.T) { + repo := setupRepo(t) + chdir(t, repo) + + m, err := initialModel() + if err != nil { + t.Fatal(err) + } + find(m.branches, "feature/unmerged").selected = true + m.force = false + m = startAndDrain(t, m, true) + wantState(t, m, stateForcePrompt, "a refused -d must raise the force prompt") + + nm, _ := m.Update(key("n")) + m = nm.(model) + wantState(t, m, stateResult, "declining must land on the results screen") + if find(m.branches, "feature/unmerged") == nil { + t.Fatal("declining the force prompt must keep the branch") + } + if len(m.forceableFailures()) != 1 { + t.Fatalf("the refusal must still be reported: %+v", m.results) + } +} + +// While deletions are in flight the keyboard is inert except for ctrl+c, so a +// stray keystroke cannot dismiss a run whose results have not landed yet. +func TestDeletingIgnoresKeysExceptCtrlC(t *testing.T) { + repo := setupRepo(t) + chdir(t, repo) + + m, err := initialModel() + if err != nil { + t.Fatal(err) + } + m.state = stateDeleting + + for _, k := range []tea.KeyMsg{key("q"), key("y"), key("d")} { + nm, cmd := m.Update(k) + if got := nm.(model).state; got != stateDeleting { + t.Fatalf("%v must not change state, got %v", k, got) + } + if cmd != nil { + t.Fatalf("%v must not issue a cmd", k) + } + } + + _, cmd := m.Update(tea.KeyMsg{Type: tea.KeyCtrlC}) + if cmd == nil { + t.Fatal("ctrl+c must abort") + } + if _, ok := cmd().(tea.QuitMsg); !ok { + t.Fatalf("ctrl+c should quit, got %T", cmd()) + } +} + +// The spinner re-arms itself only while deletions are running; once the run has +// landed the tick must die out rather than loop forever. +func TestSpinnerTickStopsAfterDeleting(t *testing.T) { + m := model{state: stateDeleting} + + nm, cmd := m.Update(spinnerTickMsg{}) + if nm.(model).spinnerFrame != 1 { + t.Fatalf("frame should advance, got %d", nm.(model).spinnerFrame) + } + if cmd == nil { + t.Fatal("the tick must re-arm while deleting") + } + + m.state = stateResult + if _, cmd := m.Update(spinnerTickMsg{}); cmd != nil { + t.Fatal("the tick must not re-arm once the run has landed") + } +} + // #4: on the confirm screen, 'y' deletes locals only while 'R' also deletes the // armed remote. func TestConfirmRemoteConfirmationSplit(t *testing.T) { @@ -465,13 +801,9 @@ func TestConfirmRemoteConfirmationSplit(t *testing.T) { m.state = stateConfirm nm, cmd := m.updateConfirm(key("y")) - if nm.(model).state != stateDeleting { - t.Fatalf("state should be stateDeleting, got %v", nm.(model).state) - } - if cmd == nil { - t.Fatal("expected a deletion batch cmd") - } - runCmd(t, cmd) + wantState(t, nm.(model), stateDeleting, "state should be stateDeleting") + m = drainDeletions(t, nm.(model), cmd) + wantState(t, m, stateResult, "a clean -D run should land on the results screen") if !remoteHasBranch(t, repo, "feature/tracked") { t.Fatal("'y' must not delete the remote branch") } @@ -492,10 +824,9 @@ func TestConfirmRemoteConfirmationSplit(t *testing.T) { m.state = stateConfirm nm, cmd := m.updateConfirm(key("R")) - if nm.(model).state != stateDeleting { - t.Fatalf("state should be stateDeleting, got %v", nm.(model).state) - } - runCmd(t, cmd) + wantState(t, nm.(model), stateDeleting, "state should be stateDeleting") + m = drainDeletions(t, nm.(model), cmd) + wantState(t, m, stateResult, "a clean -D run should land on the results screen") if remoteHasBranch(t, repo, "feature/tracked") { t.Fatal("'R' should delete the remote branch") } @@ -774,13 +1105,16 @@ func TestNonOriginRemoteResolves(t *testing.T) { } } -// origin is preferred when several remotes are configured. +// origin is preferred when several remotes are configured. The names come out +// of refs/remotes now, so the second remote has to be fetched to count — one +// that has never been fetched holds no ref a default branch could resolve to. func TestRemotesPrefersOrigin(t *testing.T) { repo := setupRepo(t) chdir(t, repo) git(t, repo, "remote", "add", "aaa-fork", repo) - got := remotes() + git(t, repo, "fetch", "-q", "aaa-fork") + got := loadRemoteRefs().names if len(got) == 0 || got[0] != "origin" { t.Fatalf("origin should sort first, got %v", got) } @@ -905,9 +1239,7 @@ func TestUnmergedBranchWithoutUpstreamIsWarned(t *testing.T) { b.selected = true nm, _ := m.updateList(key("d")) m = nm.(model) - if m.state != stateConfirm { - t.Fatalf("'d' should open the confirm screen, got %v", m.state) - } + wantState(t, m, stateConfirm, "'d' should open the confirm screen") b = find(m.branches, "feature/unmerged") if b.riskCommits != 1 { t.Fatalf("want the branch's 1 unique commit measured, got %d", b.riskCommits) @@ -967,7 +1299,7 @@ func TestForcePromptStatesCommitCount(t *testing.T) { } find(m.branches, "feature/unmerged").selected = true m.force = false - m.performDeletions() + m = startAndDrain(t, m, true) failures := m.forceableFailures() if len(failures) != 1 { @@ -977,7 +1309,7 @@ func TestForcePromptStatesCommitCount(t *testing.T) { t.Fatalf("the refused result must carry its measured cost, got %+v", failures[0]) } - m.state = stateForcePrompt + wantState(t, m, stateForcePrompt, "a refused -d must raise the force prompt") out := stripANSI(m.forcePromptView()) if !strings.Contains(out, fmt.Sprintf("1 commit(s) not in %s will be lost", m.riskBase)) { t.Fatalf("force prompt must state the commit count:\n%s", out) @@ -1007,7 +1339,7 @@ func TestRemoteDeleteDeferredUntilLocalSucceeds(t *testing.T) { tb.selected = true tb.deleteRemote = true m.force = false // safe delete, which git will refuse - m.performDeletions() + m = startAndDrain(t, m, true) r := m.results[0] if r.localOK { @@ -1027,7 +1359,9 @@ func TestRemoteDeleteDeferredUntilLocalSucceeds(t *testing.T) { } // The force retry clears the branch, so the arming is finally honoured. - m.forceDeleteUnmerged() + wantState(t, m, stateForcePrompt, "a refused -d must raise the force prompt") + nm, _ := m.Update(key("y")) + m = nm.(model) r = m.results[0] if !r.localOK || !r.remoteTried || !r.remoteOK || r.remoteSkipped { t.Fatalf("force retry should complete both deletes: %+v", r) @@ -1122,7 +1456,7 @@ func TestTagShadowingBranchName(t *testing.T) { // And the delete must land on the branch, leaving the tag alone. m.force = true - m.performDeletions() + m = startAndDrain(t, m, true) if !m.results[0].localOK { t.Fatalf("delete failed: %s", m.results[0].localErr) } @@ -1148,7 +1482,7 @@ func TestTagShadowingBranchName(t *testing.T) { git(t, repo, "tag", "main", "feature") git(t, repo, "checkout", "-q", "trunk") - if got := localDefaultBranch(""); got != "" { + if got := localDefaultBranch("", gitHasBranch); got != "" { t.Fatalf("a tag must not pose as the local default branch, got %q", got) } m, err := initialModel() @@ -1183,7 +1517,7 @@ func TestTagShadowingBranchName(t *testing.T) { tb.selected = true tb.deleteRemote = true m.force = true - m.performDeletions() + m = startAndDrain(t, m, true) r := m.results[0] if !r.localOK { @@ -1343,7 +1677,7 @@ func TestGoneCurrentBranchIsNotPruned(t *testing.T) { // Selected by hand, git refuses — a failure -D cannot rescue, so it must not // raise the force prompt offering a retry that fails identically. b.selected = true - m.performDeletions() + m = startAndDrain(t, m, true) if len(m.results) != 1 || m.results[0].localOK { t.Fatalf("deleting the checked-out branch must fail: %+v", m.results) } @@ -1375,7 +1709,7 @@ func TestLocalOnlyRepo(t *testing.T) { repo := setupLocalRepo(t) chdir(t, repo) - if got := remotes(); len(got) != 0 { + if got := loadRemoteRefs().names; len(got) != 0 { t.Fatalf("no remotes should be configured, got %v", got) } if got := remoteDefault(); got != "" { @@ -1418,7 +1752,7 @@ func TestLocalOnlyRepo(t *testing.T) { // above (it survives the fetch), so clear it first. find(m.branches, "feature/unmerged").selected = false find(m.branches, "feature/merged").selected = true - m.performDeletions() + m = startAndDrain(t, m, true) if len(m.results) != 1 || !m.results[0].localOK { t.Fatalf("merged branch should delete cleanly: %+v", m.results) } @@ -1508,7 +1842,7 @@ func TestDetachedHead(t *testing.T) { nm, _ = m.updateList(key("n")) m = nm.(model) find(m.branches, "main").selected = true - m.performDeletions() + m = startAndDrain(t, m, true) if len(m.results) != 1 || !m.results[0].localOK { t.Fatalf("main should delete cleanly while detached: %+v", m.results) } @@ -1555,11 +1889,11 @@ func TestNonStandardDefaultBranch(t *testing.T) { // The same holds on the force prompt reached after a refused safe delete. m.force = false - m.performDeletions() + m = startAndDrain(t, m, true) if len(m.forceableFailures()) != 1 { t.Fatalf("the unmerged branch should be refused and forceable: %+v", m.results) } - m.state = stateForcePrompt + wantState(t, m, stateForcePrompt, "a refused -d must raise the force prompt") if out := stripANSI(m.forcePromptView()); !strings.Contains(out, "no base branch to compare against") { t.Fatalf("force prompt must carry the warning:\n%s", out) } @@ -1623,7 +1957,7 @@ func TestUnreachableRemote(t *testing.T) { tb = find(m.branches, "feature/tracked") tb.selected = true tb.deleteRemote = true - m.performDeletions() + m = startAndDrain(t, m, true) r := m.results[0] if !r.localOK { @@ -1723,7 +2057,7 @@ func TestRemoteDeleteRace(t *testing.T) { } tb.selected = true tb.deleteRemote = true - m.performDeletions() + m = startAndDrain(t, m, true) r := m.results[0] if !r.localOK { @@ -1845,7 +2179,7 @@ func TestUnicodeNameAndEmptySubject(t *testing.T) { // And it deletes end to end, the name surviving the round trip to git. b.selected = true m.force = true - m.performDeletions() + m = startAndDrain(t, m, true) if !m.results[0].localOK { t.Fatalf("delete failed: %s", m.results[0].localErr) } @@ -1863,3 +2197,197 @@ func TestVersionString(t *testing.T) { } } } + +// setupGoneMerged builds n branches that are merged into main and report their +// upstream as gone. The upstream is configured rather than pushed and pruned: +// git reads "gone" straight out of the config when the remote-tracking ref is +// absent, which keeps the fixture free of network work. +func setupGoneMerged(t *testing.T, n int) string { + t.Helper() + tmp := initRepo(t, "main") + commitFile(t, tmp, "a", "a") + git(t, tmp, "remote", "add", "origin", filepath.Join(t.TempDir(), "absent.git")) + for i := 0; i < n; i++ { + name := fmt.Sprintf("feature/%d", i) + git(t, tmp, "checkout", "-q", "-b", name, "main") + commitFile(t, tmp, fmt.Sprintf("f%d", i), name) + git(t, tmp, "checkout", "-q", "main") + git(t, tmp, "merge", "-q", "--no-ff", name, "-m", "m"+name) + git(t, tmp, "config", "branch."+name+".remote", "origin") + git(t, tmp, "config", "branch."+name+".merge", "refs/heads/"+name) + } + return tmp +} + +// A branch whose tip is already in the base has an empty base..branch range, so +// `git cherry` can only report nothing. One `branch --merged` query answers that +// for the whole list, and it has to stay one query: a subprocess per branch put +// half a second on the load of every repo full of pruned branches. +func TestMergedBranchesCostNoSubprocess(t *testing.T) { + const n = 30 + chdir(t, setupGoneMerged(t, n)) + + before := gitProcs.total.Load() + m, err := initialModel() + if err != nil { + t.Fatal(err) + } + spawned := gitProcs.total.Load() - before + + // A literal bound, not a formula over n: a fan-out that comes back must fail + // this rather than scale along with it. + const fewProcesses = 12 + if spawned > fewProcesses { + t.Fatalf("loading %d merged branches ran %d git processes; it should not grow with the branch count", n, spawned) + } + + measured := 0 + for _, b := range m.branches { + if !b.gone { + continue + } + if !b.riskMeasured { + t.Fatalf("%s was never measured, so p cannot tell whether it is safe to select", b.name) + } + if b.riskCommits != 0 { + t.Fatalf("%s is merged into the base; the shortcut must agree with git cherry, got %d", b.name, b.riskCommits) + } + measured++ + } + if measured != n { + t.Fatalf("expected %d gone branches, saw %d", n, measured) + } +} + +// The shortcut must not swallow the branches it cannot answer for: one that is +// merged and one that is not have to come back with the counts git cherry gives. +func TestUnmergedBranchIsStillMeasured(t *testing.T) { + repo := setupGoneMerged(t, 1) // feature/0: merged, gone + chdir(t, repo) + + git(t, repo, "checkout", "-q", "-b", "feature/kept", "main") + commitFile(t, repo, "kept", "kept") + git(t, repo, "checkout", "-q", "main") + git(t, repo, "config", "branch.feature/kept.remote", "origin") + git(t, repo, "config", "branch.feature/kept.merge", "refs/heads/feature/kept") + + m, err := initialModel() + if err != nil { + t.Fatal(err) + } + if b := find(m.branches, "feature/0"); b == nil || b.riskCommits != 0 { + t.Fatalf("the merged branch should cost nothing: %+v", b) + } + kept := find(m.branches, "feature/kept") + if kept == nil || !kept.riskMeasured || kept.riskCommits != 1 { + t.Fatalf("the unmerged branch still needs its real count: %+v", kept) + } +} + +// screenRows counts the rows a rendered view occupies. +func screenRows(view string) int { return len(strings.Split(strings.TrimRight(view, "\n"), "\n")) } + +// The confirm screen asks a question whose answer deletes branches. A wide +// selection used to render every branch in full, pushing that question past the +// last row of the terminal — the user answered a prompt they could not read. +func TestConfirmPromptStaysOnScreen(t *testing.T) { + const n = 30 + chdir(t, setupGoneMerged(t, n)) + + m, err := initialModel() + if err != nil { + t.Fatal(err) + } + selectAll(&m, false) + next, _ := m.Update(key("d")) + m = next.(model) + wantState(t, m, stateConfirm, "d with a selection opens the confirm screen") + + view := m.confirmView() + if rows := screenRows(view); rows > m.height { + t.Fatalf("%d selected branches rendered %d rows into a %d-row terminal", n, rows, m.height) + } + if !strings.Contains(stripANSI(view), "Delete these branches?") { + t.Fatalf("the prompt must be on the screen:\n%s", stripANSI(view)) + } +} + +// Windowing must not hide a branch from the user: every one of them has to be +// reachable by scrolling, and the prompt has to stay put while they do it. +func TestConfirmScrollReachesTheLastBranch(t *testing.T) { + const n = 30 + chdir(t, setupGoneMerged(t, n)) + + m, err := initialModel() + if err != nil { + t.Fatal(err) + } + selectAll(&m, false) + m.state = stateConfirm + + sel := m.selectedBranches() + last := sel[len(sel)-1].name // the confirm list follows the sort order + if strings.Contains(stripANSI(m.confirmView()), last) { + t.Fatalf("%s should start below the window; the fixture proves nothing", last) + } + + next, _ := m.Update(key("G")) + m = next.(model) + view := stripANSI(m.confirmView()) + if !strings.Contains(view, last) { + t.Fatalf("G must reach the last branch:\n%s", view) + } + if !strings.Contains(view, "Delete these branches?") { + t.Fatalf("the prompt must survive scrolling:\n%s", view) + } + if rows := screenRows(m.confirmView()); rows > m.height { + t.Fatalf("the scrolled screen is %d rows in a %d-row terminal", rows, m.height) + } + + // The answer keys still answer; they are not swallowed by the scroll handler. + next, _ = m.Update(key("n")) + wantState(t, next.(model), stateList, "n cancels from a scrolled confirm screen") +} + +// The results screen has the same shape and the same failure: a wide prune used +// to bury "press q/enter to quit" under its own output. +func TestResultViewWindowsItsBody(t *testing.T) { + m := model{height: 24, width: 100, state: stateResult} + for i := 0; i < 40; i++ { + m.results = append(m.results, deleteResult{ + br: branch{name: "feature/" + strconv.Itoa(i)}, done: true, localOK: true, + }) + } + if rows := screenRows(m.resultView()); rows > m.height { + t.Fatalf("40 results rendered %d rows into a %d-row terminal", rows, m.height) + } + if !strings.Contains(stripANSI(m.resultView()), "press q/enter to quit") { + t.Fatal("the footer must stay on the screen") + } + + next, _ := m.Update(key("G")) + m = next.(model) + if o := stripANSI(m.resultView()); !strings.Contains(o, "feature/39") { + t.Fatalf("G must reach the last result:\n%s", o) + } +} + +// A body that fits needs no position line and no scrolling — the common case +// must not grow furniture it does not need. +func TestShortBodyRendersWhole(t *testing.T) { + m := model{height: 24, width: 100, state: stateResult} + for i := 0; i < 3; i++ { + m.results = append(m.results, deleteResult{ + br: branch{name: "feature/" + strconv.Itoa(i)}, done: true, localOK: true, + }) + } + out := stripANSI(m.resultView()) + if strings.Contains(out, "scroll") { + t.Fatalf("a body that fits should show no position line:\n%s", out) + } + for i := 0; i < 3; i++ { + if !strings.Contains(out, "feature/"+strconv.Itoa(i)) { + t.Fatalf("every result must be shown:\n%s", out) + } + } +}