feat(argv): answer --help and -h - #870
Conversation
|
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 SummaryThis PR adds parser-level handling for
Confidence Score: 4/5The PR is not yet safe to merge because leading help tokens still bypass forwarding for commands configured with The parser returns synthetic help events before reaching the unknown-flag positional fallback, so forwarding wrappers consume a leading Files Needing Attention: argv/src/lib.rs Important Files Changed
Reviews (6): Last reviewed commit: "test(argv): pin that a wrapper still for..." | Re-trigger Greptile |
Instruction counts
No instruction-count regression above 1%. 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
|
|
The mechanism is right, the behavior is intended, and it is now pinned by a test (df327ba). A wrapper has two ways to say "hand this on", and help interception sits before both rather than around them:
Before either takes effect,
AI-assisted — Tool: Claude Code; model: anthropic/claude-opus-5; version: unavailable. |
|
Note on the force-push: this branch and #872 had been rebuilt on a parallel copy of #866's commits rather than on #866 itself, so as pushed they were missing AI-assisted — Tool: Claude Code; model: anthropic/claude-opus-5; version: unavailable. |
The parser recognises both itself rather than a CLI declaring them, which is the only arrangement that does not change what help *says*: no spec declares `--help`, so a table entry for one would put it in the rendered page and in the emitted KDL, and the page would then disagree with the spec it came from. Recognised *after* the command's own flags, so a CLI that declares its own `--help` or `-h` keeps it and binds it like any other flag. A request comes back as `Error::Help`, carrying the command it was asked about — `mise config --help` is a question about `config`, and the parser is what knows how far the words reached. Not printed: a library that writes to stdout on its own is one an adopter cannot embed, and a parse that stops for help has produced no value, which is the shape every caller already handles. clap does the same thing for the same reason. `parse()`, the convenience that reads the process's own arguments, is the one place that prints and exits. `-h` renders the short form and `--help` the long one, as clap has them. The first attempt put the two flags in every command's parse table, which flatten then spliced twice — a command that flattened another had two `-h` entries and tripped the duplicate-key check. Recognising them in the parser instead removes the table entries, the metadata mismatch and the collision at once. Costs 111 instructions of 29,961, and only on a flag that matched nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Supplying `--help` narrows what a forwarding command can be handed, so the two mechanisms that make forwarding possible are worth stating: after a typed `--`, and after an `automatic` argument takes its first value, flag interpretation has stopped and `--help` is a value like any other. Before either takes effect, `--help` is still a question about the wrapper — `mise run --help` asks about `run` — which is the line between the two cases. Raised by greptile on the mechanism; the tests are the answer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The parser recognises both itself rather than a CLI declaring them, which is the only
arrangement that does not change what help says: no spec declares
--help, so a table entryfor one would put it in the rendered page and in the emitted KDL, and the page would then
disagree with the spec it came from.
Recognised after the command's own flags, so a CLI that declares its own
--helpor-hkeeps it and binds it like any other flag.
A request comes back as
Error::Help, carrying the command it was asked about —mise config --helpis a question aboutconfig, and the parser is what knows how far the words reached.Not printed: a library that writes to stdout on its own is one an adopter cannot embed, and a
parse that stops for help has produced no value, which is the shape every caller already
handles. clap does the same thing for the same reason.
parse(), the convenience that reads theprocess's own arguments, is the one place that prints and exits.
-hrenders the short form and--helpthe long one, as clap has them.The first attempt put the two flags in every command's parse table, which flatten then spliced
twice — a command that flattened another had two
-hentries and tripped the duplicate-keycheck. Recognising them in the parser instead removes the table entries, the metadata mismatch
and the collision at once.
Costs 111 instructions of 29,961, and only on a flag that matched nothing.
Co-Authored-By: Claude Opus 5 noreply@anthropic.com
Stack created with GitHub Stacks CLI • Give Feedback 💬
Note
Low Risk
User-facing behavior change is limited to new help handling on the parse path; embedders must handle
Error::Helpif they useparse_frominstead ofparse().Overview
Built-in help flags are recognized in the parser (not in per-command tables), so help output and emitted KDL stay aligned with specs that never declare
--help.--helpand-hare resolved only after a command’s own flags, so CLIs that define their ownhelp/hstill bind them normally.Parsing stops with
Error::Help { cmd, long }, targeting the command in scope (e.g.ex ls --help→ls).usage_argv::help::find/rendermap thatCommandto metadata and short vs long help text.parse_fromreturns the error for embedders;parse()renders to stdout and exits 0.usage-deriveintercepts help flag events beforeapplyand special-casesError::Helpinparse(). PLAN.md marks help rendering as wired; the dedicatedhelpsubcommand remains open. Conformance tests cover long/short pages, scoped help, spec omission, custom help flags, help before validation, and wrapper forwarding after--/automaticdouble-dash.Reviewed by Cursor Bugbot for commit 12f2321. Bugbot is set up for automated code reviews on this repo. Configure here.
Where help is recognised, and why it matters
In the parser, not in any command's table. That is the only arrangement that does not change
what help says: no spec declares
--help, so a table entry for one would put it in therendered page and in the emitted KDL, and the page would then disagree with the spec it came
from. The 211-page parity is unchanged by this PR, which is the check on that claim.
Recognised after a command's own flags, so a CLI that declares its own
--helpor-hkeepsit and binds it like any other flag.
How it comes back
Error::Help { cmd, long }— carrying the command it was asked about, becausemise config --helpis a question aboutconfigand the parser is what knows how far the words reached.Not printed. A library that writes to stdout on its own is one an adopter cannot embed, and a
parse that stops for help has produced no value — the shape every caller already handles. clap
does the same thing for the same reason.
parse(), the convenience that reads the process's ownarguments, is the one place that prints and exits.
-hrenders the short form,--helpthe long one, as clap has them.The design that did not work
The first attempt put both flags in every command's parse table.
flattenthen spliced themtwice — a command that flattens another had two
-hentries and tripped the duplicate-key check— and the metadata had to gain an exemption from "every flag in the table needs metadata" to keep
them out of the page. Moving the knowledge into the parser removed the table entries, the
metadata mismatch, and the collision together.
Cost
111 instructions of 29,961, and only on a flag that matched nothing — the check sits after the
lookup that failed. 850 ns against clap's 435 µs.
Verification
Six tests: the two forms differ and ask for different pages; help is about the command the words
reached rather than the root; the flags appear in neither the page nor the emitted spec; a CLI
declaring its own keeps it; help answers even when a required argument is missing (which is when
someone needs it most); and the fixture still parses normally when nobody asks.
Three mutation checks — dropping the long form, dropping the short one, and checking help
before a command's own flags, which is what would silently steal a declared
--help.Next
The
helpsubcommand —mise help config ls— which every CLI with subcommands should have, andwhich the rendered page already advertises.
AI-assisted — Tool: Claude Code; model: anthropic/claude-opus-5; version: unavailable.