fix(help): a declared name is not a short form, and blank help is no help - #916
Conversation
Both against the column that just landed, and both found by review.
**A declared name is not a short form.** A flag may carry a name the forms do not imply —
`jobs: -j --parallel` — and the split treated that whole prefix as the thing to put a
comma after. The joined string is already wider than the column being padded to, so the
padding did nothing and the space vanished:
jobs: -j,--parallel
Only a bare `-x` goes in the short column now; anything else keeps the shape it had.
**A description of only spaces is no description.** `usage-argv` filters a blank one
wherever it reads one; the template asked only whether the string was *there*, so
`help=" "` bought a column of padding and a line of trailing spaces on one side and
nothing on the other — two renderings of one spec, which is exactly what the gate exists
to prevent and could not see, because mise's spec has no such flag. Normalised where the
docs model is built, so every renderer downstream gets the same answer.
Found by Cursor Bugbot and greptile.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan includes up to 4 reviews per rolling hour; 0 remain after this review. 📝 WalkthroughWalkthroughHelp formatting now preserves nonstandard option prefixes and removes whitespace-only descriptions. New tests cover rendered output in short and long modes, declared option names, and parsed values. ChangesHelp rendering corrections
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This localized documentation, help-output, and conformance-test change has no actionable merge-blocking risk remaining and is merge-ready after normal checks and review. Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 fixes help-column formatting for flags whose declared names differ from their forms and normalizes whitespace-only flag and argument descriptions.
Confidence Score: 5/5The PR appears safe to merge with no actionable correctness or security issues identified. The two help renderers apply equivalent column rules, and blank-help normalization preserves meaningful long and markdown help fallbacks while removing only whitespace-only descriptions. Important Files Changed
Reviews (1): Last reviewed commit: "fix(help): two the flag column got wrong" | 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
|
Two findings against the flag column from #912, both from review.
A declared name is not a short form
A flag may carry a name the forms do not imply —
jobs: -j --parallel— and the split treated that whole prefix as the thing to put a comma after. The joined string is already wider than the column being padded to, so the padding did nothing and the space vanished:Only a bare
-xgoes in the short column now; anything else keeps the shape it had:A description of only spaces is no description
usage-argvfilters a blank description wherever it reads one. The template asked only whether the string was there, and Tera counts" "as present — sohelp=" "bought a column of padding and a line of trailing spaces on one side and nothing on the other.Two renderings of one spec, which is exactly what the gate exists to prevent and could not see: mise's spec has no such flag. Normalised where the docs model is built, so every renderer downstream gets one answer.
Verification
The second survived at first: my test found
--blankon the usage line, which contains the flag and has no padding to get wrong. Scoped to the flags section, it fails properly. Same trap caught me on #913's tests — worth remembering that a page names its flags twice.Found by Cursor Bugbot and greptile.
AI-assisted — Tool: Claude Code; model: anthropic/claude-opus-5; version: unavailable.