Skip to content

Commit 808e37a

Browse files
authored
Merge pull request #112 from specsnl/fix/prune-prompt-height
Show every prune candidate at once in the selection prompt
2 parents f7659ee + f0f8a0c commit 808e37a

5 files changed

Lines changed: 163 additions & 17 deletions

File tree

‎docs/content/docs/architecture/apply.md‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,24 @@ which is what `--yes` does, returns the plan unchanged.
5252
Repositories survive a selection that empties them. One whose candidates were all declined is still a
5353
repository the run visited, and still applies its creates and updates.
5454

55+
### Every candidate is on screen
56+
57+
The prompt is built by `pruneForm`, and the shape it builds is not the library default. The heading
58+
and the warning sit on the **group**, and the `MultiSelect` carries an explicit `Height` of one line
59+
per candidate.
60+
61+
A field left to size itself fits its list viewport to the option lines and then subtracts its own
62+
title and description from that, so three candidates under a two-line description leave a viewport
63+
one row tall: one repository visible, the rest reachable only by arrowing through a line that
64+
redraws in place. On a destructive prompt that is worse than ugly — the list *is* the report being
65+
decided from, and a report that shows one of three rows while claiming to be the whole question
66+
invites an enter on a selection nobody saw.
67+
68+
Hoisting the text to the group keeps it out of that subtraction, and the group still clamps the list
69+
to the window when the whole form genuinely does not fit — the one case where scrolling is the right
70+
answer. `internal/cmd/prune_form_internal_test.go` renders the form at fixed window sizes and asserts
71+
both halves.
72+
5573
## The order is a crash-consistency guarantee
5674

5775
Actions go out in exactly the order the planner emitted them: renames, then squatter recolours, then

‎docs/content/docs/architecture/library-decisions.md‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ Two rules decide most of it:
2323
| `github.com/spf13/cobra` | The command tree | `internal/cmd` | yes |
2424
| `gopkg.in/yaml.v3` | Config parsing and export rendering | `internal/config` | yes |
2525
| `charm.land/huh/v2` | The prune `MultiSelect` | `internal/cmd/prune.go` | yes |
26+
| `charm.land/bubbletea/v2` | `WindowSizeMsg`, in the prune prompt's test | `internal/cmd/prune_form_internal_test.go` | **new (direct, test only)** — indirect via huh |
2627
| `charm.land/lipgloss/v2` | Diff colouring, tables, the countdown | `internal/util/output`, `internal/plan`, `internal/github/ratelimit` | yes |
2728
| `github.com/adrg/xdg` | Config and cache directory resolution | `internal/labelsync` | yes |
2829
| `github.com/danwakefield/fnmatch` | Repository `include` / `exclude` globs | `internal/config/resolve.go` | yes |
@@ -36,7 +37,7 @@ Two rules decide most of it:
3637
`log/slog` carries debug diagnostics, from the standard library — see
3738
[Output § Debug logging]({{< ref "./output.md#debug-logging" >}}).
3839

39-
The three "new (direct)" rows are libraries the module already pulled in through lipgloss. Naming
40+
The "new (direct)" rows are libraries the module already pulled in through lipgloss. Naming
4041
them in `go.mod` does not add a dependency; it stops the code depending on a transitive one it
4142
cannot see, which is what turns a lipgloss minor upgrade into a compile error somewhere unrelated.
4243

@@ -82,7 +83,14 @@ prompt that answers "yes" for the whole list is the wrong shape for that decisio
8283
usually "these three, not those two". `huh.MultiSelect` is that answer, and it is already the form
8384
library specs-cli uses.
8485

85-
It is confined to one file, `internal/cmd/prune.go`, behind `App.Prompt` so tests replace it
86+
`bubbletea` is a direct dependency for one reason, and only in a test: the prompt's layout is a
87+
function of the terminal size, so `internal/cmd/prune_form_internal_test.go` hands the form a
88+
`tea.WindowSizeMsg` and reads the render back. Going through huh's own resize path is the point —
89+
the sizing bug it guards against
90+
([Apply § Every candidate is on screen]({{< ref "./apply.md#every-candidate-is-on-screen" >}})) only
91+
appears there. No non-test file imports it.
92+
93+
`huh` is confined to one file, `internal/cmd/prune.go`, behind `App.Prompt` so tests replace it
8694
wholesale, and it is never reached without a terminal on stdin — a prune with nobody to ask fails
8795
with `interactive_required` rather than blocking a CI job. See
8896
[Apply § Prune: the selection]({{< ref "./apply.md#prune-the-selection" >}}).

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ module github.com/specsnl/labelsync
33
go 1.26.5
44

55
require (
6+
charm.land/bubbletea/v2 v2.0.2
67
charm.land/huh/v2 v2.0.3
78
charm.land/lipgloss/v2 v2.0.6
89
github.com/adrg/xdg v0.5.3
@@ -19,7 +20,6 @@ require (
1920

2021
require (
2122
charm.land/bubbles/v2 v2.0.0 // indirect
22-
charm.land/bubbletea/v2 v2.0.2 // indirect
2323
github.com/atotto/clipboard v0.1.4 // indirect
2424
github.com/catppuccin/go v0.2.0 // indirect
2525
github.com/charmbracelet/ultraviolet v0.0.0-20260811164956-006e29f97886 // indirect

‎internal/cmd/prune.go‎

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -113,32 +113,47 @@ func (a *App) prompt() Selector {
113113
return a.multiSelect
114114
}
115115

116-
// multiSelect is the real prompt: a huh.MultiSelect over the candidates, in plan
117-
// order, with nothing pre-selected.
116+
// pruneForm builds the prompt: a huh.MultiSelect over the candidates, in plan
117+
// order, with nothing pre-selected, writing the chosen ones through selected.
118118
//
119119
// Nothing is pre-selected on purpose. A prompt that arrives with every box ticked
120120
// turns an accidental enter into a full prune, and the whole reason prune is
121121
// report-first is that the accident is not undoable.
122122
//
123-
// It draws on **stderr** rather than stdout, like every other thing that narrates
124-
// a run: stdout is the product, and a `--output=json | jq` pipeline must not
125-
// receive a redrawn form in the middle of its stream. It reads stdin, which
126-
// [App.canPrompt] has already established is a terminal.
127-
func (a *App) multiSelect(ctx context.Context, candidates []plan.Candidate) ([]plan.Candidate, error) {
123+
// The heading and the warning sit on the *group*, not on the field, and the field
124+
// carries an explicit Height of one line per candidate. Both halves of that are
125+
// load-bearing. A field with no Height sizes its own list viewport to the option
126+
// lines and then subtracts its title and description from that, so a
127+
// three-candidate prompt under a two-line description ends up with a viewport of
128+
// a single row: one repository visible, the rest reachable only by arrowing
129+
// through a line that redraws in place. Hoisting the text to the group keeps it
130+
// out of that subtraction, and the group still shrinks the list to fit a short
131+
// terminal — huh clamps the field to the window height when the whole form does
132+
// not fit, which is the one case where scrolling is the right answer.
133+
func pruneForm(candidates []plan.Candidate, selected *[]plan.Candidate) *huh.Form {
128134
options := make([]huh.Option[plan.Candidate], 0, len(candidates))
129135
for _, candidate := range candidates {
130136
options = append(options, huh.NewOption(candidate.Repo+" "+candidate.Name, candidate))
131137
}
132138

133-
var selected []plan.Candidate
134-
135-
form := huh.NewForm(huh.NewGroup(
139+
return huh.NewForm(huh.NewGroup(
136140
huh.NewMultiSelect[plan.Candidate]().
137-
Title(pruneTitle).
138-
Description(pruneDescription).
139141
Options(options...).
140-
Value(&selected),
141-
)).WithInput(a.Stdin).WithOutput(a.Stderr)
142+
Height(len(options)).
143+
Value(selected),
144+
).Title(pruneTitle).Description(pruneDescription))
145+
}
146+
147+
// multiSelect is the real prompt.
148+
//
149+
// It draws on **stderr** rather than stdout, like every other thing that narrates
150+
// a run: stdout is the product, and a `--output=json | jq` pipeline must not
151+
// receive a redrawn form in the middle of its stream. It reads stdin, which
152+
// [App.canPrompt] has already established is a terminal.
153+
func (a *App) multiSelect(ctx context.Context, candidates []plan.Candidate) ([]plan.Candidate, error) {
154+
var selected []plan.Candidate
155+
156+
form := pruneForm(candidates, &selected).WithInput(a.Stdin).WithOutput(a.Stderr)
142157

143158
// RunWithContext rather than Run, so ^C at the shell and a cancelled run stop
144159
// the same way: the context the whole command tree is executed with is the one
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
package cmd
2+
3+
// This file is an internal test — the only one in the package — because what it
4+
// checks is how the prune prompt is *rendered*, and a form is not something the
5+
// App seam can hand back. Everything else about prune is behaviour, and is tested
6+
// through the command in prune_test.go.
7+
8+
import (
9+
"fmt"
10+
"strings"
11+
"testing"
12+
13+
tea "charm.land/bubbletea/v2"
14+
15+
"github.com/specsnl/labelsync/internal/plan"
16+
)
17+
18+
// candidatesFor is n removal candidates over n repositories, all carrying the same
19+
// label name — the shape the bug showed up in: one line per repository, and every
20+
// line reading almost identically, so a viewport of one row looks like a prompt
21+
// with a single entry rather than a list that has been cut off.
22+
func candidatesFor(n int) []plan.Candidate {
23+
out := make([]plan.Candidate, 0, n)
24+
for i := range n {
25+
out = append(out, plan.Candidate{Repo: fmt.Sprintf("specsnl/php%02d", 83+i), Name: "dependencies"})
26+
}
27+
28+
return out
29+
}
30+
31+
// render draws the form once at a fixed terminal size. The window size is what
32+
// decides whether the list has to scroll, so it is the input under test, not an
33+
// incidental detail of the harness.
34+
func render(t *testing.T, candidates []plan.Candidate, width, height int) string {
35+
t.Helper()
36+
37+
var selected []plan.Candidate
38+
39+
model, _ := pruneForm(candidates, &selected).Update(tea.WindowSizeMsg{Width: width, Height: height})
40+
41+
return fmt.Sprint(model.View())
42+
}
43+
44+
// TestPruneFormShowsEveryCandidate is the regression: with room on screen, every
45+
// candidate is on screen. The prompt used to render a one-row viewport regardless
46+
// of how much space there was, because the field subtracted its own title and
47+
// description from a height derived from the option lines alone.
48+
func TestPruneFormShowsEveryCandidate(t *testing.T) {
49+
t.Parallel()
50+
51+
for _, count := range []int{1, 3, 20} {
52+
t.Run(fmt.Sprintf("%d candidates", count), func(t *testing.T) {
53+
t.Parallel()
54+
55+
candidates := candidatesFor(count)
56+
view := render(t, candidates, 120, 60)
57+
58+
for _, candidate := range candidates {
59+
if !strings.Contains(view, candidate.Repo) {
60+
t.Errorf("%s is missing from the prompt:\n%s", candidate.Repo, view)
61+
}
62+
}
63+
})
64+
}
65+
}
66+
67+
// TestPruneFormFitsAShortTerminal is the other half of the same setting: the list
68+
// is allowed to scroll when it genuinely cannot fit, and stays within a line of
69+
// the window when it does.
70+
//
71+
// Within a line, and not exactly within the window, because huh sizes the group
72+
// from the header and the help footer and does not count the blank line it puts
73+
// between them — so a form that has to scroll renders one line taller than the
74+
// height it was given. That is huh's arithmetic, not a height this package
75+
// chooses, and one line of overdraw on a terminal too short for the prompt is not
76+
// worth computing the window size here to work around.
77+
func TestPruneFormFitsAShortTerminal(t *testing.T) {
78+
t.Parallel()
79+
80+
const height = 12
81+
82+
view := render(t, candidatesFor(40), 120, height)
83+
lines := len(strings.Split(strings.TrimRight(view, "\n"), "\n"))
84+
85+
if lines > height+1 {
86+
t.Errorf("prompt is %d lines tall in a %d-line terminal:\n%s", lines, height, view)
87+
}
88+
89+
if !strings.Contains(view, "specsnl/php83") {
90+
t.Errorf("the first candidate is not visible:\n%s", view)
91+
}
92+
}
93+
94+
// TestPruneFormSelectsNothingByDefault guards the pre-selection decision from the
95+
// build-up of options the form is configured with: enter on an untouched prompt
96+
// deletes nothing.
97+
func TestPruneFormSelectsNothingByDefault(t *testing.T) {
98+
t.Parallel()
99+
100+
view := render(t, candidatesFor(3), 120, 60)
101+
102+
if strings.Contains(view, "✓") {
103+
t.Errorf("a candidate arrives pre-selected:\n%s", view)
104+
}
105+
}

0 commit comments

Comments
 (0)