Re: [PATCH] backfill: handle unexpected arguments

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

Siddharth Shrimali <r.siddharth.shrimali@gmail.com> writes:

> git backfill takes no non-option arguments. However, if extra
> arguments are passed with git backfill, parse_options() leaves
> them in argc and the command currently ignores them silently,
> giving the user no indication that something is wrong.

Well written.  If you drop unnecessary "currently", it would be
perfect ;-).

> Add a check after parse_options() to call usage_with_options()
> if any unexpected arguments remain. This prints the correct usage
> and exits with an error, consistent with how other Git commands
> such as git-gc and git-repack handle this situation.

I am not sure if this is a good idea.

When parse_options() finds an unrecognised option, you would get
usage-with-options help, so without explicitly telling the user
"Hey, you have an extra argument that I do not expect at the end of
the command line" and giving only the same usage-with-options help,
the user would not know why they are seeing the help message, as it
is totally unclear what mistake they made in their command line.

"git bugreport" is also a command that does not take any positional
arguments on its command line.  Study how it complains about an
unwanted argument, and follow its example, perhaps?

Thanks.

> Signed-off-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
> ---
>  builtin/backfill.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/builtin/backfill.c b/builtin/backfill.c
> index e9a33e81be..0eb171478a 100644
> --- a/builtin/backfill.c
> +++ b/builtin/backfill.c
> @@ -135,6 +135,9 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit
>  
>  	argc = parse_options(argc, argv, prefix, options, builtin_backfill_usage,
>  			     0);
> +	
> +	if (argc)
> +		usage_with_options(builtin_backfill_usage, options);
>  
>  	repo_config(repo, git_default_config, NULL);