Re: [PATCH v3 06/16] builtin/multi-pack-index.c: display usage on unrecognized command
- From
Jeff King <peff@peff.net>
- Date
- Mar 29, 2021, 11:42 UTC
- Message-ID
- <YGG9GogdjlijGxSF@coredump.intra.peff.net>
- In-Reply-To
- <f117e442c3460661dc88beba6f1853d6c388b0fd.1615482270.git.me@ttaylorr.com>
On Thu, Mar 11, 2021 at 12:04:57PM -0500, Taylor Blau wrote:
Show 20 quoted lines
> When given a sub-command that it doesn't understand, 'git > multi-pack-index' dies with the following message: > > $ git multi-pack-index bogus > fatal: unrecognized subcommand: bogus > > Instead of 'die()'-ing, we can display the usage text, which is much > more helpful: > > $ git.compile multi-pack-index bogus > usage: git multi-pack-index [<options>] write > or: git multi-pack-index [<options>] verify > or: git multi-pack-index [<options>] expire > or: git multi-pack-index [<options>] repack [--batch-size=<size>] > > --object-dir <file> object directory containing set of packfile and pack-index pairs > --progress force progress reporting > > While we're at it, clean up some duplication between the "no sub-command" > and "unrecognized sub-command" conditionals.
I agree that it's much nicer to give the usage. But my preference in general for cases like this is to _also_ explain what we found wrong with the options we were given.
E.g., with a bogus option, we say so:
$ git multi-pack-index --foo error: unknown option `foo' usage: git multi-pack-index [<options>] write [--preferred-pack=<pack>] [etc...]
but with a bogus sub-command, we get just the usage string:
$ git multi-pack-index foo usage: git multi-pack-index [<options>] write [--preferred-pack=<pack>] [etc...]
Sometimes it is quote obvious what is wrong, but sometimes typos can be hard to spot, especially because the usage message is so long.
I.e., I'd suggest changing this:
Show 5 quoted lines
> else
> - die(_("unrecognized subcommand: %s"), argv[0]);
> +usage:
> + usage_with_options(builtin_multi_pack_index_usage,
> + builtin_multi_pack_index_options);to:
error(_("unrecognized subcommand: %s"), argv[0]);
usage_with_options(...);-Peff