golang-cli/docs/PROPOSAL.md

<!--
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
  tree.go        Node[C], Walk, matchFixedChild, findSlotChild, expandPaths, Candidates
  tree_test.go   table tests over a fixture tree (no client needed; Walk ignores Dynamic)
  complete.go    completer[C], questionListener[C], SplitTokens, splitForCompletion
  shell.go       Shell[C] (config + Run), Dispatch, showHelpAt, unknownCommandError, ErrQuit
  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
  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.