Subject: [PATCH v2] parse-options: don't emit "ambiguous option" for aliases

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Wed, 17 Apr 2019 14:44:38 +0200

Change the option parsing machinery so that e.g. "clone --recurs ..."
doesn't error out because "clone" understands both "--recursive" and
"--recurse-submodules" to mean the same thing.

Initially "clone" just understood --recursive until the
--recurses-submodules alias was added in ccdd3da652 ("clone: Add the
--recurse-submodules option as alias for --recursive",
2010-11-04). Since bb62e0a99f ("clone: teach --recurse-submodules to
optionally take a pathspec", 2017-03-17) the longer form has been
promoted to the default.

But due to the way the options parsing machinery works this resulted
in the rather absurd situation of:

    $ git clone --recurs [...]
    error: ambiguous option: recurs (could be --recursive or --recurse-submodules)

Let's re-use the PARSE_OPT_NOCOMPLETE flag to mean "this option
doesn't contribute to abbreviation ambiguity". I was going to add a
new PARSE_OPT_NOABBREV flag, but it makes sense just to re-use
PARSE_OPT_NOCOMPLETE.

Signed-off-by: Ævar Arnfjörð Bjarmason <[email-masked]>
---

See
https://public-inbox.org/git/[email-masked]/
for v1. There wasn't consensus for 1/2 there, but this used-to-be 2/2
seems like a no-brainer bugfix.

It conflicted with some recently-landed stuff in 'master', but now
cleanly applies to it and 'pu', and with pu's
GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS.

 builtin/clone.c          | 4 ++--
 parse-options.c          | 3 ++-
 parse-options.h          | 2 ++
 t/t0040-parse-options.sh | 5 +++++
 4 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 50bde99618..4dc26969a7 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -100,8 +100,8 @@ static struct option builtin_clone_options[] = {
 		    N_("setup as shared repository")),
 	{ OPTION_CALLBACK, 0, "recursive", &option_recurse_submodules,
 	  N_("pathspec"), N_("initialize submodules in the clone"),
-	  PARSE_OPT_OPTARG | PARSE_OPT_HIDDEN, recurse_submodules_cb,
-	  (intptr_t)"." },
+	  PARSE_OPT_OPTARG | PARSE_OPT_HIDDEN | PARSE_OPT_NOCOMPLETE,
+	  recurse_submodules_cb, (intptr_t)"." },
 	{ OPTION_CALLBACK, 0, "recurse-submodules", &option_recurse_submodules,
 	  N_("pathspec"), N_("initialize submodules in the clone"),
 	  PARSE_OPT_OPTARG, recurse_submodules_cb, (intptr_t)"." },
diff --git a/parse-options.c b/parse-options.c
index cec74522e5..9899ce0171 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -292,7 +292,8 @@ static enum parse_opt_result parse_long_opt(
 		if (!rest) {
 			/* abbreviated? */
 			if (!(p->flags & PARSE_OPT_KEEP_UNKNOWN) &&
-			    !strncmp(long_name, arg, arg_end - arg)) {
+			    !strncmp(long_name, arg, arg_end - arg) &&
+			    !(options->flags & PARSE_OPT_NOCOMPLETE)) {
 is_abbreviated:
 				if (abbrev_option) {
 					/*
diff --git a/parse-options.h b/parse-options.h
index 74cce4e7fc..51c4b71ab0 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -96,6 +96,8 @@ typedef enum parse_opt_result parse_opt_ll_cb(struct parse_opt_ctx_t *ctx,
  *				Useful for options with multiple parameters.
  *   PARSE_OPT_NOCOMPLETE: by default all visible options are completable
  *			   by git-completion.bash. This option suppresses that.
+ *			   Will also skip this option when abbreviation is
+ *			   considered.
  *   PARSE_OPT_COMP_ARG: this option forces to git-completion.bash to
  *			 complete an option as --name= not --name even if
  *			 the option takes optional argument.
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index b8f366c442..e8f0371830 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -220,6 +220,11 @@ test_expect_success 'non ambiguous option (after two options it abbreviates)' '
 	test-tool parse-options --expect="string: 123" --st 123
 '
 
+test_expect_success 'NOCOMPLETE options do not contribute to abbreviation' '
+	test_when_finished "rm -rf A" &&
+	git clone --recurs . A
+'
+
 cat >typo.err <<\EOF
 error: did you mean `--boolean` (with two dashes ?)
 EOF
-- 
2.21.0.593.g511ec345e18