feat(help): list --help and --version, which every page answers - #914
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 |
960c38c to
bcf53c7
Compare
Greptile SummaryThe PR makes generated help list parser-supplied help and version flags without adding them to emitted specs.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Reviews (6): Last reviewed commit: "fix(help): a spec that turns help off sh..." | 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
|
bcf53c7 to
33a5866
Compare
33a5866 to
61cff92
Compare
|
All three addressed, and the first two fall out of the claim model rebuilt in #913 below. Hidden and negated claims (greptile, Bugbot). Short-only entries showed the wrong name (Bugbot) — real, and reproduced: The declared name is judged against the forms the entry shows, so a short-only entry called Ungrouped section order (Bugbot). Right, and the snapshot that accepted it was mine. Verification
Gate green over mise's 211 commands, workspace suite green, clippy clean. AI-assisted — Tool: Claude Code; model: anthropic/claude-opus-5; version: unavailable. |
61cff92 to
4f2822a
Compare
The last of the four help differences. Both work and neither appeared on any page: a reader looking for how to ask for help had to already know. This reverses a rule these two used to follow — that a page lists exactly what its spec declares — and the reversal is only half. The page names them; the **spec still does not**, because the parser supplies them and a spec claiming otherwise would have every reader inventing a flag its CLI never wrote. Help is written for people, the spec has its own readers, and they are not the same readers. A test asserts both halves. `--version` only where the parser accepts one: on a command whose table says so, which is the root of a CLI that declared a version. Offering a flag that would be refused is worse than staying quiet. Whether a spelling is free is asked of the same set every other decision on a page uses, so a `--help` claimed by a hidden declaration or by a negation is claimed here too — the parser yields to both, and a page that said otherwise would describe an action that typing it does not perform. The entry left over is named after the form it shows: a short-only one called `help` reads as a renamed flag and printed `help: -h`. Last in the command's own section, where clap has them, carrying no `help_heading` — and in usage-lib inserted *first* among the groups rather than pushed, because the unheaded group sorts to the front there and is emitted at the front here. A CLI that heads every one of its own flags would otherwise get `Flags:` after the headed sections in one renderer and before them in the other. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…thing else Three from review, all about `--help` and `--version` being offered where something else would bind the word. usage-lib rebuilt the claim set from the *visible* inherited list, so a hidden global that declares `--help` left the spelling looking free — and `hide` keeps a flag off the page, not out of the parse. The set the inherited walk already built now travels with its result, so both decisions are made against the same thing. It also spelled a negation with four dashes, for the reason the commit below fixes: usage-lib stores `negate="--no-colour"` with the dashes and usage-argv without, so prefixing produced `----no-colour` and matched nothing. And the supplied entries lose to a *negation* as well as to a long, which is the one place the ordering goes the other way: `long_flag` asks `find_negation` before it offers `--version`, so a CLI whose `--quiet` negates to `--version` keeps that word — even though a plain long would have beaten the same negation. Found by greptile and Cursor Bugbot. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
4f2822a to
93919e0
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 93919e0. Configure here.
|
Triaged all three. Two were already fixed on head; the third turned out to be a real regression, though not on this branch — chasing it is what found it, so thank you. Short-only flags show wrong name — fixed in #916. A short-only entry is named after the form it shows ( Ungrouped section order mismatch — fixed on head, and the code now says so where it happens: the supplied entries are Hidden claims still offered — this one was live, and worse than reported. Fixed on this branch in The part worth flagging: that commit was on this branch when you reviewed it, and every branch above this one had reverted it — an earlier restack had resolved a conflict in AI-assisted — Tool: Claude Code; model: anthropic/claude-opus-5; version: unavailable. |
`disable_help` makes `is_help_arg` refuse `--help`, `-h` and `-?` outright, and the supplied entries were appended without consulting it — so the page advertised an action its own parser would not perform. The same lie this change set out to avoid for a claimed or hidden spelling, with the claim made by the spec itself rather than by a flag. `--version` stays: nothing disabled that. No twin change in usage-argv, and no divergence either: `disable_help` is a KDL word with no equivalent in the argv tables, so no spec that crate can hold carries one. Recorded here rather than silently, since the two renderers being byte-identical is the invariant this area runs on. Reported by Bugbot on #914.
|
Second pass on this PR's review. Most of the newer comments re-report findings against pre-rebase commits — the negate double-dashes ( One was new and correct: help listed when disabled. No twin change in usage-argv, and no divergence either: AI-assisted — Tool: Claude Code; model: anthropic/claude-opus-5; version: unavailable. |

PR 3, the last of the four help differences. Both flags worked and neither appeared on any page: a reader looking for how to ask for help had to already know.
The rule this reverses, and the half it keeps
The old rule was that a page lists exactly what its spec declares. Per your call, the page now names these two — but the spec still does not, because the parser supplies them and a spec claiming otherwise would have every reader inventing a flag its CLI never wrote.
Help is written for people; the spec has its own readers, and they are not the same readers. A test asserts both halves, which is the honest way to record a reversal.
Where they are, and are not
--versiononly where the parser accepts one — a command whose table says so, which is the root of a CLI that declared a version. A subcommand's page does not offer it, becauseex other --versionis refused. Offering a flag that would be refused is worse than staying quiet.-Vtaken by--verboseleaves--versionlisted alone; the parser yields to a declaration, and the page has to say what will actually happen rather than what would be tidy.help_heading— so a CLI that groups its flags gets them at the end of the ungrouped list rather than inside somebody else's section.Verification
--versionoffered where the parser refuses itGate green over mise's 211 commands. Seven expected pages in
lib/tests/parse.rsand a handful of insta snapshots gain the line — each the intended change, checked one at a time rather than blanket-accepted.That completes the four differences from the plan.
communique generate -his now byte-for-byte clap's output apart from the wordFlags:, plus aGlobal flags:section clap has no equivalent for — and short help, which had no column at all, has one.AI-assisted — Tool: Claude Code; model: anthropic/claude-opus-5; version: unavailable.
Note
Medium Risk
User-visible help text changes across all commands, but behavior is tightly gated by claimed spellings and covered by conformance, insta, and parse snapshot tests.
Overview
Help output now shows
-h, --helpand-V, --version(when applicable) at the end of each command’s unheaded flags section, matching clap-style discoverability. The spec/KDL still does not declare these flags—the parser supplies them—so emitted specs stay unchanged while rendered pages name what users can actually type.Both
usage-argv(supplied/supplied_entrieswired throughown_and_global) andlibdocs rendering (supplied_flags, withinherited_flagsreturning the ancestor “taken” spellings) pick long/short/both variants from what is already claimed, including hidden flags and negations.--versionappears only on the root when a version exists; subcommands and unversioned CLIs omit it. Mise’sdisable_helpsuppresses the supplied help line on KDL-rendered pages.Conformance and snapshot tests were flipped from “must not list” to “listed but not declared,” with new cases for claimed spellings, hidden help, and negated
--version.Reviewed by Cursor Bugbot for commit c6c0d41. Bugbot is set up for automated code reviews on this repo. Configure here.