On Thu, Oct 26, 2023 at 11:55 AM <emilyshaffer@xxxxxxxxxx> 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@xxxxxxxxxx> > --- > 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'.)
