feat(go): render --help too, matching usage-lib on all 211 of mise's long pages - #975
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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Greptile SummaryThe PR adds Go rendering for wide
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the previously reported unused usage override has been removed, and the related long-help metadata is consistently preserved by both producers. Important Files Changed
Reviews (6): Last reviewed commit: "fix(go): a description ending in a break..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
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
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 5080454. Configure here.
…s long pages The same content as `-h` through a wider layout: help wrapped into a column, the long form of each description preferred over the short, each annotation on its own line, and an entry whose help the author already broke laid out as a block underneath rather than squeezed beside its name. **All 211 long pages match usage-lib byte for byte**, so all three renderings now do — the usage line, `-h`, and `--help`. 209 matched on the first run. The two that did not were both about where whitespace goes, and neither is guessable: The lowering's top-level `usage` is the *computed* line rather than a declared override, so preferring it printed mise's `[OPTIONS] [TASK] [COMMAND]` where the reference prints the line it works out itself. usage-argv has a `spec.usage` that does mean an override; the JSON field of the same name does not. And a description that *begins* with an empty line gets four spaces on that line, while blank lines inside the same description get none. That reads as inconsistent until you see it as a template indenting where it starts writing rather than trimming each line. Indenting every blank line instead differs on all 211 pages, which is how it was settled — by measuring, not by reading the template. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…eads Two review findings, both about the long page's own PR. `HelpSpec.Usage` was left behind when the branch reading it went. It was documented as replacing the computed usage line, mapped from the lowering's top-level `usage` — which is the *computed* line rather than an override — and read by nothing. An exported field that does nothing and says otherwise is worse than no field, so it is gone. Examples now fall back to the root's where a command declares none, which is what `page_examples` does and the same fallback `BeforeHelp` and `AfterHelp` already had. A CLI writing examples once at the top means them to appear. mise declares no root examples, so the 211-page parity suite could not see this in either direction — the third time in this stack that the largest fixture available cannot exercise a rule. Checked against the reference's own rule and pinned with a unit test instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The lowering learned `before_long_help` and `after_long_help` when the long page landed; the generator did not. mise writes its examples as `after_long_help` on 115 of its commands, so a generated CLI printed a `--help` with every example section missing — and the parity test could not see it, because it builds its tables by lowering rather than by generating. The producer comparison from the commit below caught it, which is what that test is for. The root's header had the same gap: `about_long` had nowhere to go, so it was written into `About` and would have appeared on the *short* page, which usage-lib leaves empty when a spec writes only the long one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ere compared Two gaps in this PR, both found by review. `Example.Help` is the line the long page prints above the command, introducing the invocation rather than commenting on it. The lowering filled it and the generator did not, so a generated CLI printed the command with nothing to say why. mise cannot show this either way — its examples are `after_long_help` text rather than `example` nodes — so the producer comparison could not see it and this gets a test of its own. And the long-page parity test only failed on a difference it found: a page the reference did not have was skipped, with no floor on how many were compared. An empty oracle would have passed it while logging that nothing matched. It now has the short page's floor, and a missing reference page is a difference. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A single differing line is rarely the story. A page that gained or lost a line reads as a difference at the next line with content, and the cause is above it — so the report shows a window either side, the two line counts, and which `usage` binary and Go version produced it. Written while chasing a failure that reproduces in CI and not locally, where the one line the report showed said only that ours was blank. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both sides of the reference stopped writing that blank in #970 — clap's `long_about` often ends with one, a `///` block whose last line is empty or an examples section written with a trailing newline, and it reaches the spec verbatim. The blank line under a description belongs to the renderer, so one already in the text was a second one: a stray blank under the about, and another in the middle of the `Commands:` list. The same three places usage-argv trims: the about on the short page, the about on the long page, and each entry of the long page's command list. Found by CI failing where local runs passed — CI tests the branch merged with main, which has the fix, and mise exercises it through `plugins ls-remote`. The rule gets a unit test as well: the parity suite says a page differs, not which rule broke. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The CI tests the branch merged with main, and main has #970: a description ending in a break no longer gets a blank line written under it, on either side of the reference. This stack predates it, so the Go long page still wrote one — a stray blank under the about, and another in the middle of The whole stack is rebased onto main now, and the Go renderer trims in the three places usage-argv trims: the about on the short page, the about on the long page, and each entry of the long page's command list. The rule also gets a unit test, since the parity suite says a page differs rather than which rule broke. One other change came out of chasing it: a page difference now reports a window either side of the line, both line counts, and which This comment was generated by Claude Code. |
`argv.Render` turns a binding failure into what a CLI prints to stderr: ``` error: unknown flag `--wat` Usage: ex run [-f --force] For more information, try `--help`. ``` Stacked on #975. ## The one part with no reference to match Every other renderer in this module is measured against usage-lib byte for byte. This one cannot be, and saying so plainly is the point: usage-lib prints a one-line message inside miette's frame, usage-argv renders through miette too with the offending token underlined, and neither travels — miette is a Rust library, and a Go CLI drawing the same ASCII art would be imitating a diagnostic format rather than sharing one. So it is judged on a different standard — does the message say **what** went wrong, **where**, and **what to try** — and tested by asserting those three rather than by comparing bytes. ## What the tests actually check - **Every code renders something specific.** A failure falling through to "could not be parsed" tells a user nothing, so the test is that none of the eleven does. - **The usage line names the command in scope**, not the program. On a nested command that is the difference between an actionable message and a confusing one. - **Help and version render nothing.** They arrive as errors because a parse that stops to print a page has produced no value, but a caller rendering one of them as a failure has made two mistakes at once. `missing_flag_value` says the likeliest cause out loud, because the rule behind it is not guessable: a flag-like token after a flag is refused as its value, and the attached form (`--flag=-x`) is how to force it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > User-facing CLI error text and terminal escaping only; parsing semantics unchanged aside from richer error metadata on missing flag values. > > **Overview** > Adds **`argv.Render`** to format binding failures like clap: an `error:` line, a **Usage** line for the command the user was in, and a pointer to `--help`. Help/version codes render nothing. > > **Error quality and safety:** Post-binding and parser failures now carry **`Spelling`** / **`OtherSpelling`** on `Error` and `Meta` (emitted from spec/Go gen) so messages quote flags as typed (`--workers` vs `-j`, conflict pairs) instead of guessing from names. Missing detached flag values record the token form that bound. **`safe()`** escapes control characters in user tokens for both `Render` and **`Error()`**. > > README documents the errors story; **`render_test.go`** asserts content and spelling behavior rather than byte-matching Rust diagnostics. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 2e3c1b5. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 participant
The same content as
-hthrough a wider layout: help wrapped into a column, the long form of each description preferred over the short, each annotation on its own line, and an entry whose help the author already broke laid out as a block underneath rather than squeezed beside its name.All 211 long pages match usage-lib byte for byte, so all three renderings now do:
Stacked on #974.
209 matched first run. The two that did not
Both about where whitespace goes, and neither guessable from the code:
The lowering's top-level
usageis the computed line, not a declared override. Preferring it printed mise's[OPTIONS] [TASK] [COMMAND]where the reference prints the line it works out itself. usage-argv has aspec.usagethat really does mean an override; the JSON field of the same name does not, and they are easy to conflate.A description that begins with an empty line gets four spaces on that line, while blank lines inside the same description get none. That reads as inconsistent until you see it as a template indenting where it starts writing rather than trimming each line. I first tried indenting every blank line — that differs on all 211 pages, which is how it got settled: by measuring, not by reading the template.
Why this is worth having as parity rather than snapshots
A snapshot would have frozen whatever I produced, including both of those mistakes. The oracle catches them because it is a second implementation of the same intent, written from a spec through a template rather than from static tables.
🤖 Generated with Claude Code
Note
Low Risk
Help text rendering and test-only parity checks; no parsing, binding, or security-sensitive paths. Regressions would show up as mismatched CLI help output rather than runtime failures.
Overview
Adds
argv.LongHelpfor the full--helplayout: 80-column wrapping, long descriptions and bracket text (BeforeLongHelp/AfterLongHelp,LongAbout) preferred over short forms, per-line annotations, and block layout when help already contains line breaks. Subcommand lists print descriptions indented under each name; examples can showExample.Helpabove the$ …line.-h(ShortHelp) picks up the same trimming and root example fallback as long pages (via sharedpageExamples,trimEnd,sortLines), and help metadata gains the long-page fields onHelp/HelpSpec.Conformance now asserts all 211 long pages match usage-lib byte-for-byte (alongside existing short-page checks), with a richer
firstDifffor failures; unit tests cover example fallback and trailing whitespace in descriptions. Docs drop “long page” from the missing list; producer tests also compareHelpSpecbetween lowered and generated mise tables.Reviewed by Cursor Bugbot for commit c86e060. Bugbot is set up for automated code reviews on this repo. Configure here.