Re: [PATCH v1 1/4] refs: avoid "too many arguments"

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> 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.