Re: [PATCH v3] bugreport: reject positional arguments

[Eric Sunshine <sunshine@sunshineco.com>]

On Thu, Oct 26, 2023 at 11:55 AM <emilyshaffer@google.com> wrote:
> git-bugreport already rejected unrecognized flag arguments, like
> `--diaggnose`, but this doesn't help if the user's mistake was to forget
> the `--` in front of the argument. This can result in a user's intended
> argument not being parsed with no indication to the user that something
> went wrong. Since git-bugreport presently doesn't take any positionals
> at all, let's reject all positionals and give the user a usage hint.
>
> Signed-off-by: Emily Shaffer <nasamuffin@google.com>
> ---
> Per Eric's and Dragan's comments, dropped the null checking for argv[0].
> No point in being too paranoid, I suppose :)
>
> Note that after this morning it's not likely that I'll be able to find
> time to update this again so quickly, so if there are other nits,
> reviewers can feel free to send their own rerolls rather than waiting
> for me to see it and turn the patch around.

Thanks. This version looks good enough to me. Just one minor comment below...

> diff --git a/t/t0091-bugreport.sh b/t/t0091-bugreport.sh
> @@ -69,6 +69,13 @@ test_expect_success 'incorrect arguments abort with usage' '
> +test_expect_success 'incorrect positional arguments abort with usage and hint' '
> +       test_must_fail git bugreport false 2>output &&
> +       test_i18ngrep usage output &&
> +       test_i18ngrep false output &&
> +       test_path_is_missing git-bugreport-*
> +'

I didn't really pay attention to the test in earlier rounds so didn't
notice this, but these days we just use 'grep' rather than
'test_i18ngrep'. (Indeed, the existing tests in this script use
'grep'.)