Patrick Steinhardt <ps@xxxxxx> writes:
>> - cat >expect <<-EOF &&
>> - usage: too many arguments
>> - EOF
>> + {
>> + printf "fatal: unknown argument: ${SQ}foo${SQ}\n\n" &&
>> + ( git -C repo refs migrate -h || : )
>> + } >expect &&
>
> I always have to wonder how helpful it really is to print the usage
> information in such a context. I feel that it is too distracting because
> in many cases, we end up printing dozens of lines of options that drown
> out the single line of information that the user actually cares for,
> namely why the command has failed.
>
> In this case here it is somewhat manageable, because only 4/5th of the
> output are unnecessary noise. But the picture changes as commands grow
> more options over time, making the output less and less usable.
>
> So while I think that it is a big improvement to explicitly point out
> the unknown argument, I think it is step backwards to also print the
> usage info.
Yeah, I somehow was fooled by the original that called a usage()
function ;-).
"usage:" should signal that the message given is a command line to
show the usage, i.e.
$ git grep -E -e '[^_]usage\("' builtin/\*.c
builtin/merge-index.c: usage("git merge-index [-o] [-q] <merge-program> (-a | [--] [<filename>...])");
builtin/unpack-file.c: usage("git unpack-file <blob>");
and is not a signal that the message explains what it found
problematic in this particular usage of the command. builtin/refs.c
being relatively young do not honor the tradition, it seems.
$ git grep -E -e 'usage\(' builtin/refs.c
builtin/refs.c: usage(_("too many arguments"));
builtin/refs.c: usage(_("missing --ref-format=<format>"));
I think die(_("...")) would be a lot more appropriate in these two
places, including the one I touched.
Thanks.
