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

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 6, 2024, 16:48 UTC
Message-ID: <xmqqed71vbps.fsf@gitster.g>
In-Reply-To: <ZrG_A3UQk56aYJ_m@tanuki>

Patrick Steinhardt <ps@pks.im> writes:

Show 21 quoted lines

>> -	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.