Skip to content

feat(tui): command palette and overlay compositor (#71, #59) - #142

Merged
marcoarnulfo merged 13 commits into
mainfrom
feat/command-palette
Jul 30, 2026
Merged

marcoarnulfo merged 13 commits into
mainfrom
feat/command-palette

Conversation

@marcoarnulfo

Copy link
Copy Markdown
Owner

Fifth tranche of the v1.9 TUI design system milestone. Adds the ctrl+p command palette and the overlay compositor that lets anything float over a screen instead of replacing it.

Closes #71. Closes #59.

What changes for a user

  • ctrl+p opens a fuzzy command palette that floats over the current screen. The screen underneath stays visible around the box — that is the point, since you pick a command by looking at what you are working on. Type a few letters, ↑/↓ to move, enter to run, esc to close.
  • It offers two kinds of row. The current screen's own commands, with the same keys the footer advertises; and one-step navigation to any other screen (Go to rates, Go to budgets, …). With an empty query the screen's commands come first — "what can I do here" above "take me elsewhere".
  • The matched letters light up, so you can see why a row matched. exp highlights [Exp]ort and Go to [exp]ort.

Notable decisions

The screen's commands are derived from its keymap, not written out a second time. screenActions walks screenKeys(m).paletteBindings() — a curated twin of the existing allBindings() — and three properties fall out for free: the gating (a disabled binding simply is not offered, so the same Enabled() that decides the footer decides the palette), the wording (the label is the footer's own description, so the two surfaces cannot drift), and the behavior (run replays the key through routeKey, so there is no duplicated handler to diverge). The replay rests on an invariant — every palette binding's first key round-trips through keyMsgFor — and TestEveryPaletteBindingIsReplayable walks every screen and mode to make it load-bearing rather than lucky.

Global actions are navigation only. Anything that changes state stays a screen action, where the footer teaches it and the palette repeats it. That line is what keeps the second source short and predictable; #140 is where it gets a considered exception.

Which meant extracting the screen-opening methods first. A "Go to export" cannot just goTo(screenExport) — it has to build the sub-model too, which was the key handler's job. Copying those bodies would have been exactly the second source of truth the derivation avoids, so six openX() methods moved into internal/tui/open.go, in the shape openListBrowser() already had. Handler and palette action now call the same method.

internal/fuzzy is 90 lines of our own rather than a dependency. Not NIH: the matched indices are part of the UI contract, because they decide which letters light up, and that means the match has to be optimal rather than greedy. Greedy rt against report table returns [0, 5] and highlights repor[t]; the right answer is [0, 7], [r]eport [t]able. It is a small dynamic program with a deterministic tie-break, pure, and testable without a Model.

The box is built line by line rather than through th.Box. For an overlay the rendered width is not decoration — it is the number composite uses to decide where the body resumes. The previous tranche already paid once for trusting a library's border arithmetic (table.Width() counting only column separators and then amputating the right border), so every line here is boxW cells wide by construction, with a test that checks it.

? stays as the expanded footer. #59 named the ? overlay among the compositor's intended clients, and #69 closed with a note saying promoting it later would be "a change to one function". We are not promoting it, and not because it costs anything: a help panel that covers the very screen you are asking about is worse than a footer that covers nothing. That decision is recorded in the spec as final rather than deferred, so it stops being an open question instead of becoming an issue nobody closes.

replace() clears the overlay only when the screen actually changes. The spec's rule was "an overlay belongs to the screen it was raised over; if that screen is swapped out from under it, it dissolves" — and the plan implemented that as "clear in all four nav.go functions", which is broader than the rule. Five handlers call replace(<the screen they are already on>) as a routine refresh; with the broad version, a statusesMsg landing from the fetch openFilters had just launched would close the palette and wipe the query with nothing having changed on screen. The review caught it, it contradicted the plan text, and the ruling was that the finding governs.

On the tests

Three more tests were caught passing against the very bug they existed to catch — the same failure mode that produced four in the previous tranche. All three came from the plan's own text, which is to say from me:

  • TestPaletteHighlightSurvivesTruncation was reasoned backwards. It picked a query whose matches all landed far past the truncation point, on the theory that this exercised the drop-the-indices branch. It does the opposite: indices far past the cut are never read, because the render loop only walks the truncated label. The one observable index is the ellipsis column. A reviewer proved it by mutating highlight to drop nothing at all and watching the whole suite stay green.
  • The feature's entry point had no test. Deleting if key.Matches(msg, keysFor(m).Palette) { return m.openPalette(), nil } from Update left go test ./... completely green — ctrl+p would have stopped opening the palette and nothing would have noticed. Worse, the test that said t.Fatal("ctrl+p did not open the palette") never pressed ctrl+p.
  • The demo-parity assertion could not fire. It checked msg != nil, and both the demo commands and the real ones always return a non-nil tea.Msg — the real one returns a statusesMsg with empty cached entries, or a retryableErrMsg, on any network failure. Removing the if m.demo guard would have left the test green while the code dialed out with the fake token, breaking the exact no-I/O promise the test existed to protect.

There is a fourth of the same family that was caught in the plan, before any code: TestPaletteQueryAcceptsQ was prescribed as proof that the overlay check must sit above Quit, and it cannot be — paletteKeys leaves Quit unassigned, so q reaches the query whatever the order. The plan's own second guard neutralised the plan's own red-green step. It is now documented as a regression test, and TestPaletteCtrlPClosesRatherThanReopening is the one with teeth, because ctrl+p misplaced would reopen the palette and wipe the query.

The final review ran 27 mutations; 24 were caught. The three that survived became three new tests. One of them was hiding a real defect-in-waiting: with refreshPalette's cursor reset removed, moving the cursor low and then typing a character that shrinks the list leaves idx past the end, and enter panics on items[idx]. The code was already correct — nothing tested it.

Every test written against a specific bug on this branch was verified failing against that bug before being accepted, with the transcript attached, and the reviewers reproduced the load-bearing ones independently.

Verification

gofmt, go vet, staticcheck, go build, go test ./... -race all clean. go.mod gains exactly one line — charmbracelet/x/ansi promoted from indirect to direct, already in the module graph via lipgloss, so nothing new is downloaded and go.sum is untouched. Demo-mode parity has its own test. All four new goldens and three frames of the regenerated docs/demo.gif were inspected by eye, not just diffed. No Co-Authored-By trailer on any of the 13 commits.

Also swept in: twelve British spellings across the repo, five of them introduced by this branch's own plan text.

Follow-ups filed

#140 (the palette's month/list/member switcher, split out of #71's goal line into v1.12), #141 (footer.go's clampWidth builds a style on the default renderer — pre-existing from #69, behaviorally inert, and invisible to the suite because TestMain pins the profile).

🤖 Generated with Claude Code

Address review findings on the ctrl+p command palette: strengthen two
vacuous tests (TestPaletteHighlightSurvivesTruncation matched only past
the observable window; TestPaletteOpensAndClosesWithoutTouchingNav never
actually pressed ctrl+p), stop nav.go's overlay-dissolving note from
becoming the package doc comment, and — the behavioral fix — make
replace() dissolve the overlay only when the screen actually changes, so
an async refresh landing back on the SAME screen (statusesMsg, membersMsg,
entriesReloadedMsg/entriesErrMsg, logDoneMsg/timerMsg) no longer wipes a
palette query the user is still typing. Deduplicates the repeated
overlay-clearing pair in nav.go behind closePalette().
…ose coverage gaps

Final fix wave after the whole-branch review of the command palette (#71):
rewrite three comments that named the wrong mechanism (app.go's Quit-vs-Palette
ordering, overlay.go's straddle guard twice), document composite's rectangular-
box precondition, normalize British spellings introduced by this branch plus a
handful of pre-existing ones, add three tests a mutation pass showed missing
(refreshPalette's cursor reset, the query line's shaveToWidth, openPalette
clearing helpAll), and fix two README nits (Invio/Enter consistency, the demo
tape's temporary-PATH build step).
@marcoarnulfo
marcoarnulfo merged commit f0c275d into main Jul 30, 2026
4 checks passed
@marcoarnulfo
marcoarnulfo deleted the feat/command-palette branch July 30, 2026 05:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Command palette (ctrl+p) Keymap registry + navigation stack + overlay compositor (foundations)

1 participant