From: Phillip Wood <phillip.wood123@gmail.com>
Date: Sun, 22 Mar 2026 16:38:03 GMT
Subject: Re: [PATCH v3] backfill: handle unexpected arguments
Message-ID: <45a949f3-8b90-4046-995f-da1df265abfe@gmail.com>
In-Reply-To: <20260322053207.60992-1-r.siddharth.shrimali@gmail.com>

On 22/03/2026 05:32, Siddharth Shrimali wrote:
> 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 ignores them silently, giving the
> user no indication that something is wrong.
> 
> Add a check after parse_options() to report an error if any unexpected
> arguments remain. To ensure the user understands why the command
> failed, print an error message specifying the unknown argument
> followed by the short usage string. This matches the behavior of
> other Git commands such as git bugreport.
> 
> Also, add a test in t5620 to ensure the unexpected arguments are
> rejected with the correct error message and that the full option
> descriptions are not printed.

Nicely explained

> diff --git a/t/t5620-backfill.sh b/t/t5620-backfill.sh
> index 58c81556e7..3f1eeb67e8 100755
> --- a/t/t5620-backfill.sh
> +++ b/t/t5620-backfill.sh
> @@ -176,6 +176,12 @@ test_expect_success 'backfill --sparse without cone mode (negative)' '
>   	test_line_count = 12 missing
>   '
>   
> +test_expect_success 'backfill rejects unexpected arguments' '
> +	test_must_fail git -C backfill1 backfill unexpected-arg >err 2>&1 &&
> +	grep "unknown argument .*unexpected-arg" err &&
> +	! grep "Minimum number of objects" err

Using test_grep would make test failures easier to debug as it prints a 
diagnostic message if it fails. Note that "! grep" should become 
"test_grep !" to ensure the diagnostic message is printed when the 
expression matches.

Thanks

Phillip


> +'
> +
>   . "$TEST_DIRECTORY"/lib-httpd.sh
>   start_httpd
>