feat(go): generate a struct per command, and the Parse that fills them - #990
Conversation
ac119a9 to
388490e
Compare
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThe PR adds generated Go command structs and a
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Reviews (9): Last reviewed commit: "feat(go): generate a struct per command,..." | Re-trigger Greptile |
Instruction countsNothing was compared, and so nothing was gated. No series appears on both sides: either the base has no measurements recorded, or the two were measured on different runner classes, which are deliberately not comparable — counts shift between machine types by more than a real regression does. New, nothing to compare against: Only instruction counts gate. Wall clock is shown for context — on identical hardware it moves 4-20% run to run. Measured by tak — instruction-counted CLI benchmarks, stored in this repository's git notes. Shadow comparisonParsing
|
388490e to
324e329
Compare
324e329 to
00cf140
Compare
00cf140 to
5f21d7d
Compare
5f21d7d to
a79f253
Compare
a79f253 to
2b9093b
Compare
2b9093b to
3742a3d
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 3742a3d. Configure here.
The front door. An author calls `Parse(os.Args[1:])` and gets a value with fields
rather than a loop over events; binding, the post-binding rules and the three
tables are unchanged underneath.
cli, err := mycli.Parse(os.Args[1:])
if cli.Run != nil {
fmt.Println(cli.Run.Task, cli.Run.Args)
}
Fields are `string`, `bool` and `[]string`, because that is what a usage spec
knows. A spec says what a value is *called* and never what type it is, so turning
`"8"` into an `int` stays the caller's business — the conversions added earlier in
this stack are there for exactly that, and inferring a type from an argument's
name would be guessing.
Two things mise found that a small fixture could not:
Field names collide within a struct. A command can declare a `--shell` flag
beside a `shell` subcommand, and mise does that with `shell`, `version`,
`command`, `env` and `tool`. The kind disambiguates — `Shell` and `ShellCmd` —
because that says which one it is where `Shell2` would say only that there were
two. The assignment is worked out once and shared by the declarations and by
`Parse`, so the two cannot disagree about where a value goes.
And a subcommand's defaults were being dropped. The fallback assignment started
life as a function taking the root struct, which cannot reach a subcommand's —
that lives in a local of `Parse`. mise's `bootstrap packages import --manager`
defaults to `brew` and was coming back empty. It is written inline now, where the
variables are, and only the keys of commands the words selected are in scope, so
the variable is never nil when its key is.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
3742a3d to
2fedce2
Compare

The front door. An author calls
Parseand gets a value with fields rather than a loop over events — binding, the post-binding rules and the three tables are unchanged underneath.Stacked on #989.
Fields are strings and bools, on purpose
That is what a usage spec knows: it says what a value is called and never what type it is. Turning
"8"into anintstays the caller's business — the conversions in #978 exist for exactly that, and inferring a type from an argument's name would be guessing.Two things mise found that a small fixture could not
Field names collide within a struct. A command can declare a
--shellflag beside ashellsubcommand, and mise does it withshell,version,command,envandtool. The kind disambiguates —ShellandShellCmd— because that says which one it is whereShell2would say only that there were two. The assignment is worked out once and shared by the declarations and byParse, so the two cannot disagree about where a value goes.A subcommand's defaults were dropped. The fallback assignment started as a function taking the root struct, which cannot reach a subcommand's — that lives in a local of
Parse. mise'sbootstrap packages import --managerdefaults tobrewand came back empty. It is inline now, where the variables are; only the keys of commands the words selected are in scope, so the variable is never nil when its key is.Both are tested against mise's real command lines, including the
[ARGS]… [-- ARGS_LAST]…split through the generated structs.🤖 Generated with Claude Code
Note
Low Risk
Changes are documentation, a thin env helper, and shadow integration tests; generated
Parsebehavior is exercised on real mise argv but does not alter the core parser’s hot path.Overview
Go CLI authors are steered toward generated
Parseinstead of hand-rollingargv.Newevent loops.go/README.mdnow showsmycli.Parse(os.Args[1:]), nested command structs with nil pointers for unselected branches, and notes that binding, post-rules, env/default fill, and validation all happen insideParsewithstring/bool/[]stringfields.argv.LookupEnvis added as the process-environment hook that generatedParsepasses intoFill, keeping tests injectable while real CLIs reados.LookupEnv.The mise shadow package gains integration tests for the generated front door: struct fill (
use -g node@20),--arg splitting ontasks run, subcommand defaults (bootstrap packages import→brew), and choice validation (--log-level chatty→invalid_choice).Reviewed by Cursor Bugbot for commit 2fedce2. Bugbot is set up for automated code reviews on this repo. Configure here.