Add --man man page generator from a shared usage model#400
Conversation
📝 WalkthroughWalkthroughStructured usage data now drives console help and man-page output. ChangesUsage help and man-page output
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
options.ixx (1)
471-471: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBrace placement violates repository C++ style rule
return {should place{on the next line per project style.As per coding guidelines: "Always add a new line before {".
Also applies to: 494-494
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@options.ixx` at line 471, The brace placement in the return statements violates the repository C++ style rule, so update the affected `return` expressions in `options.ixx` to put the opening `{` on its own next line instead of on the same line as `return`. Apply the same formatting fix to both occurrences mentioned in the diff, keeping the surrounding logic unchanged.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@options.ixx`:
- Line 530: The usage text for the option entry in options.ixx is missing the
value marker for a parameterized flag. Update the help string for the
--mediatek-leadout-retries entry so it includes =VALUE, matching the parser’s
expected integer argument and keeping --help and --help-man consistent with the
behavior of the option definition.
---
Nitpick comments:
In `@options.ixx`:
- Line 471: The brace placement in the return statements violates the repository
C++ style rule, so update the affected `return` expressions in `options.ixx` to
put the opening `{` on its own next line instead of on the same line as
`return`. Apply the same formatting fix to both occurrences mentioned in the
diff, keeping the surrounding logic unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| private: | ||
| static std::vector<UsageCommand> usageCommands() | ||
| { | ||
| return { |
There was a problem hiding this comment.
Regarding the return { brace placement (also at line 494): clang-format-18 — the formatter the CI clang-format Check runs — produces and accepts this layout, and reformats the brace back onto this line if it is moved to its own line, so changing it would fail the format gate. The repository's "Always add a new line before {" rule and the .clang-format Allman wrapping apply to statement and declaration blocks, not to braced-init-list expressions like return {...}. I've left it as the formatter emits it to keep the check green, but I'm happy to adjust if you'd prefer a different layout.
Generated with Claude Opus 4.8.
gmipf
force-pushed
the
man-page-generator
branch
from
f41d599 to
7cdff2d
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@options.ixx`:
- Around line 187-188: The option parser in the command-line handling logic
recognizes only the hidden flag currently checked in the key comparison, so the
intended man-page switch is not accepted. Update the parsing branch that sets
the man-page mode to accept the documented hidden flag name used by the PR
objective, and keep the existing unknown-option handling unchanged so the parser
routes the correct flag into the same man-page path.
- Around line 492-575: The help text in usageGroups() is formatting “default”
values from mutable parsed members such as retries, plextor_leadin_retries,
mediatek_leadout_retries, audio_silence_threshold, skip_fill,
cdr_error_threshold, and scsi_timeout, which lets argv overrides leak into
--help and --man output. Update usageGroups() to source these descriptions from
invariant built-in default constants or static constexpr defaults instead of
runtime state, so the rendered docs always reflect the true compile-time
defaults regardless of parsed options.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b3e15204-23ad-45c1-a810-1addabb7eb84
📒 Files selected for processing (2)
main.ccoptions.ixx
🚧 Files skipped from review as they are similar to previous changes (1)
- main.cc
gmipf
changed the title
Refactor printUsage() to build its output from a small data model (UsageCommand/UsageGroup/UsageOption) instead of hard-coded LOG lines. Add printUsageMan(), reachable via a hidden --man flag, which renders the same model as a roff man page (standard man(7) macros, renders warning-free under groff and mandoc -T lint) so packagers can generate redumper.1 without maintaining it separately. The shared model sources value-option defaults from compile-time constants, so --help and --man always print the built-in default. This keeps plain --help byte-identical apart from correcting the --mediatek-leadout-retries usage text to show =VALUE, and additionally fixes a pre-existing case where an override such as --retries=9 --help printed the override (9) as the "default" instead of 0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
gmipf
force-pushed
the
man-page-generator
branch
from
7cdff2d to
32b656d
Compare
superg
left a comment
superg
left a comment
There was a problem hiding this comment.
I'm sorry I cannot accept this for the following reasons:
- man page is different from the built-in help and it shouldn't be a part of program implementation, it's perfectly fine not to have a man page for a utility
- it's slop and it changed a lot of things human would not change
2 participants
This adds a
--manoption that emits a roff man page, generated from thesame in-code usage definition that
--helpalready prints. The goal is to givepackagers a man page that never drifts from the actual command/option set,
without maintaining a separate hand-written
.1downstream.Today
printUsage()hard-codes each command and option as aLOG(...)line,and any man page a distribution ships has to be hand-maintained in parallel, so
it drifts whenever an option changes. This refactors the usage text into a small
data model and feeds both outputs from it.
What it does:
Adds
UsageOption/UsageGroup/UsageCommandplususageCommands()andusageGroups(), holding the commands, option groups and descriptions. Numericoption defaults are interpolated with
std::formatfrom compile-time constants(a small set of
*_defaultvalues shared with the constructor), so the printeddefault always reflects the built-in value.
Rewrites
printUsage()to iterate that model. The--helpoutput isunchanged except for the corrections noted below (column widths are computed
from the longest entry).
Adds
printUsageMan(), which iterates the same model and writes a man page,reachable through a new hidden
--manflag. Packagers can then generatethe page at build time:
While here, this corrects two pre-existing issues, both of which the old
hard-coded
printUsage()already had:--mediatek-leadout-retriesbinds to an integer argument in the parser, butits usage text omitted the
=VALUEmarker. The output now includes it. Thisis the only change to plain
redumper --help.so an override combined with help — e.g.
redumper --retries=9 --help—printed
9as the "default" instead of0. Sourcing the defaults fromcompile-time constants fixes this for both
--helpand--man.So plain
redumper --helpstays byte-for-byte identical apart from the one=VALUEline; the only other behavioral change is that an override no longerleaks into the displayed default.
The output uses only the standard man(7) macros (
.TH/.SH/.SS/.TP/.B) andescapes roff metacharacters, so it renders identically under groff (Linux) and
mandoc (macOS and the BSDs). As redumper also ships a CLI on macOS, the same
generated page is reusable there (e.g. for a Homebrew formula) and on the BSDs,
not just on Linux distributions. The
.THdate is intentionally left blank forthe packager to stamp at build time (e.g. from
SOURCE_DATE_EPOCH), keeping thegenerator output reproducible.
Verification (ubuntu-24.04, clang-18 + libc++-18, matching the Linux CI):
cmake --build+ctest: all 358 tests pass.clang-format-18 --dry-run -Werror: clean.diffof plain--helpbefore and after: only the one corrected--mediatek-leadout-retriesline differs.redumper --retries=9 --helpnow prints(default: 0)instead of(default: 9).groff -mandoc -ww: no warnings.mandoc -T lint(the parser used on macOS and the BSDs): no errors; the onlynote is the intentionally blank
.THdate.The change is standard C++20 with no platform-specific code, so the Windows and
macOS build jobs should be unaffected, but I've only been able to run the Linux
toolchain locally.
A couple of things I'm happy to adjust in review:
prefer to leave generation to downstream packagers.
NAMEline wording (currently taken from the README:"low-level byte-perfect CD/DVD/HD-DVD/Blu-ray disc dumper").
Prepared with AI assistance (Claude Opus 4.8) and reviewed before submission.