fix(cli): put id argument first in report/release usage lines - #3135
fix(cli): put id argument first in report/release usage lines#3135lunetics wants to merge 1 commit into
Conversation
herdr pane report-agent, report-agent-session, release-agent, report-metadata, and herdr workspace report-metadata all take their id as the first positional argument in their hand-written parsers, but clap's default usage synopsis renders required options before positionals, so --help shows the id at the end. Following that usage line's argument order (options first, id last) produces a confusing "unknown option" error, because the parser unconditionally treats the first token as the id. Each command's usage line is now overridden to show the id first, matching an existing convention already used for several `agent` subcommands (read/prompt/rename/wait/attach) in this same file. A regression test checks all five usage lines.
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI 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 |
|
Hi @lunetics, thanks for your interest in contributing. Herdr does not accept unsolicited implementation pull requests from contributors who are not listed in The pull request author is not an approved contributor. If you encountered a reproducible bug, report the observed behavior through the bug issue template. A report does not reserve the work or authorize a pull request; accepted fixes are normally implemented by Herdr’s maintainer-controlled agents. Feature requests, behavior changes, and other proposals belong in GitHub Discussions. Do not open an issue merely to justify an implementation that was already written. If a maintainer explicitly wants this implementation, they can reopen the pull request. Reopening by anyone else will be closed again automatically. See https://github.com/herdrdev/herdr/blob/master/CONTRIBUTING.md for the contribution policy. |
Root Cause:
herdr pane report-agent,report-agent-session,release-agent,report-metadata, andherdr workspace report-metadataeach take their id as the first positional argument in a hand-written parser (args.first()). clap's default usage synopsis renders required options before positionals, so--helpshows the id last, e.g.Usage: herdr pane report-metadata [OPTIONS] --source <ID> <PANE_ID>. Following that order (--source foo bar) makes the parser treat--sourceitself as the id, then fail on the next token withunknown option: bar— a misleading error unrelated to the actual problem.The Fix: Add
.override_usage(...)to each of the five affectedCommandbuilders insrc/cli/spec.rs, listing the id first, matching the convention already used for severalagentsubcommands (read/prompt/rename/wait/attach) in the same file. No parser or behavior change — only the displayed usage line.Validation: Added
pane_and_workspace_id_reporting_commands_show_id_before_required_options, checking the exactUsage:line for all five commands viawrite_requested_help.cargo fmt --checkandcargo clippy --all-targets --locked -- -D warningsare clean. Fullcargo nextest runpasses except 5 pre-existing, unrelated failures (locale-dependent git-error-text assertions in the worktree test suite — reproduced identically against pristineorigin/masterbefore this change).References: none — no existing issue or PR covers this; found while working on an unrelated feature branch.