Re: [PATCH 1/2] bugreport: avoid duplicating options in usage()

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 7, 2021, 21:39 UTC
Message-ID: <xmqq35qg9iao.fsf@gitster.g>
In-Reply-To: <20210904021231.88534-2-carenas@gmail.com>

Carlo Marcelo Arenas Belón  <carenas@gmail.com> writes:

Show 6 quoted lines

> 238b439d69 (bugreport: add tool to generate debugging info, 2020-04-16)
> includes the options with the commandline, which then means they will
> be duplicated in the output of `git bugreport -h`.
>
> remove them and while at it, make sure usage() is called if the wrong
> number of parameters is provided (ex: `git bugreport help`)

'remove' -> 'Remove'.

>  static const char * const bugreport_usage[] = {
> -	N_("git bugreport [-o|--output-directory <file>] [-s|--suffix <format>]"),
> +	N_("git bugreport"),

I do not quite see this as an improvement. Without this change, the user will see

	usage: git bugreport [-o <file>] [-s <format>]

	    -o <file>
		... explanation of what -o does ...
	    -s <format>
		... explanation of what -s does ...

and with the patch, it becomes unclear, especially for those who are not used to "git subcommand -h" output convention, as we'd see only

	usage: git bugreport

on the first line, no?  If the patch is to use

	N_("git bugreport [<options>]")

as the new text, then that would be an improvement, though.

Show 6 quoted lines

> @@ -141,6 +140,8 @@ int cmd_bugreport(int argc, const char **argv, const char *prefix)
>  
>  	argc = parse_options(argc, argv, prefix, bugreport_options,
>  			     bugreport_usage, 0);
> +	if (argc)
> +		usage_with_options(bugreport_usage, bugreport_options);

This is a good change (until we gain positional argument to the subcommand, at which time we'd need to rethink the error checking).

>  	/* Prepare the path to put the result */
>  	prefixed_filename = prefix_filename(prefix,