On 26/10/2023 18:18, Eric Sunshine wrote:
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'.)
It is rather unfortunate that test_i18ngrep was deprecated without
providing an alternative that offers the same debugging experience. When
test_i18ngrep fails it prints a message with the pattern and text that
failed to match so it is easy to see where the test failed. If grep
fails there is no output and so unless the test is run with "-x" it can
be hard to see which command caused the test to fail.
Best Wishes
Phillip
