diff --git a/README.md b/README.md index 1e9dfa4..c69ff33 100644 --- a/README.md +++ b/README.md @@ -70,6 +70,7 @@ Everything boardwalk needs to know lives in one file at | `project` | the team project within it | | `reviewGroups` | optional — extra teams and security groups to treat as yours, named as Azure DevOps displays them | | `paletteKey` | optional — the key that opens the [command palette](#command-palette), spelled `ctrl+k`, `:`, `f1`; defaults to `ctrl+p` | +| `commands` | optional — your own [custom commands](#custom-commands), each a key on a kind of screen | Azure DevOps scopes a group's display name to wherever it lives — `[TEAM FOUNDATION]\platform-devs` for a collection group, `[MyProject]\Team @@ -158,6 +159,62 @@ menu's `↑`/`↓`/`j`/`k`), at startup. A key a view also binds is taken by the palette on that view, so pick one that is free. Like every other key, it is left to a prompt while one is open. +### Custom commands + +Bind a key on a kind of screen to a shell command of your own. boardwalk runs +it with `sh -c` (`cmd /C` on Windows), detached — it keeps the screen and reports on the status line +whether the command ran, failed (with the last line it wrote to stderr), or is +still going after two seconds and has been left to run. + +```json +"commands": [ + { + "on": "pullRequest", + "key": "ctrl+r", + "name": "review with claude", + "run": "~/bin/pr-review.sh" + } +] +``` + +`on` is the kind of screen: `pullRequest` (the list, the pull request, its +diff and files), `workItem` (the list and the item) or `build` (the list and +the logs). The command appears in the `?` panel and the palette there. + +What the screen shows reaches the command as environment variables, never +pasted into the command, so a title full of shell syntax stays a title: + +| variable | on | | +|---|---|---| +| `BOARDWALK_KIND` | all | `pullRequest`, `workItem` or `build` | +| `BOARDWALK_ORG`, `BOARDWALK_PROJECT` | all | from the config | +| `BOARDWALK_ID`, `BOARDWALK_TITLE`, `BOARDWALK_URL` | all | the selected thing | +| `BOARDWALK_REPO`, `BOARDWALK_SOURCE_BRANCH`, `BOARDWALK_TARGET_BRANCH`, `BOARDWALK_AUTHOR`, `BOARDWALK_IS_DRAFT` | pullRequest | branches without `refs/heads/` | +| `BOARDWALK_TYPE`, `BOARDWALK_STATE` | workItem | | +| `BOARDWALK_NUMBER`, `BOARDWALK_PIPELINE`, `BOARDWALK_BRANCH`, `BOARDWALK_RESULT` | build | | + +The command also inherits boardwalk's own environment, so a terminal +multiplexer's variables pass straight through. For example, a script that +opens a [herdr](https://herdr.dev) tab beside boardwalk and starts a Claude +Code review there (it needs `jq`; adjust the last line to your own review +skill): + +```sh +#!/bin/sh +# ~/bin/pr-review.sh +pane=$(herdr tab create --label "review !$BOARDWALK_ID" --cwd "$PWD" --focus | + jq -r .result.root_pane.pane_id) +herdr pane run "$pane" "claude '/review-pr $BOARDWALK_ID'" +``` + +A key is one key, spelled as for `paletteKey`. boardwalk refuses at startup a +key it needs itself, one lists or panes move with (`j`, `k`, `l`, `h`, `b`, +`u`, `f`, `d`, `g`, `G`, `/`, space, `^d`, `^u`, the arrows, `home`, `end`, +page keys), one every screen uses (`r`, `y`, `^y`, `s`, `^s`, `o`, `^o`), or +the palette key. `^r` is free. A key a single screen already +uses keeps doing what it did there, and the command is left out of that +screen's help. + ### Every list view Work items, pull requests and builds share these. The log pane is a pager diff --git a/docs/superpowers/plans/2026-10-06-custom-commands.md b/docs/superpowers/plans/2026-10-06-custom-commands.md new file mode 100644 index 0000000..f2b455a --- /dev/null +++ b/docs/superpowers/plans/2026-10-06-custom-commands.md @@ -0,0 +1,1538 @@ +# Custom Commands Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Let the config bind a single key, on a kind of screen, to a user shell command that runs detached with the screen's subject in `BOARDWALK_*` environment variables. + +**Architecture:** `config` gains a `Command` list and validates what it can alone. `ui` gains a runner (`custom.go`, `sh -c`, detached, reports within a 2 s window), a `subject` that eight views expose through an optional `Subject()` method, and Root-level dispatch: Root matches the key before the view does, skips it while the view is prompting or binds the key itself, runs the command, and delivers the outcome as a `StatusMsg` to the view it ran on — wherever that view now sits in the stack. Three views that mishandle `StatusMsg` today (Logs ignores it; PullRequestDetail and Item drop its error flag) are fixed first. Custom commands join the `?` panel, and so the palette, through a `help.KeyMap` wrapper like the existing `paletteKeys`. + +**Tech Stack:** Go 1.27, bubbletea, bubbles (`key`, `help`, `list`), `os/exec`. + +**Spec:** `docs/superpowers/specs/2026-10-06-custom-commands-design.md` + +## Global Constraints + +- Kinds are exactly `pullRequest`, `workItem`, `build`. +- Variables are `BOARDWALK_` + `KIND`, `ORG`, `PROJECT`, `ID`, `TITLE`, `URL`; pullRequest adds `REPO`, `SOURCE_BRANCH`, `TARGET_BRANCH`, `AUTHOR`, `IS_DRAFT`; workItem adds `TYPE`, `STATE`; build adds `NUMBER`, `PIPELINE`, `BRANCH`, `RESULT`. Branches have `refs/heads/` stripped. +- Values reach the command only as environment variables — never spliced into the command string. +- Commands run detached: stdin `/dev/null`, stdout discarded, stderr captured (capped), own process group; boardwalk never waits more than `commandWait` (2 s). +- Status texts, verbatim: `running ""…`, `ran ""`, `started ""`, `"" failed: `, `could not run "": `, `nothing selected`. +- A built-in key always wins over a custom one; reserved keys, list-movement keys and the palette key are refused at startup. +- CI builds on Windows and macOS: unix-only syscall fields go in a `//go:build !windows` file; Windows runs `cmd /C`. Tests that need `sh` skip on Windows. +- Code style: match the repo — doc comments that explain *why*, table-driven tests, indirected package variables for side effects (`copyToClipboard` pattern). +- Commit messages: conventional (`feat:`, `docs:`, `test:`), ending with the attribution lines from the session. + +## Review Focus + +1. **A key the list moves with but does not list in `Keys()`** (`l`, `h`, `b`, `u`, `f`, `d`, `pgup`…) — expected: refused at startup, not silently stealing paging on list views. Pinned in Task 5 (`TestCheckCommandsRefusesListMovementKeys`). +2. **The custom key pressed while a vote or draft toggle is armed** — expected: the arm sees the key (cancels), the command does not run. Pinned in Task 5 (`TestRootSkipsACommandWhileAVoteIsArmed`). +3. **Navigating away before the result arrives** — expected: the result is not shown as the status of a different screen, and is waiting on the screen it ran on when the user comes back. Pinned in Task 5 (`TestRootDeliversAResultToTheViewItRanOn`). +4. **A command that writes a lot to stderr, over many lines** — expected: only the last line is shown, memory stays bounded. Pinned in Task 2 (`TestStartShellReportsOnlyTheLastStderrLine`). +5. **A title holding shell syntax** (`$(touch pwned)`) — expected: inert. Pinned in Task 2 (`TestStartShellNeverEvaluatesValues`). + +--- + +## File Structure + +| file | responsibility | +|------|----------------| +| `internal/config/config.go` (modify) | `Command`, kind constants, `ValidateCommands` | +| `internal/ui/custom.go` (create) | runner: `startShell`, `runShell` var, `commandWait`, `tailBuffer` | +| `internal/ui/custom_unix.go` (create) | `shellCommand` for unix: `sh -c`, `Setpgid` | +| `internal/ui/custom_windows.go` (create) | `shellCommand` for Windows: `cmd /C`, new process group | +| `internal/ui/subject.go` (create) | `subject`, `subjecter`, the three subject builders | +| `internal/ui/rootcommands.go` (create) | `WithCommands`, `CheckCommands`, dispatch, help wrapper | +| 8 view files (modify) | one `Subject()` method each; Logs, PullRequestDetail and Item also learn to show a `StatusMsg` error | +| `internal/ui/root.go`, `rootpalette.go` (modify) | call into rootcommands.go | +| `main.go` (modify) | wire validation and `WithCommands` | +| `README.md` (modify) | document `commands` | + +--- + +### Task 1: Config — the `commands` list + +**Files:** +- Modify: `internal/config/config.go` +- Test: `internal/config/config_test.go` + +**Interfaces:** +- Produces: + - `type Command struct { On, Key, Name, Run string }` (json `on`, `key`, `name`, `run`) + - `const KindPullRequest = "pullRequest"`, `KindWorkItem = "workItem"`, `KindBuild = "build"` + - `var CommandKinds = []string{KindPullRequest, KindWorkItem, KindBuild}` + - `Config.Commands []Command` (json `commands,omitempty`) + - `func (c Config) ValidateCommands(path string) error` + +- [ ] **Step 1: Write the failing tests** — append to `internal/config/config_test.go`: + +```go +func TestLoadReadsCommands(t *testing.T) { + t.Setenv(EnvPath, writeConfig(t, `{ + "org": "acme", "project": "Platform", + "commands": [{"on": "pullRequest", "key": "ctrl+r", "name": "review", "run": "echo hi"}] + }`)) + + c, err := Load() + if err != nil { + t.Fatalf("Load returned %v", err) + } + want := Command{On: "pullRequest", Key: "ctrl+r", Name: "review", Run: "echo hi"} + if len(c.Commands) != 1 || c.Commands[0] != want { + t.Errorf("commands = %+v, want [%+v]", c.Commands, want) + } +} + +func TestValidateCommands(t *testing.T) { + ok := Command{On: KindPullRequest, Key: "ctrl+r", Name: "review", Run: "echo hi"} + with := func(edit func(*Command)) Command { c := ok; edit(&c); return c } + + for _, tc := range []struct { + name string + cmds []Command + want string // substring of the error; empty means valid + }{ + {"none", nil, ""}, + {"one good", []Command{ok}, ""}, + {"same key on two kinds", []Command{ok, with(func(c *Command) { c.On = KindBuild })}, ""}, + {"unknown kind", []Command{with(func(c *Command) { c.On = "pr" })}, `on "pr"`}, + {"no key", []Command{with(func(c *Command) { c.Key = "" })}, "sets no key"}, + {"no name", []Command{with(func(c *Command) { c.Name = " " })}, "sets no name"}, + {"no run", []Command{with(func(c *Command) { c.Run = "" })}, "sets no run"}, + {"duplicate", []Command{ok, ok}, "as commands[0] already does"}, + } { + t.Run(tc.name, func(t *testing.T) { + err := Config{Commands: tc.cmds}.ValidateCommands("/cfg.json") + switch { + case tc.want == "" && err != nil: + t.Errorf("got %v, want valid", err) + case tc.want != "" && (err == nil || !strings.Contains(err.Error(), tc.want)): + t.Errorf("got %v, want an error containing %q", err, tc.want) + case tc.want != "" && !strings.Contains(err.Error(), "/cfg.json"): + t.Errorf("%v does not name the file", err) + } + }) + } +} +``` + +- [ ] **Step 2: Run to verify failure** + +Run: `go test ./internal/config/ -run 'TestLoadReadsCommands|TestValidateCommands'` +Expected: FAIL — `undefined: Command`. + +- [ ] **Step 3: Implement** — in `internal/config/config.go`, add `"slices"` to imports, add the field to `Config` after `PaletteKey`: + +```go + // Commands bind a key on one kind of screen to a shell command of the + // user's own. See Command. + Commands []Command `json:"commands,omitempty"` +``` + +and below `Config`: + +```go +// The kinds of screen a command can be bound on, named for what the screen +// shows rather than which view shows it: a pull request is a pull request +// whether it is a row in the list or the pane it opens. +const ( + KindPullRequest = "pullRequest" + KindWorkItem = "workItem" + KindBuild = "build" +) + +// CommandKinds is every kind a command's On may name. +var CommandKinds = []string{KindPullRequest, KindWorkItem, KindBuild} + +// Command is one user-defined action: on a screen of kind On, the key Key +// runs Run with sh -c. Name is what the help panel, the palette and the +// status line call it. +// +// Run is the only action there is. Another kind of action would be a field +// beside it rather than a type tag, so a config written today keeps meaning +// what it says. +type Command struct { + On string `json:"on"` + Key string `json:"key"` + Name string `json:"name"` + Run string `json:"run"` +} + +// ValidateCommands reports the first command that cannot work, naming the +// file and the command's place in the list. Whether Key names a real key, and +// one boardwalk does not need itself, is ui.CheckCommands' job: only the UI +// knows its keys. +func (c Config) ValidateCommands(path string) error { + seen := make(map[[2]string]int, len(c.Commands)) + for i, cmd := range c.Commands { + where := fmt.Sprintf("%s: commands[%d]", path, i) + switch { + case !slices.Contains(CommandKinds, cmd.On): + return fmt.Errorf("%s has on %q — use one of %s", where, cmd.On, strings.Join(CommandKinds, ", ")) + case cmd.Key == "": + return fmt.Errorf("%s sets no key", where) + case strings.TrimSpace(cmd.Name) == "": + return fmt.Errorf("%s sets no name", where) + case strings.TrimSpace(cmd.Run) == "": + return fmt.Errorf("%s sets no run", where) + } + k := [2]string{cmd.On, cmd.Key} + if j, dup := seen[k]; dup { + return fmt.Errorf("%s binds %q on %s, as commands[%d] already does", where, cmd.Key, cmd.On, j) + } + seen[k] = i + } + return nil +} +``` + +- [ ] **Step 4: Run to verify pass** + +Run: `go test ./internal/config/` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add internal/config/ +git commit -m "feat: read custom commands from the config" +``` + +--- + +### Task 2: The runner + +**Files:** +- Create: `internal/ui/custom.go`, `internal/ui/custom_unix.go`, `internal/ui/custom_windows.go` +- Test: `internal/ui/custom_test.go` + +**Interfaces:** +- Produces: + - `var runShell func(name, script string, env []string) StatusMsg` (defaults to `startShell`) + - `var commandWait = 2 * time.Second` + - `func shellCommand(script string) *exec.Cmd` (per platform) + +- [ ] **Step 1: Write the failing tests** — `internal/ui/custom_test.go`: + +```go +package ui + +import ( + "os" + "path/filepath" + "runtime" + "strings" + "testing" + "time" +) + +// needSh skips a test that runs a real shell script on a platform without sh. +func needSh(t *testing.T) { + t.Helper() + if runtime.GOOS == "windows" { + t.Skip("the runner tests drive sh") + } +} + +func shortWait(t *testing.T, d time.Duration) { + t.Helper() + before := commandWait + commandWait = d + t.Cleanup(func() { commandWait = before }) +} + +func TestStartShellPassesTheEnvironment(t *testing.T) { + needSh(t) + out := filepath.Join(t.TempDir(), "env") + + got := startShell("dump", `env > "$OUT"`, []string{"OUT=" + out, "BOARDWALK_ID=4021"}) + + if got.Err || got.Text != `ran "dump"` { + t.Fatalf("status = %+v", got) + } + body, err := os.ReadFile(out) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(body), "BOARDWALK_ID=4021\n") { + t.Errorf("the command did not see BOARDWALK_ID:\n%s", body) + } + if !strings.Contains(string(body), "PATH=") { + t.Error("the command did not inherit boardwalk's environment") + } +} + +func TestStartShellReportsFailure(t *testing.T) { + needSh(t) + for _, tc := range []struct{ script, want string }{ + {`echo "no such tab" >&2; exit 3`, `"go" failed: no such tab`}, + {`exit 3`, `"go" failed: exit status 3`}, + } { + got := startShell("go", tc.script, nil) + if !got.Err || got.Text != tc.want { + t.Errorf("%s: status = %+v, want error %q", tc.script, got, tc.want) + } + } +} + +func TestStartShellReportsOnlyTheLastStderrLine(t *testing.T) { + needSh(t) + // Ten thousand lines of noise and then the reason: the status line has + // room for one, and the last is the one a failing script ends on. + got := startShell("noisy", `i=0; while [ $i -lt 10000 ]; do echo "noise $i" >&2; i=$((i+1)); done; echo "the reason" >&2; exit 1`, nil) + if got.Text != `"noisy" failed: the reason` { + t.Errorf("status = %q", got.Text) + } +} + +func TestStartShellStopsWaitingOnALongCommand(t *testing.T) { + needSh(t) + shortWait(t, 100*time.Millisecond) + + start := time.Now() + got := startShell("slow", "sleep 5", nil) + + if got.Err || got.Text != `started "slow"` { + t.Errorf("status = %+v", got) + } + if time.Since(start) > 2*time.Second { + t.Error("the runner waited on the command instead of detaching") + } +} + +func TestStartShellNeverEvaluatesValues(t *testing.T) { + needSh(t) + dir := t.TempDir() + // A title is written by somebody else. Quoted as a variable it is data. + got := startShell("t", `printf '%s' "$BOARDWALK_TITLE" > /dev/null`, + []string{"BOARDWALK_TITLE=$(touch " + filepath.Join(dir, "pwned") + ")"}) + if got.Err { + t.Fatalf("status = %+v", got) + } + if _, err := os.Stat(filepath.Join(dir, "pwned")); err == nil { + t.Error("a value was run as shell") + } +} + +func TestTailBufferKeepsTheEnd(t *testing.T) { + var b tailBuffer + for range 100 { + b.Write([]byte(strings.Repeat("x", 100) + "\n")) + } + b.Write([]byte("last\n\n")) + if len(b.b) > tailLimit { + t.Errorf("buffer grew to %d bytes", len(b.b)) + } + if b.lastLine() != "last" { + t.Errorf("lastLine = %q", b.lastLine()) + } +} +``` + +- [ ] **Step 2: Run to verify failure** + +Run: `go test ./internal/ui/ -run 'TestStartShell|TestTailBuffer'` +Expected: FAIL — `undefined: startShell`. + +- [ ] **Step 3: Implement** — `internal/ui/custom.go`: + +```go +package ui + +import ( + "fmt" + "os" + "strings" + "time" +) + +// commandWait is how long a custom command is watched before boardwalk stops +// waiting and reports it as started. A command like "open a terminal tab" +// finishes well inside it, so its real outcome is what the status line says; +// one that keeps running is left to run. A variable so tests can shorten it. +var commandWait = 2 * time.Second + +// runShell runs a custom command, indirected so Root's tests can see what +// would have run without running it. +var runShell = startShell + +// startShell runs script detached from the terminal and reports what became +// of it within commandWait. +// +// Nothing the command prints may reach the terminal: bubbletea is drawing on +// it, and a stray line would tear the screen until the next repaint. So +// stdin is /dev/null, stdout is dropped, and stderr is kept — its last line +// is the reason a failing script gives. +func startShell(name, script string, env []string) StatusMsg { + cmd := shellCommand(script) + cmd.Env = append(os.Environ(), env...) + var stderr tailBuffer + cmd.Stderr = &stderr + + if err := cmd.Start(); err != nil { + return StatusMsg{Text: fmt.Sprintf("could not run %q: %v", name, err), Err: true} + } + + // Wait runs on its own goroutine so a command still going after + // commandWait is reaped when it does finish, rather than left a zombie. + done := make(chan error, 1) + go func() { done <- cmd.Wait() }() + + select { + case err := <-done: + if err == nil { + return StatusMsg{Text: fmt.Sprintf("ran %q", name)} + } + if line := stderr.lastLine(); line != "" { + return StatusMsg{Text: fmt.Sprintf("%q failed: %s", name, line), Err: true} + } + return StatusMsg{Text: fmt.Sprintf("%q failed: %v", name, err), Err: true} + case <-time.After(commandWait): + return StatusMsg{Text: fmt.Sprintf("started %q", name)} + } +} + +// tailLimit bounds what tailBuffer keeps of a command's stderr. +const tailLimit = 4096 + +// tailBuffer keeps the end of what is written to it. Only the last line is +// ever shown, and a chatty command must not grow boardwalk without bound. +// +// It is written by exec's copying goroutine and read only after Wait has +// returned, which orders the two, so it needs no lock. +type tailBuffer struct{ b []byte } + +func (t *tailBuffer) Write(p []byte) (int, error) { + t.b = append(t.b, p...) + if len(t.b) > tailLimit { + t.b = append(t.b[:0], t.b[len(t.b)-tailLimit:]...) + } + return len(p), nil +} + +// lastLine is the last non-blank line written. +func (t *tailBuffer) lastLine() string { + lines := strings.Split(strings.TrimSpace(string(t.b)), "\n") + return strings.TrimSpace(lines[len(lines)-1]) +} +``` + +`internal/ui/custom_unix.go`: + +```go +//go:build !windows + +package ui + +import ( + "os/exec" + "syscall" +) + +// shellCommand runs script with sh, in a process group of its own. The +// group is what keeps the command alive past boardwalk: a ctrl+c that quits +// boardwalk is delivered by the terminal to its foreground group, and the +// command is no longer in it. +func shellCommand(script string) *exec.Cmd { + cmd := exec.Command("sh", "-c", script) + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + return cmd +} +``` + +`internal/ui/custom_windows.go`: + +```go +//go:build windows + +package ui + +import ( + "os/exec" + "syscall" +) + +// shellCommand runs script with cmd, in a process group of its own, for the +// reason the unix version gives. +func shellCommand(script string) *exec.Cmd { + cmd := exec.Command("cmd", "/C", script) + cmd.SysProcAttr = &syscall.SysProcAttr{CreationFlags: syscall.CREATE_NEW_PROCESS_GROUP} + return cmd +} +``` + +- [ ] **Step 4: Run to verify pass, and cross-compile** + +Run: `go test ./internal/ui/ -run 'TestStartShell|TestTailBuffer' && GOOS=windows go vet ./internal/ui/` +Expected: PASS, and vet clean for Windows. + +- [ ] **Step 5: Commit** + +```bash +git add internal/ui/custom.go internal/ui/custom_unix.go internal/ui/custom_windows.go internal/ui/custom_test.go +git commit -m "feat: run a shell command detached and report how it went" +``` + +--- + +### Task 3: Subjects + +**Files:** +- Create: `internal/ui/subject.go` +- Modify: `internal/ui/pullrequests.go`, `pullrequestdetail.go`, `prdiff.go`, `fileview.go`, `workitems.go`, `item.go`, `builds.go`, `logs.go` (one method each, placed after the view's `Prompting` or `Status`) +- Test: `internal/ui/subject_test.go` + +**Interfaces:** +- Consumes: `config.KindPullRequest`, `config.KindWorkItem`, `config.KindBuild` (Task 1) +- Produces: + - `type subject struct { kind string; env map[string]string }` — `env` keys have no `BOARDWALK_` prefix and hold `ID`, `TITLE`, `URL` plus the kind's own + - `type subjecter interface { Subject() (subject, bool) }` — `kind` is set even when the bool is false + - `pullRequestSubject(c *azdo.Client, pr azdo.PullRequest) subject`, `workItemSubject(c *azdo.Client, wi azdo.WorkItem) subject`, `buildSubject(c *azdo.Client, b azdo.Build) subject` + +- [ ] **Step 1: Write the failing tests** — `internal/ui/subject_test.go`: + +```go +package ui + +import ( + "maps" + "testing" + + "github.com/JacobAtchley/boardwalk/internal/azdo" + "github.com/JacobAtchley/boardwalk/internal/config" +) + +func subjectPR() azdo.PullRequest { + return azdo.PullRequest{ID: 812, Title: "Retry on 5xx", Repo: "platform", Author: "Dev Example", + IsDraft: true, Source: "refs/heads/feature/retry", Target: "refs/heads/main"} +} + +func TestPullRequestSubject(t *testing.T) { + c, _ := fixture() + want := map[string]string{ + "ID": "812", "TITLE": "Retry on 5xx", "URL": c.PullRequestURL("platform", 812), + "REPO": "platform", "SOURCE_BRANCH": "feature/retry", "TARGET_BRANCH": "main", + "AUTHOR": "Dev Example", "IS_DRAFT": "true", + } + got := pullRequestSubject(c, subjectPR()) + if got.kind != config.KindPullRequest || !maps.Equal(got.env, want) { + t.Errorf("subject = %+v, want %v", got, want) + } +} + +func TestWorkItemSubject(t *testing.T) { + c, items := fixture() + wi := items[0] + want := map[string]string{ + "ID": "4021", "TITLE": wi.Title, "URL": c.WorkItemURL(4021), + "TYPE": "User Story", "STATE": "Active", + } + got := workItemSubject(c, wi) + if got.kind != config.KindWorkItem || !maps.Equal(got.env, want) { + t.Errorf("subject = %+v, want %v", got, want) + } +} + +func TestBuildSubject(t *testing.T) { + c, _ := fixture() + b := buildStub() + b.SourceBranch = "refs/heads/main" + want := map[string]string{ + "ID": "9001", "TITLE": "platform-ci #20260911.3", "URL": c.BuildURL(9001), + "NUMBER": "20260911.3", "PIPELINE": "platform-ci", "BRANCH": "main", "RESULT": "succeeded", + } + got := buildSubject(c, b) + if got.kind != config.KindBuild || !maps.Equal(got.env, want) { + t.Errorf("subject = %+v, want %v", got, want) + } +} + +func TestEveryViewWithASubjectNamesIt(t *testing.T) { + c, items := fixture() + pr := subjectPR() + + prs := NewPullRequests(c) + prs.Update(prsMsg{PRs: []azdo.PullRequest{pr}}) + wis := NewWorkItems(c, items, false, false) + builds := NewBuilds(c) + builds.Update(buildsMsg{Builds: []azdo.Build{buildStub()}}) + + for _, tc := range []struct { + name string + view View + kind string + id string + }{ + {"pr list", prs, config.KindPullRequest, "812"}, + {"pr detail", NewPullRequestDetail(c, pr, nil), config.KindPullRequest, "812"}, + {"pr diff", NewPullRequestDiff(c, pr, nil, threadsAll), config.KindPullRequest, "812"}, + {"work items", wis, config.KindWorkItem, "4021"}, + {"item", NewItem(c, items[0], nil), config.KindWorkItem, "4021"}, + {"builds", builds, config.KindBuild, "9001"}, + {"logs", NewLogs(c, buildStub(), nil), config.KindBuild, "9001"}, + } { + s, ok := tc.view.(subjecter) + if !ok { + t.Errorf("%s has no Subject", tc.name) + continue + } + got, ok := s.Subject() + if !ok || got.kind != tc.kind || got.env["ID"] != tc.id { + t.Errorf("%s: Subject() = %+v, %v; want %s %s", tc.name, got, ok, tc.kind, tc.id) + } + } +} + +func TestAnEmptyListHasNoSubjectButKeepsItsKind(t *testing.T) { + c, _ := fixture() + for _, tc := range []struct { + view subjecter + kind string + }{ + {NewPullRequests(c), config.KindPullRequest}, + {NewWorkItems(c, nil, false, false), config.KindWorkItem}, + {NewBuilds(c), config.KindBuild}, + } { + got, ok := tc.view.Subject() + if ok || got.kind != tc.kind { + t.Errorf("%T: Subject() = %+v, %v; want kind %s and nothing selected", tc.view, got, ok, tc.kind) + } + } +} +``` + +Before running, check two names against the code and adjust the test (not the code) if they differ: the zero `threadFilter` constant passed to `NewPullRequestDiff` (`grep -n "threadFilter = iota" -A3 internal/ui/threadfilter.go`), and whether `NewPullRequests`/`NewBuilds` need a `tea.WindowSizeMsg` before `Selected()` returns a row (if so, send `tea.WindowSizeMsg{Width: 140, Height: 30}` first). `FileView` is covered in Task 5 through Root, because its constructor needs file text; give it the method here all the same. + +- [ ] **Step 2: Run to verify failure** + +Run: `go test ./internal/ui/ -run 'Subject'` +Expected: FAIL — `undefined: pullRequestSubject`. + +- [ ] **Step 3: Implement** — `internal/ui/subject.go`: + +```go +package ui + +import ( + "fmt" + "strconv" + "strings" + + "github.com/JacobAtchley/boardwalk/internal/azdo" + "github.com/JacobAtchley/boardwalk/internal/config" +) + +// subject is the one thing a screen is showing that a custom command can act +// on: its kind, and the variables that describe it, named without their +// BOARDWALK_ prefix. Root adds the prefix and the variables every kind shares. +type subject struct { + kind string + env map[string]string +} + +// subjecter is a view a custom command can run on. kind is set even when ok +// is false — an empty list is still a pull request screen, and Root has to +// know which commands to say "nothing selected" for. +type subjecter interface { + Subject() (s subject, ok bool) +} + +// branchName strips refs/heads/. shortRef is not used: it answers "-" for an +// empty ref, which reads well in a column and badly in a script. +func branchName(ref string) string { return strings.TrimPrefix(ref, "refs/heads/") } + +func pullRequestSubject(c *azdo.Client, pr azdo.PullRequest) subject { + return subject{kind: config.KindPullRequest, env: map[string]string{ + "ID": strconv.Itoa(pr.ID), + "TITLE": pr.Title, + "URL": c.PullRequestURL(pr.Repo, pr.ID), + "REPO": pr.Repo, + "SOURCE_BRANCH": branchName(pr.Source), + "TARGET_BRANCH": branchName(pr.Target), + "AUTHOR": pr.Author, + "IS_DRAFT": strconv.FormatBool(pr.IsDraft), + }} +} + +func workItemSubject(c *azdo.Client, wi azdo.WorkItem) subject { + return subject{kind: config.KindWorkItem, env: map[string]string{ + "ID": strconv.Itoa(wi.ID), + "TITLE": wi.Title, + "URL": c.WorkItemURL(wi.ID), + "TYPE": wi.Type, + "STATE": wi.State, + }} +} + +func buildSubject(c *azdo.Client, b azdo.Build) subject { + return subject{kind: config.KindBuild, env: map[string]string{ + "ID": strconv.Itoa(b.ID), + "TITLE": fmt.Sprintf("%s #%s", b.Pipeline, b.Number), + "URL": c.BuildURL(b.ID), + "NUMBER": b.Number, + "PIPELINE": b.Pipeline, + "BRANCH": branchName(b.SourceBranch), + "RESULT": b.Status.String(), + }} +} +``` + +Then one method per view. The list views read the selected row: + +```go +// pullrequests.go +// Subject is the selected pull request, for a custom command. +func (m *PullRequests) Subject() (subject, bool) { + if row, ok := m.browser.Selected(); ok { + if r, ok := row.(prRow); ok { + return pullRequestSubject(m.client, r.PullRequest), true + } + } + return subject{kind: config.KindPullRequest}, false +} + +// workitems.go +// Subject is the selected work item, for a custom command. +func (m *WorkItems) Subject() (subject, bool) { + if row, ok := m.browser.Selected(); ok { + if r, ok := row.(workItemRow); ok { + return workItemSubject(m.client, r.WorkItem), true + } + } + return subject{kind: config.KindWorkItem}, false +} + +// builds.go +// Subject is the selected run, for a custom command. +func (m *Builds) Subject() (subject, bool) { + if row, ok := m.browser.Selected(); ok { + if r, ok := row.(buildRow); ok { + return buildSubject(m.client, r.Build), true + } + } + return subject{kind: config.KindBuild}, false +} +``` + +The detail views always have theirs: + +```go +// pullrequestdetail.go +// Subject is the pull request this pane shows, for a custom command. +func (m *PullRequestDetail) Subject() (subject, bool) { return pullRequestSubject(m.client, m.pr), true } + +// prdiff.go +// Subject is the pull request whose change this is, for a custom command. +func (m *PullRequestDiff) Subject() (subject, bool) { return pullRequestSubject(m.client, m.pr), true } + +// fileview.go +// Subject is the pull request this file belongs to, for a custom command. +func (m *FileView) Subject() (subject, bool) { return pullRequestSubject(m.client, m.pr), true } + +// item.go +// Subject is the work item this pane shows, for a custom command. +func (m *Item) Subject() (subject, bool) { return workItemSubject(m.client, m.item), true } + +// logs.go +// Subject is the run whose log this is, for a custom command. +func (m *Logs) Subject() (subject, bool) { return buildSubject(m.client, m.build), true } +``` + +Add the `config` import to `pullrequests.go`, `workitems.go` and `builds.go` if absent. + +- [ ] **Step 4: Run to verify pass** + +Run: `go test ./internal/ui/` +Expected: PASS, whole package. + +- [ ] **Step 5: Commit** + +```bash +git add internal/ui/ +git commit -m "feat: name what each screen shows for custom commands" +``` + +--- + +### Task 4: Every subject view shows a StatusMsg, error colour included + +A custom command reports through `StatusMsg`, delivered to the view it ran on. Three of the eight views mishandle it today: `Logs` has no `case StatusMsg` at all, and `PullRequestDetail` and `Item` keep the text but drop `Err`. Their `failed` field cannot simply take `Err`: in both it also means "the discussion did not load", and the body reads it to replace the discussion with `could not load the discussion — press r to try again`. + +The fix records which status text was an error, rather than adding a sticky flag: any later status assignment changes `m.status`, and the red goes with it without every existing assignment having to clear a flag. + +**Files:** +- Modify: `internal/ui/logs.go` (`Update`), `internal/ui/pullrequestdetail.go` (struct, `Update`, `Status`), `internal/ui/item.go` (struct, `Update`, `Status`) +- Test: `internal/ui/statusmsg_test.go` + +**Interfaces:** +- Produces: on `Logs`, `PullRequestDetail` and `Item`, `Update(StatusMsg{Text, Err})` makes `Status()` return `(… + Text, Err)` until the view next sets its status; the body is unaffected. + +- [ ] **Step 1: Write the failing tests** — `internal/ui/statusmsg_test.go`: + +```go +package ui + +import ( + "strings" + "testing" +) + +func TestSubjectViewsShowAStatusMsgWithItsColour(t *testing.T) { + c, items := fixture() + for _, tc := range []struct { + name string + view View + }{ + {"logs", NewLogs(c, buildStub(), nil)}, + {"pr detail", NewPullRequestDetail(c, subjectPR(), nil)}, + {"item", NewItem(c, items[0], nil)}, + } { + v, _ := tc.view.Update(StatusMsg{Text: "boom", Err: true}) + if text, isErr := v.Status(); !strings.HasSuffix(text, "boom") || !isErr { + t.Errorf("%s: after an error, Status() = %q, %v", tc.name, text, isErr) + } + v, _ = v.Update(StatusMsg{Text: "fine"}) + if text, isErr := v.Status(); !strings.HasSuffix(text, "fine") || isErr { + t.Errorf("%s: after a success, Status() = %q, %v", tc.name, text, isErr) + } + } +} + +func TestAStatusErrorDoesNotClaimTheDiscussionFailed(t *testing.T) { + // failed means the discussion did not load, and the body says so. A + // command failing is not that. + c, items := fixture() + for _, tc := range []struct { + name string + view View + }{ + {"pr detail", NewPullRequestDetail(c, subjectPR(), nil)}, + {"item", NewItem(c, items[0], nil)}, + } { + v, _ := tc.view.Update(StatusMsg{Text: "boom", Err: true}) + if body := v.Body(120, 40); strings.Contains(body, "could not load the discussion") { + t.Errorf("%s: a status error replaced the discussion:\n%s", tc.name, body) + } + } +} +``` + +- [ ] **Step 2: Run to verify failure** + +Run: `go test ./internal/ui/ -run 'TestSubjectViewsShowAStatusMsg|TestAStatusErrorDoesNot'` +Expected: FAIL — logs shows no "boom"; pr detail and item report `isErr` false. + +- [ ] **Step 3: Implement** + +`internal/ui/logs.go`, in `Update`'s switch, beside the other cases: + +```go + case StatusMsg: + m.status, m.failed = msg.Text, msg.Err + return m, nil +``` + +(`Logs.failed` only means "the last status was an error" — it keeps a failed poll's message from being cleared — so it can take `Err` directly.) + +`internal/ui/pullrequestdetail.go` — add to the struct, after `status string`: + +```go + // errStatus is the status text, when a StatusMsg said it was an error. + // It is not a flag: failed already means the discussion did not load, and + // a flag would stay set after the next status replaced this one. Comparing + // text means the red goes when the message does. + errStatus string +``` + +replace the `StatusMsg` case: + +```go + case StatusMsg: + m.status, m.errStatus = msg.Text, "" + if msg.Err { + m.errStatus = msg.Text + } + return m, nil +``` + +and the end of `Status`: + +```go + return m.work.View() + m.status, m.failed || (m.errStatus != "" && m.errStatus == m.status) +``` + +`internal/ui/item.go` — the same field after `status string`, with the same comment; the same `StatusMsg` case; and `Status`'s last line: + +```go + return m.work.View() + m.status, m.failed || m.stateErr || m.assignErr || m.commentErr || + (m.errStatus != "" && m.errStatus == m.status) +``` + +- [ ] **Step 4: Run to verify pass** + +Run: `go test ./internal/ui/` +Expected: PASS, whole package. + +- [ ] **Step 5: Commit** + +```bash +git add internal/ui/logs.go internal/ui/pullrequestdetail.go internal/ui/item.go internal/ui/statusmsg_test.go +git commit -m "fix: show a status message, and its error colour, on every detail pane" +``` + +--- + +### Task 5: Root — dispatch, help and palette + +**Files:** +- Create: `internal/ui/rootcommands.go` +- Modify: `internal/ui/root.go` (fields, `Update`, `key`, `View`), `internal/ui/rootpalette.go` (`paletteEntries`, `paletteBinding`) +- Test: `internal/ui/rootcommands_test.go` + +**Interfaces:** +- Consumes: `config.Command` (Task 1); `runShell` (Task 2); `subject`, `subjecter` (Task 3); every subject view honouring `StatusMsg.Err` (Task 4) +- Produces: + - `func WithCommands(cmds []config.Command) RootOption` + - `func CheckCommands(cmds []config.Command, paletteKey string) error` + +- [ ] **Step 1: Write the failing tests** — `internal/ui/rootcommands_test.go`: + +```go +package ui + +import ( + "slices" + "strings" + "testing" + + "github.com/JacobAtchley/boardwalk/internal/config" + tea "github.com/charmbracelet/bubbletea" +) + +var ctrlR = tea.KeyMsg{Type: tea.KeyCtrlR} + +func reviewCommand() config.Command { + return config.Command{On: config.KindPullRequest, Key: "ctrl+r", Name: "review", Run: "review.sh"} +} + +type ranCommand struct { + name, script string + env []string +} + +// stubRunner records what would have run, and reports it ran. +func stubRunner(t *testing.T) *[]ranCommand { + t.Helper() + var ran []ranCommand + before := runShell + runShell = func(name, script string, env []string) StatusMsg { + ran = append(ran, ranCommand{name, script, env}) + return StatusMsg{Text: `ran "` + name + `"`} + } + t.Cleanup(func() { runShell = before }) + return &ran +} + +// rootOn builds a Root with cmds and v pushed on top. +func rootOn(t *testing.T, v View, cmds ...config.Command) *Root { + t.Helper() + c, _ := fixture() + r := NewRoot(c, false, false, "", WithCommands(cmds)) + r, _ = send(t, r, tea.WindowSizeMsg{Width: 140, Height: 30}, PushMsg{View: v}) + return r +} + +// run executes a command's tea.Cmd and delivers what it produced, the way +// bubbletea would. A batch is unpacked one level. +func run(t *testing.T, r *Root, cmd tea.Cmd) *Root { + t.Helper() + if cmd == nil { + return r + } + msg := cmd() + if batch, ok := msg.(tea.BatchMsg); ok { + for _, c := range batch { + r = run(t, r, c) + } + return r + } + r, _ = send(t, r, msg) + return r +} + +func statusOf(r *Root) (string, bool) { + top, _ := r.top() + return top.Status() +} + +func TestRootRunsACommandWithTheSubject(t *testing.T) { + ran := stubRunner(t) + c, _ := fixture() + r := rootOn(t, NewPullRequestDetail(c, subjectPR(), nil), reviewCommand()) + + r, cmd := send(t, r, ctrlR) + if text, _ := statusOf(r); text != `running "review"…` { + t.Errorf("status while running = %q", text) + } + r = run(t, r, cmd) + + if len(*ran) != 1 || (*ran)[0].script != "review.sh" { + t.Fatalf("ran = %+v", *ran) + } + env := (*ran)[0].env + for _, want := range []string{ + "BOARDWALK_KIND=pullRequest", "BOARDWALK_ORG=acme", "BOARDWALK_PROJECT=Platform", + "BOARDWALK_ID=812", "BOARDWALK_SOURCE_BRANCH=feature/retry", + } { + if !slices.Contains(env, want) { + t.Errorf("env is missing %s: %v", want, env) + } + } + if text, isErr := statusOf(r); text != `ran "review"` || isErr { + t.Errorf("status = %q, %v", text, isErr) + } +} + +func TestRootIgnoresACommandForAnotherKind(t *testing.T) { + ran := stubRunner(t) + c, items := fixture() + r := rootOn(t, NewItem(c, items[0], nil), reviewCommand()) + + r, cmd := send(t, r, ctrlR) + r = run(t, r, cmd) + + if len(*ran) != 0 { + t.Errorf("a pull request command ran on a work item: %+v", *ran) + } +} + +func TestRootSaysNothingSelectedOnAnEmptyList(t *testing.T) { + ran := stubRunner(t) + c, _ := fixture() + r := rootOn(t, NewPullRequests(c), reviewCommand()) + + r, cmd := send(t, r, ctrlR) + r = run(t, r, cmd) + + if len(*ran) != 0 { + t.Errorf("ran with nothing selected: %+v", *ran) + } + if text, _ := statusOf(r); text != "nothing selected" { + t.Errorf("status = %q", text) + } +} + +func TestRootLetsTheViewsOwnKeyWin(t *testing.T) { + // A on the pull request pane approves. A custom command on A must not + // shadow it, and must not be advertised where it would not run. + ran := stubRunner(t) + c, _ := fixture() + approveAlias := config.Command{On: config.KindPullRequest, Key: "A", Name: "my approve", Run: "x"} + r := rootOn(t, NewPullRequestDetail(c, subjectPR(), nil), approveAlias) + + r, cmd := send(t, r, runes("A")) + r = run(t, r, cmd) + + if len(*ran) != 0 { + t.Errorf("the custom command shadowed a built-in key: %+v", *ran) + } + top, _ := r.top() + for _, g := range r.keysFor(top).FullHelp() { + for _, b := range g { + if b.Help().Desc == "my approve" { + t.Error("a shadowed command is listed in help") + } + } + } +} + +func TestRootSkipsACommandWhileAVoteIsArmed(t *testing.T) { + ran := stubRunner(t) + c, _ := fixture() + c.MyID = "me" + r := rootOn(t, NewPullRequestDetail(c, subjectPR(), nil), reviewCommand()) + + r, _ = send(t, r, runes("A")) // arms the approve vote + r, cmd := send(t, r, ctrlR) + r = run(t, r, cmd) + + if len(*ran) != 0 { + t.Errorf("ran while a vote was armed: %+v", *ran) + } +} + +func TestRootDeliversAResultToTheViewItRanOn(t *testing.T) { + // The user can move on in the two seconds a command is watched. Its + // result belongs to the pane it ran on: not shown as news about the pane + // now on top, and there to read on coming back. + stubRunner(t) + c, items := fixture() + r := rootOn(t, NewPullRequestDetail(c, subjectPR(), nil), reviewCommand()) + + r, cmd := send(t, r, ctrlR) + r, _ = send(t, r, PushMsg{View: NewItem(c, items[0], nil)}) + r = run(t, r, cmd) + + if text, _ := statusOf(r); strings.Contains(text, "review") { + t.Errorf("the work item pane shows the pull request command's result: %q", text) + } + r, _ = send(t, r, PopMsg{}) + if text, _ := statusOf(r); text != `ran "review"` { + t.Errorf("back on the pull request, status = %q", text) + } +} + +func TestRootDropsAResultForAViewThatIsGone(t *testing.T) { + stubRunner(t) + c, _ := fixture() + r := rootOn(t, NewPullRequestDetail(c, subjectPR(), nil), reviewCommand()) + + r, cmd := send(t, r, ctrlR) + r, _ = send(t, r, PopMsg{}) + r = run(t, r, cmd) // must not panic, and has nowhere to go + + if len(r.stack) != 0 { + t.Errorf("stack = %v", r.stack) + } +} + +func TestRootListsCommandsInHelpAndRunsThemFromThePalette(t *testing.T) { + ran := stubRunner(t) + c, _ := fixture() + r := rootOn(t, NewPullRequestDetail(c, subjectPR(), nil), reviewCommand()) + + r, _ = send(t, r, runes("?")) + if !strings.Contains(r.View(), "review") { + t.Errorf("the help panel does not list the command:\n%s", r.View()) + } + r, _ = send(t, r, runes("?")) + + r, cmd := search(t, r, "review") + r = run(t, r, cmd) + if len(*ran) != 1 { + t.Errorf("running the palette entry ran %d commands", len(*ran)) + } +} + +func TestCheckCommands(t *testing.T) { + cmd := func(k string) []config.Command { + return []config.Command{{On: config.KindBuild, Key: k, Name: "n", Run: "r"}} + } + for _, tc := range []struct { + key, palette, want string + }{ + {"ctrl+r", "", ""}, + {"K", "ctrl+k", ""}, + {"ctrl+banana", "", "not a key"}, + {"q", "", "needs for itself"}, + {"ctrl+c", "", "needs for itself"}, + {"ctrl+p", "", "opens the command palette"}, + {"ctrl+k", "ctrl+k", "opens the command palette"}, + } { + err := CheckCommands(cmd(tc.key), tc.palette) + switch { + case tc.want == "" && err != nil: + t.Errorf("%s: got %v", tc.key, err) + case tc.want != "" && (err == nil || !strings.Contains(err.Error(), tc.want)): + t.Errorf("%s: got %v, want %q", tc.key, err, tc.want) + } + } +} + +func TestCheckCommandsRefusesListMovementKeys(t *testing.T) { + // A list moves its cursor on keys no view lists in its help: l pages + // right, b pages back. Root sees a key before the view does, so a command + // on one would quietly take paging away from every list. + for _, k := range []string{"l", "h", "b", "u", "f", "pgup", "home", "G"} { + err := CheckCommands([]config.Command{{On: config.KindBuild, Key: k, Name: "n", Run: "r"}}, "") + if err == nil || !strings.Contains(err.Error(), "moves the cursor") { + t.Errorf("%s: got %v", k, err) + } + } +} +``` + +Before running, confirm two helpers exist as named: `runes` (in `workitems_test.go`) and `search` (in `rootpalette_test.go`). Confirm `azdo.Client` has a `MyID` field (`grep -n MyID internal/azdo/client.go`) — `armVote` refuses to arm without it. + +- [ ] **Step 2: Run to verify failure** + +Run: `go test ./internal/ui/ -run 'TestRoot.*Command|TestRootSays|TestRootLets|TestRootSkips|TestRootDrops|TestRootIgnores|TestCheckCommands'` +Expected: FAIL — `undefined: WithCommands`. + +- [ ] **Step 3: Implement** — `internal/ui/rootcommands.go`: + +```go +package ui + +import ( + "fmt" + "maps" + "slices" + + "github.com/JacobAtchley/boardwalk/internal/config" + "github.com/charmbracelet/bubbles/help" + "github.com/charmbracelet/bubbles/key" + "github.com/charmbracelet/bubbles/list" + tea "github.com/charmbracelet/bubbletea" +) + +// WithCommands binds the user's custom commands. Check them with +// CheckCommands first. +func WithCommands(cmds []config.Command) RootOption { + return func(r *Root) { r.commands = cmds } +} + +// listMovementKeys are the keys a list moves its cursor with. No view lists +// them in its help — they belong to bubbles/list — so the help-based +// collision check cannot see them, and Root sees a key before the view does. +func listMovementKeys() []string { + km := list.DefaultKeyMap() + var keys []string + for _, b := range []key.Binding{km.CursorUp, km.CursorDown, km.PrevPage, km.NextPage, km.GoToStart, km.GoToEnd, km.Filter} { + keys = append(keys, b.Keys()...) + } + return keys +} + +// CheckCommands reports a command whose key cannot work: one that is not a +// key, one Root needs, one a list moves with, or the palette's. +func CheckCommands(cmds []config.Command, paletteKey string) error { + if paletteKey == "" { + paletteKey = DefaultPaletteKey + } + moves := listMovementKeys() + for i, c := range cmds { + switch { + case !validKey(c.Key): + return fmt.Errorf("commands[%d] key %q is not a key boardwalk recognises — try \"ctrl+r\"", i, c.Key) + case slices.Contains(reservedKeys, c.Key): + return fmt.Errorf("commands[%d] key %q is one boardwalk needs for itself", i, c.Key) + case slices.Contains(moves, c.Key): + return fmt.Errorf("commands[%d] key %q moves the cursor in lists", i, c.Key) + case c.Key == paletteKey: + return fmt.Errorf("commands[%d] key %q opens the command palette", i, c.Key) + } + } + return nil +} + +func validKey(k string) bool { _, ok := keyMsgFor(k); return ok } + +// builtinKeys is every key the view on top answers to on its own: what its +// help lists, plus what Root and the shared actions claim on every view. +func builtinKeys(top View) map[string]bool { + keys := map[string]bool{} + add := func(bs ...key.Binding) { + for _, b := range bs { + for _, k := range b.Keys() { + keys[k] = true + } + } + } + for _, g := range top.Keys().FullHelp() { + add(g...) + } + add(sharedBindings()...) + add(navBindings()...) + add(keyRefresh) + return keys +} + +// activeCommands are the custom commands that run on the view on top: bound +// for its kind, on a key it does not use itself. +func (r *Root) activeCommands(top View) []config.Command { + s, ok := top.(subjecter) + if !ok || len(r.commands) == 0 { + return nil + } + subj, _ := s.Subject() + taken := builtinKeys(top) + var out []config.Command + for _, c := range r.commands { + if c.On == subj.kind && !taken[c.Key] { + out = append(out, c) + } + } + return out +} + +// commandKey runs the custom command bound to msg on the view on top, if +// there is one. +func (r *Root) commandKey(top View, msg tea.KeyMsg) (tea.Cmd, bool) { + i := slices.IndexFunc(r.activeCommands(top), func(c config.Command) bool { return c.Key == msg.String() }) + if i < 0 { + return nil, false + } + c := r.activeCommands(top)[i] + + subj, ok := top.(subjecter).Subject() + if !ok { + r.tell(top, StatusMsg{Text: "nothing selected"}) + return nil, true + } + + env := r.commandEnv(subj) + r.tell(top, StatusMsg{Text: fmt.Sprintf("running %q…", c.Name)}) + return func() tea.Msg { + return commandDoneMsg{view: top, status: runShell(c.Name, c.Run, env)} + }, true +} + +// tell delivers a status to v wherever it sits in the stack, and does +// nothing if it is no longer there. It is called synchronously for "running…" +// rather than returned as a command: a command finishing faster than that +// message was delivered would have its result overwritten by it. +func (r *Root) tell(v View, s StatusMsg) { + if i := slices.Index(r.stack, v); i >= 0 { + r.stack[i], _ = v.Update(s) + } +} + +// commandEnv is the BOARDWALK_ variables for a subject, in a fixed order so +// a test — or a user reading `env` — sees the same thing every time. +func (r *Root) commandEnv(s subject) []string { + env := []string{ + "BOARDWALK_KIND=" + s.kind, + "BOARDWALK_ORG=" + r.client.Org, + "BOARDWALK_PROJECT=" + r.client.Project, + } + for _, k := range slices.Sorted(maps.Keys(s.env)) { + env = append(env, "BOARDWALK_"+k+"="+s.env[k]) + } + return env +} + +// commandDoneMsg is how a custom command turned out, addressed to the view +// it ran on. A plain StatusMsg would go to whatever is on top when it lands, +// which after two seconds may be somewhere else entirely. +type commandDoneMsg struct { + view View + status StatusMsg +} + +// keysFor is the view's bindings with the custom commands active on it +// added, which is what the help line, the "?" panel and the palette read. +func (r *Root) keysFor(top View) help.KeyMap { + active := r.activeCommands(top) + if len(active) == 0 { + return top.Keys() + } + bindings := make([]key.Binding, len(active)) + for i, c := range active { + bindings[i] = key.NewBinding(key.WithKeys(c.Key), key.WithHelp(shownKey(c.Key), c.Name)) + } + return commandKeys{KeyMap: top.Keys(), commands: bindings} +} + +// commandKeys adds a group of custom commands to a view's own bindings. +type commandKeys struct { + help.KeyMap + commands []key.Binding +} + +func (k commandKeys) FullHelp() [][]key.Binding { + groups := k.KeyMap.FullHelp() + out := make([][]key.Binding, len(groups), len(groups)+1) + copy(out, groups) + return append(out, k.commands) +} +``` + +In `internal/ui/rootpalette.go`, extract the help spelling from `paletteBinding` so commands spell keys the same way: + +```go +// shownKey spells a key the way the footer does: ^p, not ctrl+p. +func shownKey(k string) string { + if rest, ok := strings.CutPrefix(k, "ctrl+"); ok && len(rest) == 1 { + return "^" + rest + } + return k +} + +func paletteBinding(k string) key.Binding { + return key.NewBinding(key.WithKeys(k), key.WithHelp(shownKey(k), "commands")) +} +``` + +In `paletteEntries`, replace `top.Keys().FullHelp()` with `r.keysFor(top).FullHelp()`. + +In `internal/ui/root.go`: + +- add to `Root`, after `history`: + +```go + // commands are the user's custom commands — see rootcommands.go. + commands []config.Command +``` + + and the `config` import. + +- in `Update`, add before `case tea.KeyMsg:`: + +```go + case commandDoneMsg: + r.tell(msg.view, msg.status) + return r, nil +``` + +- in `key`, after the palette-key block and before `switch msg.String() {` (the esc/q one): + +```go + if cmd, ok := r.commandKey(top, msg); ok { + return cmd, true + } +``` + + It sits after the `typing(top)` guard, so a prompt, a filter or an armed vote or draft toggle sees the key first. + +- in `View`, replace `withPalette(top.Keys(), r.paletteKey)` with `withPalette(r.keysFor(top), r.paletteKey)`. + +- [ ] **Step 4: Run to verify pass** + +Run: `go test ./internal/ui/` +Expected: PASS, whole package — the palette and help tests included. + +- [ ] **Step 5: Commit** + +```bash +git add internal/ui/ +git commit -m "feat: run a custom command from its key, the help panel or the palette" +``` + +--- + +### Task 6: Wire it up and document it + +**Files:** +- Modify: `main.go` (`rootOptions`), `main_test.go`, `README.md` + +**Interfaces:** +- Consumes: `Config.ValidateCommands` (Task 1); `ui.CheckCommands`, `ui.WithCommands` (Task 5) + +- [ ] **Step 1: Write the failing test** — append to `main_test.go`: + +```go +func TestRootOptionsChecksCommands(t *testing.T) { + t.Setenv("BOARDWALK_CONFIG", t.TempDir()+"/boardwalk.json") + good := config.Command{On: config.KindPullRequest, Key: "ctrl+r", Name: "review", Run: "review.sh"} + + opts, err := rootOptions(config.Config{Commands: []config.Command{good}}) + if err != nil || len(opts) != 2 { + t.Errorf("rootOptions = %d options, %v; want the commands and the history", len(opts), err) + } + + bad := good + bad.On = "pr" + if _, err := rootOptions(config.Config{Commands: []config.Command{bad}}); err == nil { + t.Error("an unknown kind was accepted") + } + + bad = good + bad.Key = "q" + if _, err := rootOptions(config.Config{Commands: []config.Command{bad}}); err == nil { + t.Error("a reserved key was accepted") + } +} +``` + +- [ ] **Step 2: Run to verify failure** + +Run: `go test . -run TestRootOptionsChecksCommands` +Expected: FAIL — 1 option rather than 2. + +- [ ] **Step 3: Implement** — in `rootOptions`, before the history block: + +```go + if len(cfg.Commands) > 0 { + path, _ := config.Path() + if err := cfg.ValidateCommands(path); err != nil { + return nil, err + } + if err := ui.CheckCommands(cfg.Commands, cfg.PaletteKey); err != nil { + return nil, fmt.Errorf("%s: %w", path, err) + } + opts = append(opts, ui.WithCommands(cfg.Commands)) + } +``` + +and update its doc comment to say it also checks and binds the custom commands. + +In `README.md`, add a row to the config table after `paletteKey`: + +```markdown +| `commands` | optional — your own [custom commands](#custom-commands), each a key on a kind of screen | +``` + +and a section after `### Command palette`: + +````markdown +### Custom commands + +Bind a key on a kind of screen to a shell command of your own. boardwalk runs +it with `sh -c`, detached — it keeps the screen and reports on the status line +whether the command ran, failed (with the last line it wrote to stderr), or is +still going after two seconds and has been left to run. + +```json +"commands": [ + { + "on": "pullRequest", + "key": "ctrl+r", + "name": "review with claude", + "run": "~/bin/pr-review.sh" + } +] +``` + +`on` is the kind of screen: `pullRequest` (the list, the pull request, its +diff and files), `workItem` (the list and the item) or `build` (the list and +the logs). The command appears in the `?` panel and the palette there. + +What the screen shows reaches the command as environment variables, never +pasted into the command, so a title full of shell syntax stays a title: + +| variable | on | | +|---|---|---| +| `BOARDWALK_KIND` | all | `pullRequest`, `workItem` or `build` | +| `BOARDWALK_ORG`, `BOARDWALK_PROJECT` | all | from the config | +| `BOARDWALK_ID`, `BOARDWALK_TITLE`, `BOARDWALK_URL` | all | the selected thing | +| `BOARDWALK_REPO`, `BOARDWALK_SOURCE_BRANCH`, `BOARDWALK_TARGET_BRANCH`, `BOARDWALK_AUTHOR`, `BOARDWALK_IS_DRAFT` | pullRequest | branches without `refs/heads/` | +| `BOARDWALK_TYPE`, `BOARDWALK_STATE` | workItem | | +| `BOARDWALK_NUMBER`, `BOARDWALK_PIPELINE`, `BOARDWALK_BRANCH`, `BOARDWALK_RESULT` | build | | + +The command also inherits boardwalk's own environment, so a terminal +multiplexer's variables pass straight through. For example, a script that +opens a [herdr](https://herdr.dev) tab beside boardwalk and starts a Claude +Code review there: + +```sh +#!/bin/sh +# ~/bin/pr-review.sh — adjust to your herdr version and review skill. +herdr tab create --label "review !$BOARDWALK_ID" --focus +herdr pane send-text "claude '/review-pr $BOARDWALK_ID'" +herdr pane send-keys enter +``` + +A key is one key, spelled as for `paletteKey`. boardwalk refuses at startup a +key it needs itself, one lists move with (`j`, `k`, `l`, `h`, `b`, `u`, `f`, +`g`, `G`, `/`, the arrows, page keys), or the palette key. A key a screen +already uses keeps doing what it did there, and the command is left out of +that screen's help. +```` + +Before committing, run `herdr tab create --help` and `herdr pane send-text --help` and correct the sample script's flags to what this herdr accepts — the README must not ship a script that does not run. If `send-text` needs a pane id, read it from `herdr tab create`'s output (check whether it prints JSON). + +- [ ] **Step 4: Run the whole suite and a build for both platforms** + +Run: `go test ./... && go vet ./... && GOOS=windows go build ./...` +Expected: PASS, clean, builds. + +- [ ] **Step 5: Commit** + +```bash +git add main.go main_test.go README.md +git commit -m "feat: bind custom commands from the config" +``` + +- [ ] **Step 6: Try it for real** + +Write `/tmp/bw-env.sh` containing `env | grep BOARDWALK_ > /tmp/bw-env.out`, bind it to `ctrl+r` on `pullRequest` in a scratch config (`BOARDWALK_CONFIG=...`), run `go run . prs`, press `ctrl+r` on a row, and confirm `/tmp/bw-env.out` holds the variables and the status line read `ran "…"`. Remove the scratch config afterwards. diff --git a/docs/superpowers/specs/2026-10-06-custom-commands-design.md b/docs/superpowers/specs/2026-10-06-custom-commands-design.md new file mode 100644 index 0000000..5933951 --- /dev/null +++ b/docs/superpowers/specs/2026-10-06-custom-commands-design.md @@ -0,0 +1,245 @@ +# boardwalk: custom commands + +Bind a key on a screen to a shell command of your own, run with what that +screen is showing — so "review this pull request with Claude in a new herdr +tab" is one keypress away, without boardwalk knowing anything about herdr or +Claude. + +## Goals + +- The config file maps a single key, on a kind of screen, to a shell command. +- The command learns what the screen is showing through `BOARDWALK_*` + environment variables. +- Commands run detached: boardwalk stays on screen and reports the outcome on + the status line. +- Custom commands appear in the `?` panel and the command palette like any + built-in binding. +- A custom key can never shadow a built-in one. + +## Non-goals (v1) + +- Template substitution in the command string (`{{.ID}}`). Possible later; it + would need every value shell-quoted. +- Running in the foreground (suspending boardwalk for `vim`, `lazygit`). +- Multi-key sequences (`ctrl+x c`). +- A global scope with no subject. +- Action types other than a shell command. +- Reloading the config without a restart. + +## Motivating example + +On a pull request, `ctrl+r` opens a new herdr tab in the current workspace and +starts a Claude Code session running a PR review skill for that pull request. +The herdr and Claude details live in the user's script; boardwalk supplies the +key, the pull request, and the environment herdr's own variables pass through +in. + +## Config + +`~/.config/boardwalk.json` gains a `commands` list: + +```json +{ + "org": "my-org", + "project": "MyProject", + "commands": [ + { + "on": "pullRequest", + "key": "ctrl+r", + "name": "review with claude", + "run": "~/bin/pr-review.sh" + } + ] +} +``` + +| field | meaning | +|--------|---------| +| `on` | the kind of screen: `pullRequest`, `workItem` or `build` | +| `key` | one key, spelled as bubbletea names it — the same spelling as `paletteKey` | +| `name` | the help text in `?`, the palette and the status line | +| `run` | a shell command, run with `sh -c` | + +There is no `type` field. A later action kind gets a field of its own beside +`run` (for example `open`), and a command sets exactly one of them. + +### Validation at startup + +Loading fails, naming the config file, when a command: + +- has an `on` that is not one of the three kinds; +- has a `key` that `keyMsgFor` cannot parse; +- has a `key` in `reservedKeys` (`esc`, `q`, `?`, `ctrl+c`, `enter`, `up`, + `down`, `j`, `k`) or equal to the palette key; +- has a `key` a list moves its cursor with (bubbles/list's default key map: + `l`, `h`, `b`, `u`, `f`, `g`, `G`, `/`, page and arrow keys) — no view lists + these in its help, so the collision check below could not see them; +- shares its `on` and `key` with another command; +- has an empty `name` or `run`. + +### Collisions with a screen's own keys + +Which keys a view binds is only known once the view exists, so a custom key +that a screen already uses cannot be caught at startup. The built-in binding +always wins: on that screen the custom command is left out of `?` and the +palette, and pressing the key does what it always did. A user therefore cannot +shadow approve, reject or any other built-in act by accident. + +The collision check matches the custom key against every key of every binding +in the top view's `Keys().FullHelp()`, plus the keys `SharedAction` claims on +list views (`y`, `ctrl+y`, `s`, `ctrl+s`, `o`, `ctrl+o`) — the same set of +bindings the palette is built from. + +## Environment + +The command inherits boardwalk's environment, with these added: + +| variable | set for | value | +|----------|---------|-------| +| `BOARDWALK_KIND` | all | `pullRequest`, `workItem` or `build` | +| `BOARDWALK_ORG` | all | the config's `org` | +| `BOARDWALK_PROJECT` | all | the config's `project` | +| `BOARDWALK_ID` | all | the pull request, work item or build id | +| `BOARDWALK_TITLE` | all | its title (a build's is its pipeline and number) | +| `BOARDWALK_URL` | all | its web URL | +| `BOARDWALK_REPO` | pullRequest | repository name | +| `BOARDWALK_SOURCE_BRANCH` | pullRequest | source branch, `refs/heads/` stripped | +| `BOARDWALK_TARGET_BRANCH` | pullRequest | target branch, `refs/heads/` stripped | +| `BOARDWALK_AUTHOR` | pullRequest | author display name | +| `BOARDWALK_IS_DRAFT` | pullRequest | `true` or `false` | +| `BOARDWALK_TYPE` | workItem | work item type, e.g. `Bug` | +| `BOARDWALK_STATE` | workItem | its state | +| `BOARDWALK_NUMBER` | build | the build number | +| `BOARDWALK_PIPELINE` | build | the pipeline name | +| `BOARDWALK_BRANCH` | build | the branch built, `refs/heads/` stripped | +| `BOARDWALK_RESULT` | build | its result, or its status while running | + +Values are passed as environment variables rather than pasted into the command +string because titles, branch names and descriptions are written by other +people. A title of `fix $(rm -rf ~)` is inert in `"$BOARDWALK_TITLE"`; pasted +into shell source it would run. + +## Architecture + +### Subjects + +Views gain one optional interface: + +```go +// subjecter is a view showing one thing a custom command can act on. +type subjecter interface { + Subject() (s subject, ok bool) +} + +type subject struct { + kind string + env map[string]string +} +``` + +`subject.env` holds the kind-specific variables without their `BOARDWALK_` prefix; +Root adds the prefix and the variables common to every kind. `ok` is false +when there is nothing to act on — an empty list, or a detail view whose +subject has not loaded. + +| kind | views | +|------|-------| +| `pullRequest` | `PullRequests` (selected row), `PullRequestDetail`, `PullRequestDiff`, `FileView` | +| `workItem` | `WorkItems` (selected row), `Item` | +| `build` | `Builds` (selected row), `Logs` | + +The menu, `CreateBranch` and `Status` have no subject, and a custom key does +nothing there. + +### Dispatch + +Root already sees every key before the view on top does. A custom command +runs when all of these hold: + +1. the palette is closed and the top view is not `Prompting()` — so the key + typed into a reply box or a filter is text, not a command; +2. the top view is a `subjecter`; +3. a command's `on` matches the view's kind and its `key` matches the press; +4. the key is not one the view binds itself. + +If the view's `Subject` returns `ok=false`, the status line reads +`nothing selected` and nothing runs. + +### Help and palette + +Root wraps the top view's `Keys()` the way `paletteKeys` adds the palette +key, appending a group of the custom commands active on that screen. The `?` +panel shows them; the palette, built from that panel, lists them, and running +one from the palette replays its key through `Root.Update` like any other +entry. Palette history ids follow the existing `act:` scheme. + +### Running + +A new `internal/ui/custom.go`. The runner sits behind a package variable, as +`copyToClipboard` and `openBrowser` do, so tests can stub it. + +- `exec.Command("sh", "-c", run)`, environment `os.Environ()` plus the + `BOARDWALK_*` variables, working directory boardwalk's own. +- stdin is `/dev/null`, stdout is discarded, stderr goes to a capped buffer. + Nothing the command prints may reach the terminal bubbletea is drawing. +- `SysProcAttr{Setpgid: true}`, so a `ctrl+c` that quits boardwalk is not also + delivered to the command, and the command outlives boardwalk. +- `running ""…` is delivered to the view at once. The run is a + `tea.Cmd` that waits up to two seconds, then produces a `StatusMsg`, which + Root delivers to the view the command ran on wherever it now sits in the + stack (dropped if it has been closed), so a result never appears as news + about a different screen: + - exited 0: `ran ""`; + - exited non-zero: an error, `"" failed: `, or the + exit code when stderr is empty; + - still running: `started ""`; a goroutine waits on it so it does not + become a zombie; + - could not start: an error naming the reason. + +## Wiring + +- `config.Config` gains `Commands []Command`. `ValidateCommands(path)` — + separate from `Validate`, whose errors print the org/project example — + covers the rules that need nothing from the UI: a known `on`, a non-empty `name` and `run`, + no duplicate `on` + `key`. The key rules — parseable, not reserved, not a list + movement key, not the palette key — need `keyMsgFor` and `reservedKeys`, so they live in + `ui.CheckCommands(cmds, paletteKey)`, which `main` calls beside + `CheckPaletteKey`. `config` keeps not importing `ui`. +- `ui.WithCommands([]config.Command)` hands them to Root, alongside + `WithPaletteKey` and `WithHistory`. Root also needs the org and project for + the common variables. +- `config.Example` stays as it is; the README documents `commands`. + +## Documentation + +A README section: the config shape, the environment table, the collision and +detachment rules, and the herdr example as a sample script. + +## Testing + +Test-first, following the existing table-driven style. + +- **config**: parsing `commands`; each validation error, with the file named. +- **subjects**: for each view, the kind and variables for a known subject; + `refs/heads/` stripped; an empty list gives `ok=false`. +- **Root**: + - the key runs the stubbed runner with the right command and environment; + - a built-in key on that screen wins and the command is absent from help; + - nothing runs while the view is prompting; + - a key bound for another kind does nothing; + - `ok=false` reports `nothing selected`; + - the command appears in the `?` panel and the palette, and choosing it + from the palette runs it. +- **runner**, against a real `sh` with a short wait window: + - the command sees the `BOARDWALK_*` variables (it writes `env` to a temp + file); + - exit 0, non-zero with stderr, non-zero without, and still-running each + give the status described above. + +## Status line fixes + +`Logs` ignores `StatusMsg`, and `PullRequestDetail` and `Item` drop its +`Err`. All three are fixed to show the text in the error colour when `Err` is +set. In the two detail views `failed` cannot take it — it also means the +discussion did not load, and the body reads it — so they remember which +status text was an error and colour it only while that text is still shown. diff --git a/internal/config/config.go b/internal/config/config.go index 5738cb3..2332ed7 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -11,6 +11,7 @@ import ( "io/fs" "os" "path/filepath" + "slices" "strings" ) @@ -46,6 +47,36 @@ type Config struct { // PaletteKey opens the command palette, spelled the way bubbletea names // keys: "ctrl+p", "ctrl+k", ":". Empty means the default, ctrl+p. PaletteKey string `json:"paletteKey,omitempty"` + + // Commands bind a key on one kind of screen to a shell command of the + // user's own. See Command. + Commands []Command `json:"commands,omitempty"` +} + +// The kinds of screen a command can be bound on, named for what the screen +// shows rather than which view shows it: a pull request is a pull request +// whether it is a row in the list or the pane it opens. +const ( + KindPullRequest = "pullRequest" + KindWorkItem = "workItem" + KindBuild = "build" +) + +// CommandKinds is every kind a command's On may name. +var CommandKinds = []string{KindPullRequest, KindWorkItem, KindBuild} + +// Command is one user-defined action: on a screen of kind On, the key Key +// runs Run with sh -c. Name is what the help panel, the palette and the +// status line call it. +// +// Run is the only action there is. Another kind of action would be a field +// beside it rather than a type tag, so a config written today keeps meaning +// what it says. +type Command struct { + On string `json:"on"` + Key string `json:"key"` + Name string `json:"name"` + Run string `json:"run"` } // ErrMissing is returned when there is no config file at all, so a caller can @@ -120,6 +151,33 @@ func (c Config) Validate(path string) error { return nil } +// ValidateCommands reports the first command that cannot work, naming the +// file and the command's place in the list. Whether Key names a real key, and +// one boardwalk does not need itself, is ui.CheckCommands' job: only the UI +// knows its keys. +func (c Config) ValidateCommands(path string) error { + seen := make(map[[2]string]int, len(c.Commands)) + for i, cmd := range c.Commands { + where := fmt.Sprintf("%s: commands[%d]", path, i) + switch { + case !slices.Contains(CommandKinds, cmd.On): + return fmt.Errorf("%s has on %q — use one of %s", where, cmd.On, strings.Join(CommandKinds, ", ")) + case cmd.Key == "": + return fmt.Errorf("%s sets no key", where) + case strings.TrimSpace(cmd.Name) == "": + return fmt.Errorf("%s sets no name", where) + case strings.TrimSpace(cmd.Run) == "": + return fmt.Errorf("%s sets no run", where) + } + k := [2]string{cmd.On, cmd.Key} + if j, dup := seen[k]; dup { + return fmt.Errorf("%s binds %q on %s, as commands[%d] already does", where, cmd.Key, cmd.On, j) + } + seen[k] = i + } + return nil +} + // Example is what a working config looks like, for an error message to show // rather than describe. func Example() string { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index dcc5b14..e55206c 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -175,3 +175,51 @@ func TestExampleIsItselfValid(t *testing.T) { t.Errorf("the example config does not validate: %v", err) } } + +func TestLoadReadsCommands(t *testing.T) { + t.Setenv(EnvPath, writeConfig(t, `{ + "org": "acme", "project": "Platform", + "commands": [{"on": "pullRequest", "key": "ctrl+r", "name": "review", "run": "echo hi"}] + }`)) + + c, err := Load() + if err != nil { + t.Fatalf("Load returned %v", err) + } + want := Command{On: "pullRequest", Key: "ctrl+r", Name: "review", Run: "echo hi"} + if len(c.Commands) != 1 || c.Commands[0] != want { + t.Errorf("commands = %+v, want [%+v]", c.Commands, want) + } +} + +func TestValidateCommands(t *testing.T) { + ok := Command{On: KindPullRequest, Key: "ctrl+r", Name: "review", Run: "echo hi"} + with := func(edit func(*Command)) Command { c := ok; edit(&c); return c } + + for _, tc := range []struct { + name string + cmds []Command + want string // substring of the error; empty means valid + }{ + {"none", nil, ""}, + {"one good", []Command{ok}, ""}, + {"same key on two kinds", []Command{ok, with(func(c *Command) { c.On = KindBuild })}, ""}, + {"unknown kind", []Command{with(func(c *Command) { c.On = "pr" })}, `on "pr"`}, + {"no key", []Command{with(func(c *Command) { c.Key = "" })}, "sets no key"}, + {"no name", []Command{with(func(c *Command) { c.Name = " " })}, "sets no name"}, + {"no run", []Command{with(func(c *Command) { c.Run = "" })}, "sets no run"}, + {"duplicate", []Command{ok, ok}, "as commands[0] already does"}, + } { + t.Run(tc.name, func(t *testing.T) { + err := Config{Commands: tc.cmds}.ValidateCommands("/cfg.json") + switch { + case tc.want == "" && err != nil: + t.Errorf("got %v, want valid", err) + case tc.want != "" && (err == nil || !strings.Contains(err.Error(), tc.want)): + t.Errorf("got %v, want an error containing %q", err, tc.want) + case tc.want != "" && !strings.Contains(err.Error(), "/cfg.json"): + t.Errorf("%v does not name the file", err) + } + }) + } +} diff --git a/internal/ui/builds.go b/internal/ui/builds.go index ebc241b..6229b28 100644 --- a/internal/ui/builds.go +++ b/internal/ui/builds.go @@ -6,6 +6,7 @@ import ( "time" "github.com/JacobAtchley/boardwalk/internal/azdo" + "github.com/JacobAtchley/boardwalk/internal/config" "github.com/charmbracelet/bubbles/help" "github.com/charmbracelet/bubbles/key" "github.com/charmbracelet/bubbles/spinner" @@ -693,3 +694,13 @@ func (m *Builds) Status() (string, bool) { func (m *Builds) Prompting() bool { return m.branchPrompt != nil || m.armedBuild != nil || m.browser.Filtering() } + +// Subject is the selected run, for a custom command. +func (m *Builds) Subject() (subject, bool) { + if row, ok := m.browser.Selected(); ok { + if r, ok := row.(buildRow); ok { + return buildSubject(m.client, r.Build), true + } + } + return subject{kind: config.KindBuild}, false +} diff --git a/internal/ui/custom.go b/internal/ui/custom.go new file mode 100644 index 0000000..09574ed --- /dev/null +++ b/internal/ui/custom.go @@ -0,0 +1,91 @@ +package ui + +import ( + "fmt" + "io" + "os" + "strings" + "time" +) + +// commandWait is how long a custom command is watched before boardwalk stops +// waiting and reports it as started. A command like "open a terminal tab" +// finishes well inside it, so its real outcome is what the status line says; +// one that keeps running is left to run. A variable so tests can shorten it. +var commandWait = 2 * time.Second + +// runShell runs a custom command, indirected so Root's tests can see what +// would have run without running it. +var runShell = startShell + +// startShell runs script detached from the terminal and reports what became +// of it within commandWait. +// +// Nothing the command prints may reach the terminal: bubbletea is drawing on +// it, and a stray line would tear the screen until the next repaint. So +// stdin is /dev/null, stdout is dropped, and stderr is kept — its last line +// is the reason a failing script gives. +// +// Stderr goes to a temp file, not a pipe. A pipe's read end is held by +// boardwalk: when boardwalk quits it closes, and a command still running dies +// of SIGPIPE on its next write, so it would not outlive boardwalk. A script +// that backgrounds a child also leaves that child holding the pipe open, and +// Wait blocks until the child exits. A file has neither problem. It is +// unlinked at once where the platform allows, so it can't be left behind. +func startShell(name, script string, env []string) StatusMsg { + cmd := shellCommand(script) + cmd.Env = append(os.Environ(), env...) + + f, err := os.CreateTemp("", "boardwalk-cmd-*") + if err != nil { + return StatusMsg{Text: fmt.Sprintf("could not run %q: %v", name, err), Err: true} + } + // Windows refuses to remove an open file, so that is retried on close. + _ = os.Remove(f.Name()) + defer func() { + f.Close() + _ = os.Remove(f.Name()) + }() + cmd.Stderr = f + + if err := cmd.Start(); err != nil { + return StatusMsg{Text: fmt.Sprintf("could not run %q: %v", name, err), Err: true} + } + + // Wait runs on its own goroutine so a command still going after + // commandWait is reaped when it does finish, rather than left a zombie. + // The child has its own descriptor, so closing ours on the timeout path + // does not disturb it. + done := make(chan error, 1) + go func() { done <- cmd.Wait() }() + + select { + case err := <-done: + if err == nil { + return StatusMsg{Text: fmt.Sprintf("ran %q", name)} + } + if line := stderrTail(f); line != "" { + return StatusMsg{Text: fmt.Sprintf("%q failed: %s", name, line), Err: true} + } + return StatusMsg{Text: fmt.Sprintf("%q failed: %v", name, err), Err: true} + case <-time.After(commandWait): + return StatusMsg{Text: fmt.Sprintf("started %q", name)} + } +} + +// tailLimit bounds how much of a command's stderr is read back. +const tailLimit = 4096 + +// stderrTail is the last non-blank line among the last tailLimit bytes of f. +// Only that line is ever shown, and a chatty command must not cost boardwalk +// more than a bounded read. +func stderrTail(f *os.File) string { + end, err := f.Seek(0, io.SeekEnd) + if err != nil { + return "" + } + buf := make([]byte, min(end, tailLimit)) + n, _ := f.ReadAt(buf, end-int64(len(buf))) + lines := strings.Split(strings.TrimSpace(string(buf[:n])), "\n") + return strings.TrimSpace(lines[len(lines)-1]) +} diff --git a/internal/ui/custom_test.go b/internal/ui/custom_test.go new file mode 100644 index 0000000..044dfdb --- /dev/null +++ b/internal/ui/custom_test.go @@ -0,0 +1,138 @@ +package ui + +import ( + "os" + "path/filepath" + "runtime" + "strings" + "testing" + "time" +) + +// needSh skips a test that runs a real shell script on a platform without sh. +func needSh(t *testing.T) { + t.Helper() + if runtime.GOOS == "windows" { + t.Skip("the runner tests drive sh") + } +} + +func shortWait(t *testing.T, d time.Duration) { + t.Helper() + before := commandWait + commandWait = d + t.Cleanup(func() { commandWait = before }) +} + +func TestStartShellPassesTheEnvironment(t *testing.T) { + needSh(t) + out := filepath.Join(t.TempDir(), "env") + + got := startShell("dump", `env > "$OUT"`, []string{"OUT=" + out, "BOARDWALK_ID=4021"}) + + if got.Err || got.Text != `ran "dump"` { + t.Fatalf("status = %+v", got) + } + body, err := os.ReadFile(out) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(body), "BOARDWALK_ID=4021\n") { + t.Errorf("the command did not see BOARDWALK_ID:\n%s", body) + } + if !strings.Contains(string(body), "PATH=") { + t.Error("the command did not inherit boardwalk's environment") + } +} + +func TestStartShellReportsFailure(t *testing.T) { + needSh(t) + for _, tc := range []struct{ script, want string }{ + {`echo "no such tab" >&2; exit 3`, `"go" failed: no such tab`}, + {`exit 3`, `"go" failed: exit status 3`}, + } { + got := startShell("go", tc.script, nil) + if !got.Err || got.Text != tc.want { + t.Errorf("%s: status = %+v, want error %q", tc.script, got, tc.want) + } + } +} + +func TestStartShellReportsOnlyTheLastStderrLine(t *testing.T) { + needSh(t) + // Ten thousand lines of noise and then the reason: the status line has + // room for one, and the last is the one a failing script ends on. + got := startShell("noisy", `i=0; while [ $i -lt 10000 ]; do echo "noise $i" >&2; i=$((i+1)); done; echo "the reason" >&2; exit 1`, nil) + if got.Text != `"noisy" failed: the reason` { + t.Errorf("status = %q", got.Text) + } +} + +func TestStartShellStopsWaitingOnALongCommand(t *testing.T) { + needSh(t) + shortWait(t, 100*time.Millisecond) + + start := time.Now() + got := startShell("slow", "sleep 5", nil) + + if got.Err || got.Text != `started "slow"` { + t.Errorf("status = %+v", got) + } + if time.Since(start) > 2*time.Second { + t.Error("the runner waited on the command instead of detaching") + } +} + +func TestStartShellDoesNotWaitOnABackgroundChild(t *testing.T) { + needSh(t) + shortWait(t, 5*time.Second) + + // The script exits at once but leaves a child holding stderr. With a pipe + // for stderr, Wait would block until the child let go. + start := time.Now() + got := startShell("bg", "sleep 2 >&2 &", nil) + + if got.Err || got.Text != `ran "bg"` { + t.Errorf("status = %+v", got) + } + if time.Since(start) > time.Second { + t.Errorf("waited %v on a backgrounded child", time.Since(start)) + } +} + +func TestStartShellNeverEvaluatesValues(t *testing.T) { + needSh(t) + dir := t.TempDir() + // A title is written by somebody else. Quoted as a variable it is data. + got := startShell("t", `printf '%s' "$BOARDWALK_TITLE" > /dev/null`, + []string{"BOARDWALK_TITLE=$(touch " + filepath.Join(dir, "pwned") + ")"}) + if got.Err { + t.Fatalf("status = %+v", got) + } + if _, err := os.Stat(filepath.Join(dir, "pwned")); err == nil { + t.Error("a value was run as shell") + } +} + +func TestStderrTailKeepsTheEnd(t *testing.T) { + tests := []struct { + name, content, want string + }{ + {"empty", "", ""}, + {"blank only", "\n \n", ""}, + {"last non-blank line", "first\nlast\n\n", "last"}, + {"a long run is cut to the cap", strings.Repeat(strings.Repeat("x", 100)+"\n", 100) + "last\n\n", "last"}, + {"a single line longer than the cap", strings.Repeat("y", 3*tailLimit), strings.Repeat("y", tailLimit)}, + } + for _, tc := range tests { + f, err := os.CreateTemp(t.TempDir(), "tail") + if err != nil { + t.Fatal(err) + } + f.WriteString(tc.content) + if got := stderrTail(f); got != tc.want { + t.Errorf("%s: stderrTail = %q, want %q", tc.name, got, tc.want) + } + f.Close() + } +} diff --git a/internal/ui/custom_unix.go b/internal/ui/custom_unix.go new file mode 100644 index 0000000..d3a537c --- /dev/null +++ b/internal/ui/custom_unix.go @@ -0,0 +1,18 @@ +//go:build !windows + +package ui + +import ( + "os/exec" + "syscall" +) + +// shellCommand runs script with sh, in a process group of its own. The +// group is what keeps the command alive past boardwalk: a ctrl+c that quits +// boardwalk is delivered by the terminal to its foreground group, and the +// command is no longer in it. +func shellCommand(script string) *exec.Cmd { + cmd := exec.Command("sh", "-c", script) + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + return cmd +} diff --git a/internal/ui/custom_windows.go b/internal/ui/custom_windows.go new file mode 100644 index 0000000..ca92c6f --- /dev/null +++ b/internal/ui/custom_windows.go @@ -0,0 +1,23 @@ +//go:build windows + +package ui + +import ( + "os/exec" + "syscall" +) + +// shellCommand runs script with cmd, in a process group of its own, for the +// reason the unix version gives. The CmdLine field builds the full command +// line ourselves because exec.Command escapes quotes that cmd doesn't +// understand. With /S and a quoted whole command, cmd always strips exactly +// the outer quotes and runs the rest as typed; without /S it keeps them when +// the script is a single quoted path containing spaces. +func shellCommand(script string) *exec.Cmd { + cmd := exec.Command("cmd") + cmd.SysProcAttr = &syscall.SysProcAttr{ + CmdLine: `cmd /S /C "` + script + `"`, + CreationFlags: syscall.CREATE_NEW_PROCESS_GROUP, + } + return cmd +} diff --git a/internal/ui/fileview.go b/internal/ui/fileview.go index 2f1e88b..1f984d2 100644 --- a/internal/ui/fileview.go +++ b/internal/ui/fileview.go @@ -405,3 +405,6 @@ func (m *FileView) Status() (string, bool) { // Prompting reports whether the comment prompt is open, so Root leaves esc // to it rather than treating it as navigation. func (m *FileView) Prompting() bool { return m.commentPrompt != nil } + +// Subject is the pull request this file belongs to, for a custom command. +func (m *FileView) Subject() (subject, bool) { return pullRequestSubject(m.client, m.pr), true } diff --git a/internal/ui/item.go b/internal/ui/item.go index 737432f..9b45ea7 100644 --- a/internal/ui/item.go +++ b/internal/ui/item.go @@ -77,8 +77,13 @@ type Item struct { renderedAt int status string - work work - now func() time.Time + // errStatus is the status text, when a StatusMsg said it was an error. + // It is not a flag: failed already means the discussion did not load, and + // a flag would stay set after the next status replaced this one. Comparing + // text means the red goes when the message does. + errStatus string + work work + now func() time.Time // statePicker is non-nil while the state picker is open. See the type's // own doc in statepicker.go; it holds the same modal discipline the @@ -311,7 +316,10 @@ func (m *Item) Update(msg tea.Msg) (View, tea.Cmd) { return m, nil case StatusMsg: - m.status = msg.Text + m.status, m.errStatus = msg.Text, "" + if msg.Err { + m.errStatus = msg.Text + } return m, nil case tea.KeyMsg: @@ -547,7 +555,8 @@ func (m *Item) Status() (string, bool) { if m.statePicker != nil { return m.statePicker.View(), false } - return m.work.View() + m.status, m.failed || m.stateErr || m.assignErr || m.commentErr + return m.work.View() + m.status, m.failed || m.stateErr || m.assignErr || m.commentErr || + (m.errStatus != "" && m.errStatus == m.status) } // Prompting reports whether the state picker or the comment prompt is open, @@ -570,3 +579,6 @@ func (r itemRow) Render(width int) string { return "" } func (r itemRow) CopyID() string { return fmt.Sprint(r.ID) } func (r itemRow) Label() string { return fmt.Sprintf("#%d %s", r.ID, r.Title) } func (r itemRow) URL() string { return r.url } + +// Subject is the work item this pane shows, for a custom command. +func (m *Item) Subject() (subject, bool) { return workItemSubject(m.client, m.item), true } diff --git a/internal/ui/logs.go b/internal/ui/logs.go index 75020a9..b3b7e1b 100644 --- a/internal/ui/logs.go +++ b/internal/ui/logs.go @@ -212,6 +212,10 @@ func (m *Logs) Update(msg tea.Msg) (View, tea.Cmd) { case tailTickMsg: return m, m.startFetch() + case StatusMsg: + m.status, m.failed = msg.Text, msg.Err + return m, nil + case tea.KeyMsg: // Matched against the bindings in keys.go rather than against raw // strings: this view's footer is built from those bindings, and a @@ -621,3 +625,6 @@ func (m *Logs) Keys() help.KeyMap { // Status is the transient status line. func (m *Logs) Status() (string, bool) { return m.work.View() + m.status, m.failed } + +// Subject is the run whose log this is, for a custom command. +func (m *Logs) Subject() (subject, bool) { return buildSubject(m.client, m.build), true } diff --git a/internal/ui/prdiff.go b/internal/ui/prdiff.go index 830f347..cfd2478 100644 --- a/internal/ui/prdiff.go +++ b/internal/ui/prdiff.go @@ -814,3 +814,6 @@ func (m *PullRequestDiff) Status() (string, bool) { // Prompting reports whether a text prompt is open, so Root leaves esc and q to // the prompt rather than treating them as navigation. func (m *PullRequestDiff) Prompting() bool { return m.browser.Filtering() } + +// Subject is the pull request whose change this is, for a custom command. +func (m *PullRequestDiff) Subject() (subject, bool) { return pullRequestSubject(m.client, m.pr), true } diff --git a/internal/ui/pullrequestdetail.go b/internal/ui/pullrequestdetail.go index e626344..b7d178b 100644 --- a/internal/ui/pullrequestdetail.go +++ b/internal/ui/pullrequestdetail.go @@ -128,8 +128,13 @@ type PullRequestDetail struct { renderedAt int status string - work work - now func() time.Time + // errStatus is the status text, when a StatusMsg said it was an error. + // It is not a flag: failed already means the discussion did not load, and + // a flag would stay set after the next status replaced this one. Comparing + // text means the red goes when the message does. + errStatus string + work work + now func() time.Time // replyPrompt is non-nil while a reply to a thread is being typed. replyPrompt *textinput.Model @@ -538,7 +543,10 @@ func (m *PullRequestDetail) Update(msg tea.Msg) (View, tea.Cmd) { return m, func() tea.Msg { return PushMsg{View: builds} } case StatusMsg: - m.status = msg.Text + m.status, m.errStatus = msg.Text, "" + if msg.Err { + m.errStatus = msg.Text + } return m, nil case tea.KeyMsg: @@ -886,7 +894,7 @@ func (m *PullRequestDetail) Status() (string, bool) { if m.replyPrompt != nil { return m.replyPrompt.View(), false } - return m.work.View() + m.status, m.failed + return m.work.View() + m.status, m.failed || (m.errStatus != "" && m.errStatus == m.status) } // Prompting reports whether the reply prompt or one of the two arms is open, @@ -908,3 +916,8 @@ func (r prDetailRow) Render(width int) string { return "" } func (r prDetailRow) CopyID() string { return fmt.Sprint(r.ID) } func (r prDetailRow) Label() string { return fmt.Sprintf("!%d %s", r.ID, r.Title) } func (r prDetailRow) URL() string { return r.url } + +// Subject is the pull request this pane shows, for a custom command. +func (m *PullRequestDetail) Subject() (subject, bool) { + return pullRequestSubject(m.client, m.pr), true +} diff --git a/internal/ui/pullrequests.go b/internal/ui/pullrequests.go index 8ffb388..e2fb956 100644 --- a/internal/ui/pullrequests.go +++ b/internal/ui/pullrequests.go @@ -6,6 +6,7 @@ import ( "time" "github.com/JacobAtchley/boardwalk/internal/azdo" + "github.com/JacobAtchley/boardwalk/internal/config" "github.com/charmbracelet/bubbles/help" "github.com/charmbracelet/bubbles/key" "github.com/charmbracelet/bubbles/spinner" @@ -508,3 +509,13 @@ func (m *PullRequests) Status() (string, bool) { // Root leaves esc and q to this view rather than treating them as navigation — // esc has to cancel the arm, not pop the pane out from under it. func (m *PullRequests) Prompting() bool { return m.browser.Filtering() || m.armedDraft != nil } + +// Subject is the selected pull request, for a custom command. +func (m *PullRequests) Subject() (subject, bool) { + if row, ok := m.browser.Selected(); ok { + if r, ok := row.(prRow); ok { + return pullRequestSubject(m.client, r.PullRequest), true + } + } + return subject{kind: config.KindPullRequest}, false +} diff --git a/internal/ui/root.go b/internal/ui/root.go index 98b1943..41047d2 100644 --- a/internal/ui/root.go +++ b/internal/ui/root.go @@ -5,6 +5,7 @@ import ( "strings" "github.com/JacobAtchley/boardwalk/internal/azdo" + "github.com/JacobAtchley/boardwalk/internal/config" "github.com/JacobAtchley/boardwalk/internal/history" "github.com/charmbracelet/bubbles/help" "github.com/charmbracelet/bubbles/key" @@ -54,6 +55,9 @@ type Root struct { paletteKey key.Binding history History + // commands are the user's custom commands — see rootcommands.go. + commands []config.Command + // help renders a view's bindings: one line by default, every group when // the user presses "?". help help.Model @@ -146,6 +150,10 @@ func (r *Root) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case goToMsg: return r, r.goTo(msg.name) + case commandDoneMsg: + r.tell(msg.view, msg.status) + return r, nil + case tea.KeyMsg: if cmd, handled := r.key(msg); handled { return r, cmd @@ -259,6 +267,10 @@ func (r *Root) key(msg tea.KeyMsg) (tea.Cmd, bool) { return nil, true } + if cmd, ok := r.commandKey(top, msg); ok { + return cmd, true + } + switch msg.String() { case "esc": r.stack = r.stack[:len(r.stack)-1] @@ -308,7 +320,7 @@ func (r *Root) View() string { // from what it actually renders rather than from a constant. The title and // the status line are one row each. r.help.Width = r.width - helpView := r.help.View(withPalette(top.Keys(), r.paletteKey)) + helpView := r.help.View(withPalette(r.keysFor(top), r.paletteKey)) body := top.Body(r.width, max(1, r.height-lipgloss.Height(helpView)-2)) status, isErr := top.Status() diff --git a/internal/ui/rootcommands.go b/internal/ui/rootcommands.go new file mode 100644 index 0000000..3672021 --- /dev/null +++ b/internal/ui/rootcommands.go @@ -0,0 +1,196 @@ +package ui + +import ( + "fmt" + "maps" + "slices" + + "github.com/JacobAtchley/boardwalk/internal/config" + "github.com/charmbracelet/bubbles/help" + "github.com/charmbracelet/bubbles/key" + "github.com/charmbracelet/bubbles/list" + "github.com/charmbracelet/bubbles/viewport" + tea "github.com/charmbracelet/bubbletea" +) + +// WithCommands binds the user's custom commands. Check them with +// CheckCommands first. +func WithCommands(cmds []config.Command) RootOption { + return func(r *Root) { r.commands = cmds } +} + +// movementKeys are the keys a view moves or scrolls with: a list's cursor +// keys, and the default bubbles viewport keys the detail panes forward +// unhandled keys to. No view lists them in its help — they belong to +// bubbles — so the help-based collision check cannot see them, and Root sees +// a key before the view does. +func movementKeys() []string { + lm := list.DefaultKeyMap() + vm := viewport.DefaultKeyMap() + var keys []string + for _, b := range []key.Binding{ + lm.CursorUp, lm.CursorDown, lm.PrevPage, lm.NextPage, lm.GoToStart, lm.GoToEnd, lm.Filter, + vm.PageDown, vm.PageUp, vm.HalfPageDown, vm.HalfPageUp, vm.Up, vm.Down, vm.Left, vm.Right, + } { + keys = append(keys, b.Keys()...) + } + return keys +} + +// everyScreenKeys are the keys built in on every subject screen: the shared +// actions and refresh. A command on one would lose to the built-in each time +// (activeCommands drops it), so it passes startup and can never run. +func everyScreenKeys() []string { + var keys []string + for _, b := range append(sharedBindings(), keyRefresh) { + keys = append(keys, b.Keys()...) + } + return keys +} + +// CheckCommands reports a command whose key cannot work: one that is not a +// key, one Root needs, one a list moves with, one every screen already uses, +// or the palette's. +func CheckCommands(cmds []config.Command, paletteKey string) error { + if paletteKey == "" { + paletteKey = DefaultPaletteKey + } + moves := movementKeys() + everywhere := everyScreenKeys() + for i, c := range cmds { + switch { + case !validKey(c.Key): + return fmt.Errorf("commands[%d] key %q is not a key boardwalk recognises — try \"ctrl+r\"", i, c.Key) + case slices.Contains(reservedKeys, c.Key): + return fmt.Errorf("commands[%d] key %q is one boardwalk needs for itself", i, c.Key) + case slices.Contains(moves, c.Key): + return fmt.Errorf("commands[%d] key %q moves the cursor or scrolls", i, c.Key) + case slices.Contains(everywhere, c.Key): + return fmt.Errorf("commands[%d] key %q is already bound on every screen", i, c.Key) + case c.Key == paletteKey: + return fmt.Errorf("commands[%d] key %q opens the command palette", i, c.Key) + } + } + return nil +} + +func validKey(k string) bool { _, ok := keyMsgFor(k); return ok } + +// builtinKeys is every key the view on top answers to on its own: what its +// help lists, plus what Root and the shared actions claim on every view. +func builtinKeys(top View) map[string]bool { + keys := map[string]bool{} + add := func(bs ...key.Binding) { + for _, b := range bs { + for _, k := range b.Keys() { + keys[k] = true + } + } + } + for _, g := range top.Keys().FullHelp() { + add(g...) + } + add(sharedBindings()...) + add(navBindings()...) + add(keyRefresh) + return keys +} + +// activeCommands are the custom commands that run on the view on top: bound +// for its kind, on a key it does not use itself. +func (r *Root) activeCommands(top View) []config.Command { + s, ok := top.(subjecter) + if !ok || len(r.commands) == 0 { + return nil + } + subj, _ := s.Subject() + taken := builtinKeys(top) + var out []config.Command + for _, c := range r.commands { + if c.On == subj.kind && !taken[c.Key] { + out = append(out, c) + } + } + return out +} + +// commandKey runs the custom command bound to msg on the view on top, if +// there is one. +func (r *Root) commandKey(top View, msg tea.KeyMsg) (tea.Cmd, bool) { + i := slices.IndexFunc(r.activeCommands(top), func(c config.Command) bool { return c.Key == msg.String() }) + if i < 0 { + return nil, false + } + c := r.activeCommands(top)[i] + + subj, ok := top.(subjecter).Subject() + if !ok { + r.tell(top, StatusMsg{Text: "nothing selected"}) + return nil, true + } + + env := r.commandEnv(subj) + r.tell(top, StatusMsg{Text: fmt.Sprintf("running %q…", c.Name)}) + return func() tea.Msg { + return commandDoneMsg{view: top, status: runShell(c.Name, c.Run, env)} + }, true +} + +// tell delivers a status to v wherever it sits in the stack, and does +// nothing if it is no longer there. It is called synchronously for "running…" +// rather than returned as a command: a command finishing faster than that +// message was delivered would have its result overwritten by it. +func (r *Root) tell(v View, s StatusMsg) { + if i := slices.Index(r.stack, v); i >= 0 { + r.stack[i], _ = v.Update(s) + } +} + +// commandEnv is the BOARDWALK_ variables for a subject, in a fixed order so +// a test — or a user reading `env` — sees the same thing every time. +func (r *Root) commandEnv(s subject) []string { + env := []string{ + "BOARDWALK_KIND=" + s.kind, + "BOARDWALK_ORG=" + r.client.Org, + "BOARDWALK_PROJECT=" + r.client.Project, + } + for _, k := range slices.Sorted(maps.Keys(s.env)) { + env = append(env, "BOARDWALK_"+k+"="+s.env[k]) + } + return env +} + +// commandDoneMsg is how a custom command turned out, addressed to the view +// it ran on. A plain StatusMsg would go to whatever is on top when it lands, +// which after two seconds may be somewhere else entirely. +type commandDoneMsg struct { + view View + status StatusMsg +} + +// keysFor is the view's bindings with the custom commands active on it +// added, which is what the help line, the "?" panel and the palette read. +func (r *Root) keysFor(top View) help.KeyMap { + active := r.activeCommands(top) + if len(active) == 0 { + return top.Keys() + } + bindings := make([]key.Binding, len(active)) + for i, c := range active { + bindings[i] = key.NewBinding(key.WithKeys(c.Key), key.WithHelp(shownKey(c.Key), c.Name)) + } + return commandKeys{KeyMap: top.Keys(), commands: bindings} +} + +// commandKeys adds a group of custom commands to a view's own bindings. +type commandKeys struct { + help.KeyMap + commands []key.Binding +} + +func (k commandKeys) FullHelp() [][]key.Binding { + groups := k.KeyMap.FullHelp() + out := make([][]key.Binding, len(groups), len(groups)+1) + copy(out, groups) + return append(out, k.commands) +} diff --git a/internal/ui/rootcommands_test.go b/internal/ui/rootcommands_test.go new file mode 100644 index 0000000..4339ea3 --- /dev/null +++ b/internal/ui/rootcommands_test.go @@ -0,0 +1,322 @@ +package ui + +import ( + "slices" + "strings" + "testing" + + "github.com/JacobAtchley/boardwalk/internal/azdo" + "github.com/JacobAtchley/boardwalk/internal/config" + tea "github.com/charmbracelet/bubbletea" +) + +var ctrlR = tea.KeyMsg{Type: tea.KeyCtrlR} + +func reviewCommand() config.Command { + return config.Command{On: config.KindPullRequest, Key: "ctrl+r", Name: "review", Run: "review.sh"} +} + +type ranCommand struct { + name, script string + env []string +} + +// stubRunner records what would have run, and reports it ran. +func stubRunner(t *testing.T) *[]ranCommand { + t.Helper() + var ran []ranCommand + before := runShell + runShell = func(name, script string, env []string) StatusMsg { + ran = append(ran, ranCommand{name, script, env}) + return StatusMsg{Text: `ran "` + name + `"`} + } + t.Cleanup(func() { runShell = before }) + return &ran +} + +// rootOn builds a Root with cmds and v pushed on top. +func rootOn(t *testing.T, v View, cmds ...config.Command) *Root { + t.Helper() + c, _ := fixture() + r := NewRoot(c, false, false, "", WithCommands(cmds)) + r, _ = send(t, r, tea.WindowSizeMsg{Width: 140, Height: 30}, PushMsg{View: v}) + return r +} + +// run executes a command's tea.Cmd and delivers what it produced, the way +// bubbletea would. A batch is unpacked one level. +func run(t *testing.T, r *Root, cmd tea.Cmd) *Root { + t.Helper() + if cmd == nil { + return r + } + msg := cmd() + if batch, ok := msg.(tea.BatchMsg); ok { + for _, c := range batch { + r = run(t, r, c) + } + return r + } + r, _ = send(t, r, msg) + return r +} + +func statusOf(r *Root) (string, bool) { + top, _ := r.top() + text, isErr := top.Status() + // A pane pushed in a test never gets its fetches answered, so its spinner + // is still turning in front of the status. It is not what is asserted on. + return strings.TrimLeftFunc(text, func(r rune) bool { return r == ' ' || (r >= 0x2800 && r <= 0x28ff) }), isErr +} + +func TestRootRunsACommandWithTheSubject(t *testing.T) { + ran := stubRunner(t) + c, _ := fixture() + r := rootOn(t, NewPullRequestDetail(c, subjectPR(), nil), reviewCommand()) + + r, cmd := send(t, r, ctrlR) + if text, _ := statusOf(r); text != `running "review"…` { + t.Errorf("status while running = %q", text) + } + r = run(t, r, cmd) + + if len(*ran) != 1 || (*ran)[0].script != "review.sh" { + t.Fatalf("ran = %+v", *ran) + } + env := (*ran)[0].env + for _, want := range []string{ + "BOARDWALK_KIND=pullRequest", "BOARDWALK_ORG=acme", "BOARDWALK_PROJECT=Platform", + "BOARDWALK_ID=812", "BOARDWALK_SOURCE_BRANCH=feature/retry", + } { + if !slices.Contains(env, want) { + t.Errorf("env is missing %s: %v", want, env) + } + } + if text, isErr := statusOf(r); text != `ran "review"` || isErr { + t.Errorf("status = %q, %v", text, isErr) + } +} + +func TestRootIgnoresACommandForAnotherKind(t *testing.T) { + ran := stubRunner(t) + c, items := fixture() + r := rootOn(t, NewItem(c, items[0], nil), reviewCommand()) + + r, cmd := send(t, r, ctrlR) + r = run(t, r, cmd) + + if len(*ran) != 0 { + t.Errorf("a pull request command ran on a work item: %+v", *ran) + } +} + +func TestRootSaysNothingSelectedOnAnEmptyList(t *testing.T) { + ran := stubRunner(t) + c, _ := fixture() + r := rootOn(t, NewPullRequests(c), reviewCommand()) + + r, cmd := send(t, r, ctrlR) + r = run(t, r, cmd) + + if len(*ran) != 0 { + t.Errorf("ran with nothing selected: %+v", *ran) + } + if text, _ := statusOf(r); text != "nothing selected" { + t.Errorf("status = %q", text) + } +} + +func TestRootLetsTheViewsOwnKeyWin(t *testing.T) { + // A on the pull request pane approves. A custom command on A must not + // shadow it, and must not be advertised where it would not run. + ran := stubRunner(t) + c, _ := fixture() + approveAlias := config.Command{On: config.KindPullRequest, Key: "A", Name: "my approve", Run: "x"} + r := rootOn(t, NewPullRequestDetail(c, subjectPR(), nil), approveAlias) + + r, cmd := send(t, r, runes("A")) + r = run(t, r, cmd) + + if len(*ran) != 0 { + t.Errorf("the custom command shadowed a built-in key: %+v", *ran) + } + top, _ := r.top() + for _, g := range r.keysFor(top).FullHelp() { + for _, b := range g { + if b.Help().Desc == "my approve" { + t.Error("a shadowed command is listed in help") + } + } + } +} + +func TestRootSkipsACommandWhileAVoteIsArmed(t *testing.T) { + ran := stubRunner(t) + c, _ := fixture() + c.MyID = "me" + pr := subjectPR() + pr.Reviewers = []azdo.Reviewer{{Name: "Me", Key: c.Me, ID: "me"}} // armVote refuses a non-reviewer + r := rootOn(t, NewPullRequestDetail(c, pr, nil), reviewCommand()) + + r, _ = send(t, r, runes("A")) // arms the approve vote + r, cmd := send(t, r, ctrlR) + r = run(t, r, cmd) + + if len(*ran) != 0 { + t.Errorf("ran while a vote was armed: %+v", *ran) + } +} + +func TestRootDeliversAResultToTheViewItRanOn(t *testing.T) { + // The user can move on in the two seconds a command is watched. Its + // result belongs to the pane it ran on: not shown as news about the pane + // now on top, and there to read on coming back. + stubRunner(t) + c, items := fixture() + r := rootOn(t, NewPullRequestDetail(c, subjectPR(), nil), reviewCommand()) + + r, cmd := send(t, r, ctrlR) + r, _ = send(t, r, PushMsg{View: NewItem(c, items[0], nil)}) + r = run(t, r, cmd) + + if text, _ := statusOf(r); strings.Contains(text, "review") { + t.Errorf("the work item pane shows the pull request command's result: %q", text) + } + r, _ = send(t, r, PopMsg{}) + if text, _ := statusOf(r); text != `ran "review"` { + t.Errorf("back on the pull request, status = %q", text) + } +} + +func TestRootDropsAResultForAViewThatIsGone(t *testing.T) { + stubRunner(t) + c, _ := fixture() + r := rootOn(t, NewPullRequestDetail(c, subjectPR(), nil), reviewCommand()) + + r, cmd := send(t, r, ctrlR) + r, _ = send(t, r, PopMsg{}) + r = run(t, r, cmd) // must not panic, and has nowhere to go + + if len(r.stack) != 0 { + t.Errorf("stack = %v", r.stack) + } +} + +func TestRootListsCommandsInHelpAndRunsThemFromThePalette(t *testing.T) { + ran := stubRunner(t) + c, _ := fixture() + r := rootOn(t, NewPullRequestDetail(c, subjectPR(), nil), reviewCommand()) + + // The binding renders as "^r review". The word alone proves nothing: the + // pane has a "reviewers" label of its own. + const listed = "^r review" + if strings.Contains(r.View(), listed) { + t.Errorf("the closed footer already lists the command:\n%s", r.View()) + } + r, _ = send(t, r, runes("?")) + if !strings.Contains(r.View(), listed) { + t.Errorf("the help panel does not list the command:\n%s", r.View()) + } + r, _ = send(t, r, runes("?")) + + r, cmd := search(t, r, "review") + r = run(t, r, cmd) + if len(*ran) != 1 { + t.Errorf("running the palette entry ran %d commands", len(*ran)) + } +} + +func TestRootPaletteKeepsACommandNamedLikeABuiltIn(t *testing.T) { + ran := stubRunner(t) + c, _ := fixture() + cmd := config.Command{On: config.KindPullRequest, Key: "ctrl+r", Name: "refresh", Run: "refresh.sh"} + r := rootOn(t, NewPullRequestDetail(c, subjectPR(), nil), cmd) + + // Built-ins are listed first under "act:", so a command + // sharing the description would be deduplicated away and share history. + var builtin, custom *paletteEntry + entries := r.paletteEntries() + for i := range entries { + switch entries[i].id { + case "act:refresh": + builtin = &entries[i] + case "act:cmd:refresh": + custom = &entries[i] + } + } + if builtin == nil { + t.Fatal("the built-in refresh left the palette") + } + if custom == nil { + t.Fatalf("the custom command is not in the palette: %+v", entries) + } + r, runCmd := send(t, r, custom.msg) + r = run(t, r, runCmd) + if len(*ran) != 1 || (*ran)[0].script != "refresh.sh" { + t.Errorf("running the palette entry ran %+v", *ran) + } +} + +func TestCheckCommands(t *testing.T) { + cmd := func(k string) []config.Command { + return []config.Command{{On: config.KindBuild, Key: k, Name: "n", Run: "r"}} + } + for _, tc := range []struct { + key, palette, want string + }{ + {"ctrl+r", "", ""}, + {"K", "ctrl+k", ""}, + {"ctrl+banana", "", "not a key"}, + {"q", "", "needs for itself"}, + {"ctrl+c", "", "needs for itself"}, + {"ctrl+p", "", "opens the command palette"}, + {"ctrl+k", "ctrl+k", "opens the command palette"}, + } { + err := CheckCommands(cmd(tc.key), tc.palette) + switch { + case tc.want == "" && err != nil: + t.Errorf("%s: got %v", tc.key, err) + case tc.want != "" && (err == nil || !strings.Contains(err.Error(), tc.want)): + t.Errorf("%s: got %v, want %q", tc.key, err, tc.want) + } + } +} + +func TestCheckCommandsRefusesListMovementKeys(t *testing.T) { + // A list moves its cursor on keys no view lists in its help: l pages + // right, b pages back. Root sees a key before the view does, so a command + // on one would quietly take paging away from every list. + for _, k := range []string{"l", "h", "b", "u", "f", "pgup", "home", "G", " ", "ctrl+d", "ctrl+u"} { + err := CheckCommands([]config.Command{{On: config.KindBuild, Key: k, Name: "n", Run: "r"}}, "") + if err == nil || !strings.Contains(err.Error(), "moves the cursor or scrolls") { + t.Errorf("%s: got %v", k, err) + } + } +} + +func TestCheckCommandsRefusesKeysEveryScreenUses(t *testing.T) { + // Built in on every subject screen, these always win over a command, so + // a command on one would pass startup and never run. + for _, k := range []string{"r", "y", "ctrl+y", "s", "ctrl+s", "o", "ctrl+o"} { + err := CheckCommands([]config.Command{{On: config.KindBuild, Key: k, Name: "n", Run: "r"}}, "") + if err == nil || !strings.Contains(err.Error(), "already bound on every screen") { + t.Errorf("%s: got %v", k, err) + } + } +} + +func TestRootRunsACommandOnAFileView(t *testing.T) { + // The file pane is reached by drilling in from a diff, so Root is the only + // way to see its Subject at work. + ran := stubRunner(t) + c, _ := fixture() + fv := NewFileView(c, subjectPR(), "internal/retry.go", "package retry\n", nil, nil, filterAll) + r := rootOn(t, fv, reviewCommand()) + + r, cmd := send(t, r, ctrlR) + r = run(t, r, cmd) + + if len(*ran) != 1 || !slices.Contains((*ran)[0].env, "BOARDWALK_ID=812") { + t.Errorf("ran = %+v", *ran) + } +} diff --git a/internal/ui/rootpalette.go b/internal/ui/rootpalette.go index eb92c96..13e21b2 100644 --- a/internal/ui/rootpalette.go +++ b/internal/ui/rootpalette.go @@ -63,11 +63,15 @@ func CheckPaletteKey(k string) error { // paletteBinding is the binding for the palette key, with its help spelled // the way the rest of the footer spells control keys: ^p, not ctrl+p. func paletteBinding(k string) key.Binding { - shown := k + return key.NewBinding(key.WithKeys(k), key.WithHelp(shownKey(k), "commands")) +} + +// shownKey spells a key the way the footer does: ^p, not ctrl+p. +func shownKey(k string) string { if rest, ok := strings.CutPrefix(k, "ctrl+"); ok && len(rest) == 1 { - shown = "^" + rest + return "^" + rest } - return key.NewBinding(key.WithKeys(k), key.WithHelp(shown, "commands")) + return k } // goToMsg asks Root to show one of the main views, from however deep the @@ -126,30 +130,42 @@ func (r *Root) paletteEntries() []paletteEntry { return entries } + // A custom command gets an id namespace of its own: built-ins come first + // under "act:", so a command named "refresh" would otherwise + // be dropped as a duplicate of the built-in and share its history. seen := make(map[string]struct{}, 32) - for _, group := range top.Keys().FullHelp() { - for _, b := range group { - h := b.Help() - if !b.Enabled() || h.Desc == "" || len(b.Keys()) == 0 { - continue - } - // "?" would open a panel listing these same entries. - first := b.Keys()[0] - if slices.Contains(keyHelp.Keys(), first) { - continue + add := func(groups [][]key.Binding, prefix string) { + for _, group := range groups { + for _, b := range group { + h := b.Help() + if !b.Enabled() || h.Desc == "" || len(b.Keys()) == 0 { + continue + } + // "?" would open a panel listing these same entries. + first := b.Keys()[0] + if slices.Contains(keyHelp.Keys(), first) { + continue + } + id := prefix + h.Desc + if _, dup := seen[id]; dup { + continue + } + msg, ok := keyMsgFor(first) + if !ok { + continue + } + seen[id] = struct{}{} + entries = append(entries, paletteEntry{id: id, title: h.Desc, hint: h.Key, msg: msg}) } - id := "act:" + h.Desc - if _, dup := seen[id]; dup { - continue - } - msg, ok := keyMsgFor(first) - if !ok { - continue - } - seen[id] = struct{}{} - entries = append(entries, paletteEntry{id: id, title: h.Desc, hint: h.Key, msg: msg}) } } + km := r.keysFor(top) + if ck, ok := km.(commandKeys); ok { + add(ck.KeyMap.FullHelp(), "act:") + add([][]key.Binding{ck.commands}, "act:cmd:") + } else { + add(km.FullHelp(), "act:") + } return entries } diff --git a/internal/ui/statusmsg_test.go b/internal/ui/statusmsg_test.go new file mode 100644 index 0000000..77fc682 --- /dev/null +++ b/internal/ui/statusmsg_test.go @@ -0,0 +1,45 @@ +package ui + +import ( + "strings" + "testing" +) + +func TestSubjectViewsShowAStatusMsgWithItsColour(t *testing.T) { + c, items := fixture() + for _, tc := range []struct { + name string + view View + }{ + {"logs", NewLogs(c, buildStub(), nil)}, + {"pr detail", NewPullRequestDetail(c, subjectPR(), nil)}, + {"item", NewItem(c, items[0], nil)}, + } { + v, _ := tc.view.Update(StatusMsg{Text: "boom", Err: true}) + if text, isErr := v.Status(); !strings.HasSuffix(text, "boom") || !isErr { + t.Errorf("%s: after an error, Status() = %q, %v", tc.name, text, isErr) + } + v, _ = v.Update(StatusMsg{Text: "fine"}) + if text, isErr := v.Status(); !strings.HasSuffix(text, "fine") || isErr { + t.Errorf("%s: after a success, Status() = %q, %v", tc.name, text, isErr) + } + } +} + +func TestAStatusErrorDoesNotClaimTheDiscussionFailed(t *testing.T) { + // failed means the discussion did not load, and the body says so. A + // command failing is not that. + c, items := fixture() + for _, tc := range []struct { + name string + view View + }{ + {"pr detail", NewPullRequestDetail(c, subjectPR(), nil)}, + {"item", NewItem(c, items[0], nil)}, + } { + v, _ := tc.view.Update(StatusMsg{Text: "boom", Err: true}) + if body := v.Body(120, 40); strings.Contains(body, "could not load the discussion") { + t.Errorf("%s: a status error replaced the discussion:\n%s", tc.name, body) + } + } +} diff --git a/internal/ui/subject.go b/internal/ui/subject.go new file mode 100644 index 0000000..6b9d2ab --- /dev/null +++ b/internal/ui/subject.go @@ -0,0 +1,64 @@ +package ui + +import ( + "fmt" + "strconv" + "strings" + + "github.com/JacobAtchley/boardwalk/internal/azdo" + "github.com/JacobAtchley/boardwalk/internal/config" +) + +// subject is the one thing a screen is showing that a custom command can act +// on: its kind, and the variables that describe it, named without their +// BOARDWALK_ prefix. Root adds the prefix and the variables every kind shares. +type subject struct { + kind string + env map[string]string +} + +// subjecter is a view a custom command can run on. kind is set even when ok +// is false — an empty list is still a pull request screen, and Root has to +// know which commands to say "nothing selected" for. +type subjecter interface { + Subject() (s subject, ok bool) +} + +// refBranch strips refs/heads/. shortRef is not used: it answers "-" for an +// empty ref, which reads well in a column and badly in a script. +func refBranch(ref string) string { return strings.TrimPrefix(ref, "refs/heads/") } + +func pullRequestSubject(c *azdo.Client, pr azdo.PullRequest) subject { + return subject{kind: config.KindPullRequest, env: map[string]string{ + "ID": strconv.Itoa(pr.ID), + "TITLE": pr.Title, + "URL": c.PullRequestURL(pr.Repo, pr.ID), + "REPO": pr.Repo, + "SOURCE_BRANCH": refBranch(pr.Source), + "TARGET_BRANCH": refBranch(pr.Target), + "AUTHOR": pr.Author, + "IS_DRAFT": strconv.FormatBool(pr.IsDraft), + }} +} + +func workItemSubject(c *azdo.Client, wi azdo.WorkItem) subject { + return subject{kind: config.KindWorkItem, env: map[string]string{ + "ID": strconv.Itoa(wi.ID), + "TITLE": wi.Title, + "URL": c.WorkItemURL(wi.ID), + "TYPE": wi.Type, + "STATE": wi.State, + }} +} + +func buildSubject(c *azdo.Client, b azdo.Build) subject { + return subject{kind: config.KindBuild, env: map[string]string{ + "ID": strconv.Itoa(b.ID), + "TITLE": fmt.Sprintf("%s #%s", b.Pipeline, b.Number), + "URL": c.BuildURL(b.ID), + "NUMBER": b.Number, + "PIPELINE": b.Pipeline, + "BRANCH": refBranch(b.SourceBranch), + "RESULT": b.Status.String(), + }} +} diff --git a/internal/ui/subject_test.go b/internal/ui/subject_test.go new file mode 100644 index 0000000..c67e4b0 --- /dev/null +++ b/internal/ui/subject_test.go @@ -0,0 +1,111 @@ +package ui + +import ( + "maps" + "testing" + + "github.com/JacobAtchley/boardwalk/internal/azdo" + "github.com/JacobAtchley/boardwalk/internal/config" +) + +func subjectPR() azdo.PullRequest { + return azdo.PullRequest{ID: 812, Title: "Retry on 5xx", Repo: "platform", Author: "Dev Example", + IsDraft: true, Source: "refs/heads/feature/retry", Target: "refs/heads/main"} +} + +func TestPullRequestSubject(t *testing.T) { + c, _ := fixture() + want := map[string]string{ + "ID": "812", "TITLE": "Retry on 5xx", "URL": c.PullRequestURL("platform", 812), + "REPO": "platform", "SOURCE_BRANCH": "feature/retry", "TARGET_BRANCH": "main", + "AUTHOR": "Dev Example", "IS_DRAFT": "true", + } + got := pullRequestSubject(c, subjectPR()) + if got.kind != config.KindPullRequest || !maps.Equal(got.env, want) { + t.Errorf("subject = %+v, want %v", got, want) + } +} + +func TestWorkItemSubject(t *testing.T) { + c, items := fixture() + wi := items[0] + want := map[string]string{ + "ID": "4021", "TITLE": wi.Title, "URL": c.WorkItemURL(4021), + "TYPE": "User Story", "STATE": "Active", + } + got := workItemSubject(c, wi) + if got.kind != config.KindWorkItem || !maps.Equal(got.env, want) { + t.Errorf("subject = %+v, want %v", got, want) + } +} + +func TestBuildSubject(t *testing.T) { + c, _ := fixture() + b := buildStub() + b.SourceBranch = "refs/heads/main" + want := map[string]string{ + "ID": "9001", "TITLE": "platform-ci #20260911.3", "URL": c.BuildURL(9001), + "NUMBER": "20260911.3", "PIPELINE": "platform-ci", "BRANCH": "main", "RESULT": "succeeded", + } + got := buildSubject(c, b) + if got.kind != config.KindBuild || !maps.Equal(got.env, want) { + t.Errorf("subject = %+v, want %v", got, want) + } +} + +func TestEveryViewWithASubjectNamesIt(t *testing.T) { + c, items := fixture() + pr := subjectPR() + + prs := NewPullRequests(c) + // NewPullRequests scopes to the working directory's repository when there + // is one; pin it so the test does not depend on this checkout. + prs.repoOnly = false + prs.drafts = draftsAll // subjectPR is a draft, which the list hides by default + prs.Update(prsMsg{PRs: []azdo.PullRequest{pr}}) + wis := NewWorkItems(c, items, false, false) + builds := NewBuilds(c) + builds.Update(buildsMsg{Builds: []azdo.Build{buildStub()}}) + + for _, tc := range []struct { + name string + view View + kind string + id string + }{ + {"pr list", prs, config.KindPullRequest, "812"}, + {"pr detail", NewPullRequestDetail(c, pr, nil), config.KindPullRequest, "812"}, + {"pr diff", NewPullRequestDiff(c, pr, nil, filterAll), config.KindPullRequest, "812"}, + {"work items", wis, config.KindWorkItem, "4021"}, + {"item", NewItem(c, items[0], nil), config.KindWorkItem, "4021"}, + {"builds", builds, config.KindBuild, "9001"}, + {"logs", NewLogs(c, buildStub(), nil), config.KindBuild, "9001"}, + } { + s, ok := tc.view.(subjecter) + if !ok { + t.Errorf("%s has no Subject", tc.name) + continue + } + got, ok := s.Subject() + if !ok || got.kind != tc.kind || got.env["ID"] != tc.id { + t.Errorf("%s: Subject() = %+v, %v; want %s %s", tc.name, got, ok, tc.kind, tc.id) + } + } +} + +func TestAnEmptyListHasNoSubjectButKeepsItsKind(t *testing.T) { + c, _ := fixture() + for _, tc := range []struct { + view subjecter + kind string + }{ + {NewPullRequests(c), config.KindPullRequest}, + {NewWorkItems(c, nil, false, false), config.KindWorkItem}, + {NewBuilds(c), config.KindBuild}, + } { + got, ok := tc.view.Subject() + if ok || got.kind != tc.kind { + t.Errorf("%T: Subject() = %+v, %v; want kind %s and nothing selected", tc.view, got, ok, tc.kind) + } + } +} diff --git a/internal/ui/workitems.go b/internal/ui/workitems.go index 8e0eafc..6295a0d 100644 --- a/internal/ui/workitems.go +++ b/internal/ui/workitems.go @@ -7,6 +7,7 @@ import ( "time" "github.com/JacobAtchley/boardwalk/internal/azdo" + "github.com/JacobAtchley/boardwalk/internal/config" "github.com/charmbracelet/bubbles/help" "github.com/charmbracelet/bubbles/key" "github.com/charmbracelet/bubbles/spinner" @@ -462,3 +463,13 @@ func (m *WorkItems) Status() (string, bool) { func (m *WorkItems) Prompting() bool { return m.statePicker != nil || m.browser.Filtering() } + +// Subject is the selected work item, for a custom command. +func (m *WorkItems) Subject() (subject, bool) { + if row, ok := m.browser.Selected(); ok { + if r, ok := row.(workItemRow); ok { + return workItemSubject(m.client, r.WorkItem), true + } + } + return subject{kind: config.KindWorkItem}, false +} diff --git a/main.go b/main.go index 86a6e9d..2a8657b 100644 --- a/main.go +++ b/main.go @@ -142,7 +142,8 @@ func loadConfig() (config.Config, error) { // rootOptions turns the palette settings into options for the TUI: the key // that opens it, checked here so a typo is reported at startup rather than -// as a key that silently does nothing, and where its history is kept. +// as a key that silently does nothing, the custom commands, checked and bound +// the same way, and where the palette's history is kept. func rootOptions(cfg config.Config) ([]ui.RootOption, error) { var opts []ui.RootOption if cfg.PaletteKey != "" { @@ -152,6 +153,16 @@ func rootOptions(cfg config.Config) ([]ui.RootOption, error) { } opts = append(opts, ui.WithPaletteKey(cfg.PaletteKey)) } + if len(cfg.Commands) > 0 { + path, _ := config.Path() + if err := cfg.ValidateCommands(path); err != nil { + return nil, err + } + if err := ui.CheckCommands(cfg.Commands, cfg.PaletteKey); err != nil { + return nil, fmt.Errorf("%s: %w", path, err) + } + opts = append(opts, ui.WithCommands(cfg.Commands)) + } // No history path means no home directory to put it in; the palette // then remembers for this session only, which is the default. if path, err := config.HistoryPath(); err == nil { diff --git a/main_test.go b/main_test.go index cfa907a..dbddd09 100644 --- a/main_test.go +++ b/main_test.go @@ -117,3 +117,25 @@ func TestRootOptionsRejectsAPaletteKeyItCannotUse(t *testing.T) { t.Errorf("an unset key gave %d options, want only the history", len(opts)) } } + +func TestRootOptionsChecksCommands(t *testing.T) { + t.Setenv("BOARDWALK_CONFIG", t.TempDir()+"/boardwalk.json") + good := config.Command{On: config.KindPullRequest, Key: "ctrl+r", Name: "review", Run: "review.sh"} + + opts, err := rootOptions(config.Config{Commands: []config.Command{good}}) + if err != nil || len(opts) != 2 { + t.Errorf("rootOptions = %d options, %v; want the commands and the history", len(opts), err) + } + + bad := good + bad.On = "pr" + if _, err := rootOptions(config.Config{Commands: []config.Command{bad}}); err == nil { + t.Error("an unknown kind was accepted") + } + + bad = good + bad.Key = "q" + if _, err := rootOptions(config.Config{Commands: []config.Command{bad}}); err == nil { + t.Error("a reserved key was accepted") + } +}