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

Patrick Steinhardt [off-list ref] writes:

quoted

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