Re: git bugreport with invalid CLI argument does not report error
- From
Emily Shaffer <nasamuffin@google.com>
- Date
- Oct 25, 2023, 22:53 UTC
- Message-ID
- <ZTmcVJaY2TjFCVyw@google.com>
- In-Reply-To
- <7a92b537-ba88-4667-bb18-2e8c74aa9915@gmail.com>
On Thu, Oct 26, 2023 at 07:59:16AM +1100, Sheik wrote:
Show 14 quoted lines
> > Hi Maintainers, > > > Running git bugreport with an invalid CLI argument in a valid Git directory > does not report error. Expected behaviour would be that it reports an error. > > > #Example git commands which should have reported an error but continues to > succeed > > cd $ToAnyDirectory > > git bugreport diagnose
It looks like parse-options.[ch] helps us here for misspelled dashed options, like `--diaggnose`. But it doesn't complain when there are unexpected positional arguments. I think we can just notice if there are any argc left over, complain, and print usage.
I put together a quick patch; could be that we don't need to leave this error about "positional arguments" and can leave it as an exercise to the reader to compare their previous command to the usage text. I guess we could also unroll remaining argv but it was just a hair more time than I wanted to spend ;)
- Emily
--- 8< ---
From 2031c7f55652559b8b4ec3c67ce4c4f94a355762 Mon Sep 17 00:00:00 2001 From: Emily Shaffer <nasamuffin@google.com> Date: Wed, 25 Oct 2023 15:45:25 -0700 Subject: [PATCH] bugreport: reject positional arguments
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> --- builtin/bugreport.c | 5 +++++ t/t0091-bugreport.sh | 6 ++++++ 2 files changed, 11 insertions(+)
diff --git a/builtin/bugreport.c b/builtin/bugreport.c index d2ae5c305d..eb6234a50d 100644 --- a/builtin/bugreport.c +++ b/builtin/bugreport.c @@ -126,6 +126,11 @@ int cmd_bugreport(int argc, const char **argv, const char *prefix) argc = parse_options(argc, argv, prefix, bugreport_options, bugreport_usage, 0); + if (argc) { + error(_("git bugreport does not take positional arguments")); + usage(bugreport_usage[0]); + } + /* Prepare the path to put the result */ prefixed_filename = prefix_filename(prefix, option_output ? option_output : ""); diff --git a/t/t0091-bugreport.sh b/t/t0091-bugreport.sh index f6998269be..2061d6f386 100755 --- a/t/t0091-bugreport.sh +++ b/t/t0091-bugreport.sh @@ -69,6 +69,12 @@ test_expect_success 'incorrect arguments abort with usage' ' test_path_is_missing git-bugreport-* ' +test_expect_success 'incorrect positional arguments abort with usage' ' + test_must_fail git bugreport false 2>output && + test_i18ngrep usage output && + test_path_is_missing git-bugreport-* +' + test_expect_success 'runs outside of a git dir' ' test_when_finished rm non-repo/git-bugreport-* && nongit git bugreport
-- 2.42.0.758.gaed0368e0e-goog