diff --git a/CLAUDE.md b/CLAUDE.md index e3c9729..7f7bc85 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -69,13 +69,12 @@ e2e/ black-box tests driving the real binary as a subprocess 3. Add `cmd/frob.go` — a thin adapter that parses flags and calls `mutate("frob", asJSON, func(env stack.Env, s *stack.State) (*stack.OpResult, error) { return stack.Frobnicate(env, s, arg) })`, self-registering via `init()` → `register(&Command{...})`. Add a `--json` flag. - If `frob` parses any flag beyond `--json`, also add a `frobFlagSet()` constructor - to `cmd/flagsets.go` declaring those flags, set `NewFlagSet: frobFlagSet` in its - `register(&Command{...})`, and list `frob`'s flags in `TestCommandFlagsMatchExpected` - (`cmd/execute_test.go`). `Run` parses the flags either way; this wiring keeps - `st help --json` (and the declared-flags contract in `docs/AGENT.md`) reporting - exactly what `Run` accepts — skip it and `make ci` fails with "update flagsets.go - or this table". + If `frob` parses any flag beyond `--json`, declare those flags once in + `cmd/flagsets.go`: a `frobOpts` struct and `newFrobFlags(o *frobOpts)` (the single + declaration site), plus a no-arg `frobFlagSet()` wrapper. `runFrob` calls + `fs := newFrobFlags(&o)` and reads `o.`; `register` sets + `NewFlagSet: frobFlagSet`. `Run` and `st help --json` then build the flags from + the same constructor, so the declared-flags contract in `docs/AGENT.md` can't drift. 4. If it has interesting CLI output, add a golden test (`cmd/golden_test.go`, regenerate with `go test ./cmd -run Golden -update`). 5. `make ci`. Adding the command shifts the help golden — regenerate it deliberately. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f8d42b2..065e42c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -42,10 +42,10 @@ interface (JSON, exit codes). 3. **Adapter** — `cmd/frob.go`, a thin wrapper that self-registers and calls `mutate("frob", asJSON, func(env stack.Env, s *stack.State) (*stack.OpResult, error) { return stack.Frobnicate(env, s, arg) })`. Add a `--json` flag (see `docs/AGENT.md` — every command speaks JSON). For any - flag beyond `--json`, also add a `frobFlagSet()` to `cmd/flagsets.go`, set - `NewFlagSet: frobFlagSet` in `register()`, and list `frob`'s flags in - `TestCommandFlagsMatchExpected` — this keeps `help --json` reporting exactly - what `Run` accepts (or `make ci` fails with "update flagsets.go or this table"). + flag beyond `--json`, declare it once in `cmd/flagsets.go` (a `frobOpts` struct + + `newFrobFlags(o)` constructor + a no-arg `frobFlagSet()` wrapper); `runFrob` reads + `o.` and `register` sets `NewFlagSet: frobFlagSet`, so `help --json` reports + exactly what `Run` parses, from the same declaration. 4. **Golden output** (optional) — `cmd/golden_test.go`; regenerate with `go test ./cmd -run Golden -update`. 5. `make ci`. Adding a command shifts the `--help` golden — regenerate it diff --git a/cmd/create.go b/cmd/create.go index 0a43a9d..77a1d74 100644 --- a/cmd/create.go +++ b/cmd/create.go @@ -2,7 +2,6 @@ package cmd import ( "errors" - "fmt" "stacked/internal/stack" ) @@ -19,21 +18,12 @@ func init() { } func runCreate(args []string) error { - var asJSON bool - fs := newFlagSet("create", &asJSON) - fs.Usage = func() { - fmt.Fprintln(fs.Output(), usageLine("create")) - fs.PrintDefaults() - } - var message string - fs.StringVar(&message, "m", "", "commit message for the new branch") - fs.StringVar(&message, "message", "", "commit message for the new branch") - var all bool - fs.BoolVar(&all, "a", false, "stage all changes before committing") - fs.BoolVar(&all, "all", false, "stage all changes before committing") + var o createOpts + fs := newCreateFlags(&o) if err := parseArgs(fs, args); err != nil { return err } + asJSON, message, all := o.asJSON, o.message, o.all rest := fs.Args() if len(rest) != 1 { usageUnlessJSON(fs, args) diff --git a/cmd/delete.go b/cmd/delete.go index c198d2f..1f380c8 100644 --- a/cmd/delete.go +++ b/cmd/delete.go @@ -2,7 +2,6 @@ package cmd import ( "errors" - "fmt" "stacked/internal/stack" ) @@ -19,18 +18,12 @@ func init() { } func runDelete(args []string) error { - var asJSON bool - fs := newFlagSet("delete", &asJSON) - fs.Usage = func() { - fmt.Fprintln(fs.Output(), usageLine("delete")) - fs.PrintDefaults() - } - var force bool - fs.BoolVar(&force, "f", false, "force delete the branch even if not fully merged") - fs.BoolVar(&force, "force", false, "force delete the branch even if not fully merged") + var o deleteOpts + fs := newDeleteFlags(&o) if err := parseArgs(fs, args); err != nil { return err } + asJSON, force := o.asJSON, o.force rest := fs.Args() if len(rest) != 1 { usageUnlessJSON(fs, args) diff --git a/cmd/execute_test.go b/cmd/execute_test.go index 570632e..25405f3 100644 --- a/cmd/execute_test.go +++ b/cmd/execute_test.go @@ -6,7 +6,6 @@ import ( "fmt" "io" "os" - "slices" "strings" "testing" @@ -314,41 +313,6 @@ func TestHelpReportsOnlyRealFlags(t *testing.T) { } } -// TestCommandFlagsMatchExpected pins each command's reported flag set to a -// maintained table, so a change to flagsets.go (or a new command) that drifts the -// help-reported flags fails loudly. TestHelpReportsOnlyRealFlags covers the -// over-report direction (a listed flag the command rejects); this pins the set -// itself. Every command also reports --json; completion has none. -func TestCommandFlagsMatchExpected(t *testing.T) { - expected := map[string][]string{ - "create": {"a", "all", "json", "m", "message"}, - "modify": {"a", "all", "commit", "json", "m", "message"}, - "delete": {"f", "force", "json"}, - "init": {"json", "trunk"}, - "track": {"json", "parent"}, - "submit": {"dry-run", "json", "remote"}, - "sync": {"dry-run", "json", "no-delete", "remote"}, - "restack": {"dry-run", "json"}, - "squash": {"json", "m", "message"}, - } - for _, c := range registry { - got := make([]string, 0) - for _, f := range commandFlags(c) { // VisitAll order is lexicographical - got = append(got, f.Name) - } - want, ok := expected[c.Name] - if !ok { - want = []string{"json"} // every command without its own flag set has exactly --json - if c.Name == "completion" { - want = []string{} // completion declares no flags - } - } - if !slices.Equal(got, want) { - t.Errorf("commandFlags(%q) = %v, want %v (update flagsets.go or this table)", c.Name, got, want) - } - } -} - // TestHelpJSONIncludesFlags pins the structured flag list: help --json // reports each declared flag with its type. func TestHelpJSONIncludesFlags(t *testing.T) { diff --git a/cmd/flagsets.go b/cmd/flagsets.go index 2483884..5d6c2a9 100644 --- a/cmd/flagsets.go +++ b/cmd/flagsets.go @@ -1,86 +1,162 @@ package cmd -import "flag" - -// Introspection flag sets for `help --json`. Each declares exactly the flags its -// command's Run parses (Run still builds and binds its own set; these mirror it -// for introspection only). They are gathered here so the full set is reviewable -// at a glance, and TestHelpReportsOnlyRealFlags asserts every flag listed here is -// actually accepted by the command — so help can never advertise a flag the -// command rejects. Commands whose only flag is --json need no entry (help derives -// that from newFlagSet); completion has none. - -func createFlagSet() *flag.FlagSet { - var asJSON bool - fs := newFlagSet("create", &asJSON) - fs.String("m", "", "commit message for the new branch") - fs.String("message", "", "commit message for the new branch") - fs.Bool("a", false, "stage all changes before committing") - fs.Bool("all", false, "stage all changes before committing") +import ( + "flag" + "fmt" +) + +// Each command with flags beyond --json declares them ONCE here: newXxxFlags +// binds a *xxxOpts that its Run reads, and the no-arg xxxFlagSet() wrapper feeds +// the same declaration to help introspection (Command.NewFlagSet). So `help +// --json` reports exactly what Run parses, by construction — no second +// declaration to drift. Aliases (-m/--message, -a/--all, -f/--force) bind to one +// field so command-line last-wins is preserved. (completion has no flags; +// commands whose only flag is --json need no entry — help derives that from +// newFlagSet.) + +// withDefaults gives a command's flag set a usage line plus its flag defaults, +// the help body the five flag-rich commands print on -h. +func withDefaults(fs *flag.FlagSet, name string) *flag.FlagSet { + fs.Usage = func() { + fmt.Fprintln(fs.Output(), usageLine(name)) + fs.PrintDefaults() + } return fs } -func modifyFlagSet() *flag.FlagSet { - var asJSON bool - fs := newFlagSet("modify", &asJSON) - fs.String("m", "", "commit message") - fs.String("message", "", "commit message") - fs.Bool("a", true, "stage all tracked changes before amending/committing") - fs.Bool("all", true, "stage all tracked changes before amending/committing") - fs.Bool("commit", false, "create a new commit instead of amending the tip") - return fs +type createOpts struct { + asJSON bool + message string + all bool } -func deleteFlagSet() *flag.FlagSet { - var asJSON bool - fs := newFlagSet("delete", &asJSON) - fs.Bool("f", false, "force delete the branch even if not fully merged") - fs.Bool("force", false, "force delete the branch even if not fully merged") - return fs +func newCreateFlags(o *createOpts) *flag.FlagSet { + fs := newFlagSet("create", &o.asJSON) + fs.StringVar(&o.message, "m", "", "commit message for the new branch") + fs.StringVar(&o.message, "message", "", "commit message for the new branch") + fs.BoolVar(&o.all, "a", false, "stage all changes before committing") + fs.BoolVar(&o.all, "all", false, "stage all changes before committing") + return withDefaults(fs, "create") } -func initFlagSet() *flag.FlagSet { - var asJSON bool - fs := newFlagSet("init", &asJSON) - fs.String("trunk", "", "name of the trunk branch (default: detected)") - return fs +func createFlagSet() *flag.FlagSet { return newCreateFlags(&createOpts{}) } + +type modifyOpts struct { + asJSON bool + message string + all bool + commit bool } -func trackFlagSet() *flag.FlagSet { - var asJSON bool - fs := newFlagSet("track", &asJSON) - fs.String("parent", "", "parent branch (trunk or a tracked branch)") - return fs +func newModifyFlags(o *modifyOpts) *flag.FlagSet { + fs := newFlagSet("modify", &o.asJSON) + fs.StringVar(&o.message, "m", "", "commit message") + fs.StringVar(&o.message, "message", "", "commit message") + // Default to staging all changes so a bare invocation (and the "amend" alias) + // behaves like "stage all + amend"; only -a=false disables it. + fs.BoolVar(&o.all, "a", true, "stage all tracked changes before amending/committing") + fs.BoolVar(&o.all, "all", true, "stage all tracked changes before amending/committing") + fs.BoolVar(&o.commit, "commit", false, "create a new commit instead of amending the tip") + return withDefaults(fs, "modify") +} + +func modifyFlagSet() *flag.FlagSet { return newModifyFlags(&modifyOpts{}) } + +type deleteOpts struct { + asJSON bool + force bool +} + +func newDeleteFlags(o *deleteOpts) *flag.FlagSet { + fs := newFlagSet("delete", &o.asJSON) + fs.BoolVar(&o.force, "f", false, "force delete the branch even if not fully merged") + fs.BoolVar(&o.force, "force", false, "force delete the branch even if not fully merged") + return withDefaults(fs, "delete") } -func submitFlagSet() *flag.FlagSet { - var asJSON bool - fs := newFlagSet("submit", &asJSON) - fs.String("remote", "origin", "remote to push to") - fs.Bool("dry-run", false, "print what would be pushed without pushing") +func deleteFlagSet() *flag.FlagSet { return newDeleteFlags(&deleteOpts{}) } + +type initOpts struct { + asJSON bool + trunk string +} + +func newInitFlags(o *initOpts) *flag.FlagSet { + fs := newFlagSet("init", &o.asJSON) + fs.StringVar(&o.trunk, "trunk", "", "name of the trunk branch (default: detected)") + return withDefaults(fs, "init") +} + +func initFlagSet() *flag.FlagSet { return newInitFlags(&initOpts{}) } + +type trackOpts struct { + asJSON bool + parent string +} + +func newTrackFlags(o *trackOpts) *flag.FlagSet { + fs := newFlagSet("track", &o.asJSON) + fs.StringVar(&o.parent, "parent", "", "parent branch (trunk or a tracked branch)") + return withDefaults(fs, "track") +} + +func trackFlagSet() *flag.FlagSet { return newTrackFlags(&trackOpts{}) } + +type submitOpts struct { + asJSON bool + remote string + dryRun bool +} + +func newSubmitFlags(o *submitOpts) *flag.FlagSet { + fs := newFlagSet("submit", &o.asJSON) + fs.StringVar(&o.remote, "remote", "origin", "remote to push to") + fs.BoolVar(&o.dryRun, "dry-run", false, "print what would be pushed without pushing") return fs } -func syncFlagSet() *flag.FlagSet { - var asJSON bool - fs := newFlagSet("sync", &asJSON) - fs.Bool("no-delete", false, "do not delete merged branches") - fs.String("remote", "origin", "remote to fetch and fast-forward from") - fs.Bool("dry-run", false, "show what would be pruned/restacked without changing anything") +func submitFlagSet() *flag.FlagSet { return newSubmitFlags(&submitOpts{}) } + +type syncOpts struct { + asJSON bool + noDelete bool + remote string + dryRun bool +} + +func newSyncFlags(o *syncOpts) *flag.FlagSet { + fs := newFlagSet("sync", &o.asJSON) + fs.BoolVar(&o.noDelete, "no-delete", false, "do not delete merged branches") + fs.StringVar(&o.remote, "remote", "origin", "remote to fetch and fast-forward from") + fs.BoolVar(&o.dryRun, "dry-run", false, "show what would be pruned/restacked without changing anything") return fs } -func restackFlagSet() *flag.FlagSet { - var asJSON bool - fs := newFlagSet("restack", &asJSON) - fs.Bool("dry-run", false, "show what would be restacked without changing anything") +func syncFlagSet() *flag.FlagSet { return newSyncFlags(&syncOpts{}) } + +type restackOpts struct { + asJSON bool + dryRun bool +} + +func newRestackFlags(o *restackOpts) *flag.FlagSet { + fs := newFlagSet("restack", &o.asJSON) + fs.BoolVar(&o.dryRun, "dry-run", false, "show what would be restacked without changing anything") return fs } -func squashFlagSet() *flag.FlagSet { - var asJSON bool - fs := newFlagSet("squash", &asJSON) - fs.String("m", "", "commit message for the squashed commit") - fs.String("message", "", "commit message for the squashed commit") +func restackFlagSet() *flag.FlagSet { return newRestackFlags(&restackOpts{}) } + +type squashOpts struct { + asJSON bool + message string +} + +func newSquashFlags(o *squashOpts) *flag.FlagSet { + fs := newFlagSet("squash", &o.asJSON) + fs.StringVar(&o.message, "m", "", "commit message for the squashed commit") + fs.StringVar(&o.message, "message", "", "commit message for the squashed commit") return fs } + +func squashFlagSet() *flag.FlagSet { return newSquashFlags(&squashOpts{}) } diff --git a/cmd/init.go b/cmd/init.go index dca2975..81dd3f1 100644 --- a/cmd/init.go +++ b/cmd/init.go @@ -23,20 +23,15 @@ func init() { // the trunk branch from the --trunk flag, falling back to the remote default // branch, the current branch, and finally "main". func runInit(args []string) error { - var asJSON bool - fs := newFlagSet("init", &asJSON) - var trunk string - fs.StringVar(&trunk, "trunk", "", "name of the trunk branch (default: detected)") - fs.Usage = func() { - fmt.Fprintln(fs.Output(), usageLine("init")) - fs.PrintDefaults() - } + var o initOpts + fs := newInitFlags(&o) if err := parseFlagSet(fs, args); err != nil { return err } if err := rejectArgs("init", fs.Args()); err != nil { return err } + asJSON, trunk := o.asJSON, o.trunk // Ensure we are inside a git repository. if _, err := git.RepoRoot(); err != nil { diff --git a/cmd/modify.go b/cmd/modify.go index a9d46cf..ed9cf5f 100644 --- a/cmd/modify.go +++ b/cmd/modify.go @@ -1,10 +1,6 @@ package cmd -import ( - "fmt" - - "stacked/internal/stack" -) +import "stacked/internal/stack" func init() { register(&Command{ @@ -18,28 +14,15 @@ func init() { } func runModify(args []string) error { - var asJSON bool - fs := newFlagSet("modify", &asJSON) - fs.Usage = func() { - fmt.Fprintln(fs.Output(), usageLine("modify")) - fs.PrintDefaults() - } - var message string - fs.StringVar(&message, "m", "", "commit message") - fs.StringVar(&message, "message", "", "commit message") - // Default to staging all changes so a bare invocation (and the "amend" alias) - // behaves like "stage all + amend". - all := true - fs.BoolVar(&all, "a", true, "stage all tracked changes before amending/committing") - fs.BoolVar(&all, "all", true, "stage all tracked changes before amending/committing") - var commit bool - fs.BoolVar(&commit, "commit", false, "create a new commit instead of amending the tip") + var o modifyOpts + fs := newModifyFlags(&o) if err := parseFlagSet(fs, args); err != nil { return err } if err := rejectArgs("modify", fs.Args()); err != nil { return err } + asJSON, message, all, commit := o.asJSON, o.message, o.all, o.commit return mutate("modify", asJSON, func(env stack.Env, s *stack.State) (*stack.OpResult, error) { return stack.Modify(env, s, message, all, commit) diff --git a/cmd/restack.go b/cmd/restack.go index 30b2ba9..5649eb8 100644 --- a/cmd/restack.go +++ b/cmd/restack.go @@ -14,16 +14,15 @@ func init() { } func runRestack(args []string) error { - var asJSON bool - fs := newFlagSet("restack", &asJSON) - var dryRun bool - fs.BoolVar(&dryRun, "dry-run", false, "show what would be restacked without changing anything") + var o restackOpts + fs := newRestackFlags(&o) if err := parseFlagSet(fs, args); err != nil { return err } if err := rejectArgs("restack", fs.Args()); err != nil { return err } + asJSON, dryRun := o.asJSON, o.dryRun if dryRun { s, err := loadState() diff --git a/cmd/root.go b/cmd/root.go index e3fb642..649295c 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -33,9 +33,10 @@ type Command struct { // Run executes the command with the arguments following the command name. Run func(args []string) error // NewFlagSet, when set, builds a fresh flag set declaring every flag the - // command accepts. Run and the help-introspection path both call it, so a - // command's flags are declared once and `help --json` cannot drift from what - // Run actually parses. Commands whose only flag is --json leave it nil. + // command accepts, for help introspection. Run builds its set from the same + // per-command constructor (cmd/flagsets.go), so a command's flags are declared + // once and `help --json` cannot drift from what Run parses. Commands whose + // only flag is --json leave it nil. NewFlagSet func() *flag.FlagSet } diff --git a/cmd/squash.go b/cmd/squash.go index f5e68eb..7eec9e9 100644 --- a/cmd/squash.go +++ b/cmd/squash.go @@ -13,17 +13,15 @@ func init() { } func runSquash(args []string) error { - var asJSON bool - fs := newFlagSet("squash", &asJSON) - var message string - fs.StringVar(&message, "m", "", "commit message for the squashed commit") - fs.StringVar(&message, "message", "", "commit message for the squashed commit") + var o squashOpts + fs := newSquashFlags(&o) if err := parseArgs(fs, args); err != nil { return err } if err := rejectArgs("squash", fs.Args()); err != nil { return err } + asJSON, message := o.asJSON, o.message return mutate("squash", asJSON, func(env stack.Env, s *stack.State) (*stack.OpResult, error) { return stack.Squash(env, s, message) diff --git a/cmd/submit.go b/cmd/submit.go index 90daa28..db1c6ac 100644 --- a/cmd/submit.go +++ b/cmd/submit.go @@ -39,20 +39,15 @@ type submitResult struct { // PRs on their host afterwards. With --dry-run no branches are pushed and the // planned pushes are printed instead. func runSubmit(args []string) error { - var asJSON bool - fs := newFlagSet("submit", &asJSON) - - var remote string - fs.StringVar(&remote, "remote", "origin", "remote to push to") - - var dryRun bool - fs.BoolVar(&dryRun, "dry-run", false, "print what would be pushed without pushing") + var o submitOpts + fs := newSubmitFlags(&o) if err := parseFlagSet(fs, args); err != nil { return err } if err := rejectArgs("submit", fs.Args()); err != nil { return err } + asJSON, remote, dryRun := o.asJSON, o.remote, o.dryRun state, err := loadState() if err != nil { diff --git a/cmd/sync.go b/cmd/sync.go index 37cbfe8..ca9fbe6 100644 --- a/cmd/sync.go +++ b/cmd/sync.go @@ -17,20 +17,15 @@ func init() { } func runSync(args []string) error { - var asJSON bool - fs := newFlagSet("sync", &asJSON) - var noDelete bool - fs.BoolVar(&noDelete, "no-delete", false, "do not delete merged branches") - remote := "origin" - fs.StringVar(&remote, "remote", "origin", "remote to fetch and fast-forward from") - var dryRun bool - fs.BoolVar(&dryRun, "dry-run", false, "show what would be pruned/restacked without changing anything") + var o syncOpts + fs := newSyncFlags(&o) if err := parseFlagSet(fs, args); err != nil { return err } if err := rejectArgs("sync", fs.Args()); err != nil { return err } + asJSON, noDelete, remote, dryRun := o.asJSON, o.noDelete, o.remote, o.dryRun if dryRun { s, err := loadState() diff --git a/cmd/track.go b/cmd/track.go index 8d4c44f..904d3fc 100644 --- a/cmd/track.go +++ b/cmd/track.go @@ -1,10 +1,6 @@ package cmd -import ( - "fmt" - - "stacked/internal/stack" -) +import "stacked/internal/stack" func init() { register(&Command{ @@ -17,20 +13,15 @@ func init() { } func runTrack(args []string) error { - var asJSON bool - fs := newFlagSet("track", &asJSON) - fs.Usage = func() { - fmt.Fprintln(fs.Output(), usageLine("track")) - fs.PrintDefaults() - } - var parent string - fs.StringVar(&parent, "parent", "", "parent branch (trunk or a tracked branch)") + var o trackOpts + fs := newTrackFlags(&o) if err := parseFlagSet(fs, args); err != nil { return err } if err := rejectArgs("track", fs.Args()); err != nil { return err } + asJSON, parent := o.asJSON, o.parent return mutate("track", asJSON, func(env stack.Env, s *stack.State) (*stack.OpResult, error) { return stack.TrackBranch(env, s, parent)