feat: Validate + keypress subpackage; RFC-style design.md (v1.2.0)

Validate(root): optional startup/test check for tree authoring faults —
>1 slot child per node, empty word, duplicate sibling words, dead-end
node — traversing circular slots without looping (#3).

keypress subpackage: WaitForKey(ctx, cancel) cancels a context on any
keystroke for watch-style streaming commands, with per-GOOS cbreak
(linux TCGETS/TCSETS, BSD TIOCGETA/TIOCSETA) and a non-tty/unsupported
fallback that just waits on ctx. Lifts the last OpenBSD-specific bit out
of evpnc/maglevc's watch.go (#6).

docs: replace PROPOSAL.md with an RFC-2119 design.md (FR/NFR for the
library). Example now dogfoods Validate (a unit test) and keypress (a
bounded `watch` command).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-06-05 22:11:13 +02:00
parent e030cd28e9
commit 9e0a98ed07
12 changed files with 530 additions and 260 deletions
-260
View File
@@ -1,260 +0,0 @@
<!--
SPDX-FileCopyrightText: (C) Copyright 2026 Pim van Pelt <pim@ipng.ch>
SPDX-License-Identifier: Apache-2.0
-->
# Proposal: extract the command-tree CLI into a reusable `golang-cli` package
Status: draft / for discussion. Nothing in `vpp-evpn` or `vpp-maglev` has been
changed. This document describes how to lift the command-tree CLI out of
`cmd/evpnc` (vpp-evpn) and `cmd/client` (vpp-maglev) into a standalone module at
`~/src/golang-cli`, and re-import it in both with **no functional difference**.
## 1. Why
Both projects carry the same hand-rolled CLI: a declarative command tree from
which dispatch, `?`-help, and tab-completion are all derived, driven by
`chzyer/readline`. The two copies have already drifted (see §3), and a third
copy would just cargo-cult the drift forward. The parser, the dynamic-node
mechanism, and the command registration are genuinely generic — only the gRPC
*client type* and the *tree contents* are app-specific.
## 2. The three components (as you framed them)
| Your name | Concrete artifact today | Reusable? |
|---|---|---|
| "the parsing tree" | `Node` struct + `Walk` / `matchFixedChild` / `findSlotChild` / `expandPaths` (`tree.go`) | **Yes, verbatim** (modulo client type) |
| "dynamic node function registration" | `Node.Dynamic func(ctx, client, args) []string`, resolved in `Candidates` and the `?` listener | **Yes** — this is the only real API-shape decision (see §3.1) |
| "command function registration" | `Node.Run func(ctx, client, args) error`, dispatched by `dispatch` | **Yes, verbatim** |
Everything that *consumes* the tree is also generic and moves with it:
`Completer` (readline `AutoCompleter`), `questionListener` (the `?` key handler),
`dispatch`, `showHelpAt`, `unknownCommandError`, the REPL loop (`runShell`),
`splitTokens` / `splitForCompletion`, and the OpenBSD termios shim
(`shell_term_openbsd.go` / `_default.go`).
What does **not** move — it stays in each app:
- `buildTree()` and every `run*` / `dyn*` function (`commands.go`) — the actual
command set.
- `color.go` — the `formatError` gRPC-status unwrap stays in-app (wired via
`Shell.FormatError`). The generic `label`/`paint`/`colorEnabled` half **moved
into the library** as `cli.Label`/`cli.Paint`/`cli.SetColor` (see `color.go`,
`output.go`).
- `watch.go` (gRPC server-stream consumer) — app/proto-specific. *But* its
generic half — "cancel a context on any keypress" — can optionally move (§4.3).
- `main.go` — flag parsing, gRPC dial, color-mode defaults.
## 3. What differs between the two copies today
The copies are ~95% identical. The differences, and how the library reconciles
them so neither app changes behaviour:
### 3.1 `Dynamic` signature — the one real decision
- `vpp-evpn`: `func(context.Context, pb.EvpnrClient, []string) []string` — the
trailing `[]string` is the args captured by earlier slot nodes, so `<evpn>`
can list the EVPNs *of the `<id>` already typed*.
- `vpp-maglev`: `func(context.Context, grpcapi.MaglevClient) []string` — no args.
The evpn signature is a strict superset. **The library adopts the args-bearing
form.** maglev's `dyn*` functions gain an ignored `_ []string` parameter — a
purely mechanical edit, zero behaviour change.
### 3.2 Client type — solved with generics
Both projects are `go 1.25`, so generics are available. The package is generic
over the client type `C`:
```go
type Node[C any] struct {
Word string
Help string
Dynamic func(context.Context, C, []string) []string // non-nil => slot node
Children []*Node[C]
Run func(context.Context, C, []string) error
}
```
`C` is `pb.EvpnrClient` in evpnc and `grpcapi.MaglevClient` in maglevc. Run/Dynamic
funcs receive the concrete client — **no `any`, no type assertions** in app code.
(Alternative considered: a non-generic `any` client with assertions in every
`run*`. Rejected — it pushes boilerplate and runtime panics into the apps. See §7.)
### 3.3 Cosmetic drift the library erases (all no-ops behaviourally)
- `Candidates` arg order: evpnc `(ctx, client, root, tokens, partial)` vs maglev
`(root, tokens, partial, ctx, client)`. Library picks the evpnc order.
- evpnc's `Candidates` passes captured `args` to `Dynamic`; maglev discards them
(`node, _, remaining`). Library passes them (superset; maglev's funcs ignore).
- `RunShell`: evpnc calls `applyTermFuncs(cfg)` (OpenBSD fix), maglev does not.
Library always calls it — it's a no-op off OpenBSD, so maglev gains the fix
with no change on Linux/macOS.
- `splitTokens` lives in `complete.go` (maglev) vs `complete.go` (evpnc) — same
body; library exports one `SplitTokens`.
- The `?` listener's partial-token branch carries different comments but identical
logic; unified.
## 4. Proposed package shape
Module: `git.ipng.ch/ipng/golang-cli` (matches your git host). Single package
`cli` to start; the optional keypress helper can be a subpackage.
```
~/src/golang-cli/
go.mod module git.ipng.ch/ipng/golang-cli, go 1.25
go.sum
LICENSE Apache-2.0 (matches the SPDX headers already in the files)
README.md
Makefile check (= fixstyle vet lint test), build, cross (linux + openbsd)
tree.go Node[C], Walk, matchFixedChild, findSlotChild, expandPaths, Candidates
tree_test.go table tests over a fixture tree (no client needed; Walk ignores Dynamic)
builder.go For[C] / Builder[C]: Root/Dir/Cmd/SlotDir/Slot (no repeated [C])
complete.go completer[C], questionListener[C], SplitTokens, splitForCompletion
shell.go Shell[C] (config + Run), Dispatch, showHelpAt, unknownCommandError, ErrQuit
app.go App[C]: standard flags + banner + connect + one-shot/shell -> Main()
color.go SetColor, Paint, Label + the ANSI palette (Red/Green/Blue/Yellow/Cyan)
output.go SetFormat, Emit (text-or-JSON), KV, IsJSON
term_openbsd.go applyTermFuncs (//go:build openbsd) <- from shell_term_openbsd.go
term_default.go applyTermFuncs no-op (//go:build !openbsd) <- from shell_term_default.go
example/main.go self-contained, dependency-free demo CLI (uses Builder + App)
keypress/ (optional) WaitForKeypress + cbreak shim, per-GOOS <- §4.3
```
### 4.1 The one new type: `Shell[C]`
The REPL loop today hard-codes the prompt, the banner, the `errQuit` sentinel,
and `formatError`. Those become a small config struct so each app keeps its
exact strings and error rendering:
```go
type Shell[C any] struct {
Root *Node[C]
Client C
Prompt string // "evpn> " / "maglev> "
FormatError func(error) string // default: err.Error(); evpnc/maglev pass their gRPC-desc unwrap
}
func (s *Shell[C]) Run(ctx context.Context) error // the readline REPL loop
func Dispatch[C any](ctx context.Context, root *Node[C], client C, tokens []string) error
var ErrQuit = errors.New("quit") // a quit/exit Run func returns this; Run() stops on it
```
Banner printing stays in `main.go` (it needs `buildinfo.Version()` etc., which
is app-specific) — `main` prints the banner, then calls `shell.Run(ctx)`.
`FormatError` is how each app keeps its gRPC `desc =` unwrap + red coloring
without the library depending on gRPC at all.
### 4.2 What the app's `main.go` looks like after (evpnc)
```go
root := buildTree() // returns *cli.Node[pb.EvpnrClient]
if len(args) == 0 {
fmt.Printf("evpnc %s ...\n", buildinfo.Version(), ...) // banner
return (&cli.Shell[pb.EvpnrClient]{
Root: root, Client: client, Prompt: "evpn> ", FormatError: formatError,
}).Run(ctx)
}
return cli.Dispatch(ctx, root, client, cli.SplitTokens(strings.Join(args, " ")))
```
`buildTree`, `runQuit` (`return cli.ErrQuit`), all `run*`/`dyn*`, `color.go`,
`watch.go` are unchanged except `Node``cli.Node[pb.EvpnrClient]` and the
`dyn*` signature tweak in maglev.
### 4.3 Optional: the keypress helper
`watch.go` in both apps shares "put stdin in cbreak, cancel ctx on any key", with
per-GOOS termios ioctls (`watch_linux.go` / `watch_bsd.go` in evpnc; inline in
maglev). The proto-streaming half is app-specific and stays, but
`WaitForKeypress(ctx, cancel)` + the cbreak shim are reusable. Suggest a
`cli/keypress` subpackage so apps don't recopy the ioctl tables. Low priority —
it's the least-drifted, most-self-contained piece. Can land in a follow-up.
## 5. Migration plan
**Phase 0 — stand up the module (no app changes).**
1. `git init ~/src/golang-cli`; `go mod init git.ipng.ch/ipng/golang-cli`; `go 1.25`.
2. Copy `tree.go`, `complete.go`, `shell.go` pieces, `shell_term_*.go` in.
Make `Node`, `Walk`, `Candidates`, `Completer`, `questionListener`, `Dispatch`,
`Shell` generic over `C`. Add `Shell.FormatError` and `ErrQuit`.
3. Port `tree_test.go` to a self-contained fixture tree (a tiny `Node[*fakeClient]`
with a couple of slot nodes + a circular watch-opts node) so the package tests
`Walk`/`expandPaths`/`Candidates` with no gRPC dependency. `go test ./...` green.
4. Dependencies: `github.com/chzyer/readline`, `golang.org/x/sys/unix` (both
already in each app's `go.sum`). Tag `v0.1.0`.
**Phase 1 — convert vpp-evpn (the args-bearing reference, less work).**
1. `go get git.ipng.ch/ipng/golang-cli@v0.1.0`; during local dev add a
`replace git.ipng.ch/ipng/golang-cli => ../golang-cli` to iterate.
2. Delete `tree.go`, `complete.go`, `shell.go` body, `shell_term_*.go` from
`cmd/evpnc`. Keep `commands.go`, `color.go`, `watch*.go`, `main.go`.
3. Replace `*Node` with `*cli.Node[pb.EvpnrClient]`; `runQuit` returns
`cli.ErrQuit`; `main` builds a `cli.Shell` (§4.2). `Walk`/`expandPaths` refs in
`watch.go`? none — good.
4. Run the existing checkpoint: `make fixstyle test lint vet`. The
`cmd/evpnc/tree_test.go` `TestWalk`/`TestExpandPaths` move to exercising
`buildTree()` through the library's exported `Walk`/`ExpandPaths` (rename as
needed) — they assert the *app's* tree, so they stay in the app.
**Phase 2 — convert vpp-maglev.**
1. Same wiring. Plus the mechanical edits from §3.1: add `_ []string` to every
`dyn*`; `Candidates`/`Dynamic` call sites now come from the library.
2. maglev's `main.go` color-default logic is equivalent to evpnc's; unchanged.
3. `make` checkpoint green.
**Phase 3 — drop the replace directives, tag a real version, pin both apps.**
## 6. Risks / things to verify during the port
- **Generics + method values.** `Node[C]` is fine; the only subtlety is that
`Completer[C]` and `questionListener[C]` must carry the type param through to
their `readline` interface methods. `readline.AutoCompleter` / `Listener` are
non-generic interfaces, but a concrete `*Completer[pb.EvpnrClient]` satisfies
them — verified shape, no reflection needed.
- **The `?` echo trick** (`questionListener.OnChange` writing the prompt+line to
`rl.Stderr()` before help) depends on readline's wrapWriter behaviour. It's
copied verbatim; no change.
- **OpenBSD.** evpnc has the termios shim and a `make pkg-openbsd`; the library
must keep the `//go:build openbsd` split so maglev gets it without pulling
OpenBSD code into its Linux builds. Verify a `GOOS=openbsd go build` of the
library.
- **No hidden client coupling.** `Walk`/`expandPaths` never touch the client
(the existing `TestWalk` comment confirms it) — so they could even be
non-generic with `Dynamic any`. Keeping them generic is simpler and uniform.
## 7. Open decisions for you
1. **Generics vs `any` client.** Recommended: generics (§3.2) — type-safe,
zero boilerplate in apps. The cost is `[]*cli.Node[pb.EvpnrClient]` verbosity
in `commands.go`; a per-app `type node = cli.Node[pb.EvpnrClient]` alias hides
it entirely.
2. **Module host/name.** `git.ipng.ch/ipng/golang-cli` (consistent) vs a GitHub
mirror if you want it public. Package name `cli` vs something less likely to
collide (`cmdtree`, `cmdline`).
3. **Scope of v0.1.** Minimum = tree + completion + shell. Stretch = the
`keypress` subpackage (§4.3) and maybe a tiny `color` helper if a third client
wants it. Suggest shipping the minimum first; both apps prove the API before
it ossifies.
4. **Whose `Dynamic` shape wins** — already answered (args-bearing, §3.1) unless
you'd rather keep maglev argless and have evpnc pass args by closure capture.
The superset is cleaner.
## 8. Bottom line
"What would it take to make this its own standalone package?" — concretely:
1. A new `go 1.25` module with three files made generic over the client type
(`tree.go`, `complete.go`, `shell.go`) plus the two OpenBSD term files and a
self-contained test — ~1 day including tests.
2. One genuine API choice (args-bearing `Dynamic`, generic `C`) and one new
config type (`Shell[C]` with an injectable `FormatError`). Everything else is
moved verbatim.
3. Mechanical edits in each app: swap `Node` for `cli.Node[ClientT]`, route the
REPL through `cli.Shell`, return `cli.ErrQuit` from quit, and (maglev only)
add an ignored args param to each `dyn*`. No behaviour changes; both `make`
checkpoints stay green.
The tree, the dynamic-node mechanism, and the command/run registration are all
already cleanly separated in these files — the refactor is mostly *relocation +
generics*, not redesign.
+158
View File
@@ -0,0 +1,158 @@
<!--
SPDX-FileCopyrightText: (C) Copyright 2026 Pim van Pelt <pim@ipng.ch>
SPDX-License-Identifier: Apache-2.0
-->
# golang-cli Design Document
| | |
| --- | --- |
| **Status** | Describes shipped behavior as of `v1.2.0` |
| **Author** | Pim van Pelt `<pim@ipng.ch>` |
| **Last updated** | 2026-06-05 |
| **Audience** | Contributors, and authors of CLIs built on this library |
The key words **MUST**, **MUST NOT**, **SHOULD**, **SHOULD NOT**, and **MAY** are
used as described in [RFC 2119](https://datatracker.ietf.org/doc/html/rfc2119),
and are reserved for requirements enforced in code. Plain-language "can"/"will"/
"does" are descriptive, not normative.
## Summary
`golang-cli` builds network-daemon-style CLIs: an interactive shell with
tab-completion and `?`-help, plus a one-shot mode for scripts, both driven by one
declarative command tree. It is the shared core extracted from `evpnc`
(`vpp-evpn`) and `maglevc` (`vpp-maglev`), which had each grown their own copy.
It is generic over a *client* type, and renders command output as colorized text
or JSON from the same code.
## Goals
1. **One tree, one truth.** Dispatch, help, and completion all derive from a
single command tree, so they cannot drift.
2. **Type-safe over any backend.** Command code receives its concrete client, no
runtime type assertions.
3. **Drop-in for `evpnc` and `maglevc`** with no functional regression.
4. **Low ceremony.** A tree and a `main()` in a few lines, not a copied file.
5. **Text or JSON** from the same command code.
6. **Linux and OpenBSD** are both first-class.
## Non-Goals
- Not a flag parser (uses stdlib `flag`) and not a TUI toolkit.
- Does not own transport: it never dials, connects, or speaks a wire protocol.
- Defines no command set, no output format — those are the caller's.
- Holds no state beyond two process-global render switches (color, format).
## Functional Requirements
**FR-1 Command tree and resolution**
- **FR-1.1** The command set MUST be one tree of `Node[C]`; dispatch, `?`-help,
and completion MUST all derive from it, with no second command table.
- **FR-1.2** `Walk` MUST prefer a fixed child over the slot child, matching fixed
by exact word then unique prefix.
- **FR-1.3** A unique-prefix token MUST resolve to that child; an ambiguous
prefix MUST NOT resolve.
- **FR-1.4** A node MAY both run and have children.
- **FR-1.5** A slot node (`Dynamic != nil`) MUST capture any token as an
argument; captured args MUST reach the matched node's `Run` in order.
- **FR-1.6** An unconsumable token MUST be returned to the caller and reported as
an unknown-command error naming the first bad token, not anchored at the root.
- **FR-1.7** At most one slot child per node is reachable; `Validate` SHOULD flag
more than one.
**FR-2 Completion and help**
- **FR-2.1** `Dynamic` MUST receive the args captured by earlier slots, so
candidates can depend on tokens already typed.
- **FR-2.2** Completion MUST offer fixed children and slot values that have the
partial token as a prefix.
- **FR-2.3** If a confirmed token is unknown, completion MUST offer nothing.
- **FR-2.4** Each completion/`?` `Dynamic` call MUST be timeout-bounded
(default 1s).
- **FR-2.5** `?` MUST list the reachable runnable paths with help and live slot
values, without submitting the line.
**FR-3 Shell and one-shot**
- **FR-3.1** No positional args MUST start the REPL; positional args MUST run one
command and exit.
- **FR-3.2** A command MUST be able to stop the REPL by returning `ErrQuit`.
- **FR-3.3** A REPL command error MUST be printed and MUST NOT stop the loop; a
one-shot command error MUST exit non-zero.
- **FR-3.4** `Ctrl-C` MUST abandon the line and continue; `Ctrl-D` MUST exit.
**FR-4 Output**
- **FR-4.1** A command MUST be able to describe one result once and have it
rendered as text or JSON per a process-global format (`Emit`).
- **FR-4.2** In JSON mode the framework MUST marshal the command's machine value;
supplying it is the command's responsibility.
- **FR-4.3** With color off or JSON selected, no ANSI escapes MUST be emitted.
- **FR-4.4** Color MUST default on in the shell, off one-shot; `-color` overrides;
`-json` forces it off.
**FR-5 `App` entry point**
- **FR-5.1** `App` MUST register `-color`, `-json`, `-version`, and `-server`
(only when a server is configured).
- **FR-5.2** `App` MUST NOT dial anything; the client is built in a caller's
`Connect` callback.
- **FR-5.3** `-version` MUST print and exit without connecting.
- **FR-5.4** The server address MUST resolve from `-server`, else the configured
env var, else the configured default.
**FR-6 Authoring helpers**
- **FR-6.1** `For[C]()`/`Builder` MUST build the tree without repeating `[C]` per
node, returning plain `*Node[C]` so builder and literal construction mix.
- **FR-6.2** `Validate` MUST report: >1 slot child per node, an empty word on a
non-root node, duplicate fixed words among siblings, and a node that neither
runs nor has children; it MUST handle circular slots without looping.
**FR-7 Cancel-on-keypress (`keypress` subpackage)**
- **FR-7.1** A streaming command MUST be able to cancel a context on any keypress.
- **FR-7.2** When stdin is not a terminal, it MUST NOT consume input or cancel
spuriously; it MUST just wait on the context.
- **FR-7.3** It MUST use cbreak mode (single keystroke, echo off, output
post-processing intact) and MUST restore the terminal on return.
## Non-Functional Requirements
- **NFR-1** The library MUST be generic over the client type `C`; no `any`-typed
clients, no reflection on the client.
- **NFR-2** `Builder`, `App`, and `Emit` MUST be optional — a caller MUST be able
to use struct-literal `Node`s with `Shell`/`Dispatch` directly.
- **NFR-3** MUST build for `linux` and `openbsd` (SHOULD also for other BSDs and
macOS). The OpenBSD `readline` termios workaround MUST be transparent and a
no-op elsewhere.
- **NFR-4** Core dependencies MUST be only `chzyer/readline` and `golang.org/x/sys`
— no gRPC, protobuf, or web dependency.
- **NFR-5** Releases MUST be semver tags; the import path is stable across
`v0`/`v1`; a breaking `v2` MUST use the `/v2` suffix.
- **NFR-6** Requires Go 1.25+ (generics with methods on generic types).
- **NFR-7** `make check` (gofmt, vet, golangci-lint, tests) MUST pass before a
commit; new behavior SHOULD ship with a server-free test.
## Architecture
```
argv ──▶ App.Run ──┬─ no args ─▶ Shell.Run ─▶ readline loop ─▶ Dispatch ─▶ Node.Run
│ ▲ ▲
│ Completer ─┘ └─ '?' listener
└─ args ─────────▶ Dispatch ───────────────────────────▶ Node.Run
```
Both modes share everything below `Dispatch`. Completion and `?` read the same
tree the dispatcher walks (FR-1.1) — that is what keeps them from drifting.
## Open Questions
- **Quoted arguments.** Tokenizing is `strings.Fields`, so no argument can
contain a space; a shell-style splitter would be needed for quoted values.
- **Output sink.** `Emit` writes to `os.Stdout`; an injectable `io.Writer` would
make ported commands' output unit-testable. Not needed yet.
- **JSON errors.** One-shot errors print as text on stderr even under `-json`;
`{"error": "..."}` may be worth it once a consumer needs it.