All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
	"Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
	"Denton Liu" <liu.denton@gmail.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH v2] parse-options: don't emit "ambiguous option" for aliases
Date: Wed, 17 Apr 2019 14:44:38 +0200	[thread overview]
Message-ID: <20190417124438.8191-1-avarab@gmail.com> (raw)
In-Reply-To: <20190325202329.26033-3-avarab@gmail.com>

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 <avarab@gmail.com>
---

See
https://public-inbox.org/git/20190325202329.26033-1-avarab@gmail.com/
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


  reply	other threads:[~2019-04-17 12:44 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-25 18:14 [PATCH 0/8] Do not use abbreviated options in tests Johannes Schindelin via GitGitGadget
2019-03-25 18:14 ` [PATCH 1/8] tests (rebase): spell out the `--keep-empty` option Johannes Schindelin via GitGitGadget
2019-03-25 18:14 ` [PATCH 2/8] tests (rebase): spell out the `--force-rebase` option Johannes Schindelin via GitGitGadget
2019-03-25 18:14 ` [PATCH 3/8] t7810: do not abbreviate `--no-exclude-standard` nor `--invert-match` Johannes Schindelin via GitGitGadget
2019-03-25 18:14 ` [PATCH 4/8] t5531: avoid using an abbreviated option Johannes Schindelin via GitGitGadget
2019-03-25 18:14 ` [PATCH 5/8] tests (push): do not abbreviate the `--follow-tags` option Johannes Schindelin via GitGitGadget
2019-03-25 18:14 ` [PATCH 6/8] tests (status): spell out the `--find-renames` option in full Johannes Schindelin via GitGitGadget
2019-03-25 18:14 ` [PATCH 7/8] tests (pack-objects): use the full, unabbreviated `--revs` option Johannes Schindelin via GitGitGadget
2019-03-25 18:14 ` [PATCH 8/8] tests: disallow the use of abbreviated options (by default) Johannes Schindelin via GitGitGadget
2019-03-25 18:35   ` Denton Liu
2019-03-25 20:26     ` Ævar Arnfjörð Bjarmason
2019-04-12  8:48       ` Johannes Schindelin
2019-04-12  8:50     ` Johannes Schindelin
2019-03-25 19:47   ` Ævar Arnfjörð Bjarmason
2019-04-12  8:59     ` Johannes Schindelin
2019-03-25 20:23 ` [PATCH 0/2] allow for configuring option abbreviation + fix Ævar Arnfjörð Bjarmason
2019-03-25 20:23 ` [PATCH 1/2] parse-options: allow for configuring option abbreviation Ævar Arnfjörð Bjarmason
2019-03-25 21:23   ` Eric Sunshine
2019-03-25 22:47     ` Ævar Arnfjörð Bjarmason
2019-03-26  4:14       ` Duy Nguyen
2019-03-26  6:28         ` Ævar Arnfjörð Bjarmason
2019-03-26  7:13           ` Duy Nguyen
2019-03-26 11:00             ` Ævar Arnfjörð Bjarmason
2019-04-01 10:47     ` Junio C Hamano
2019-04-12  9:06       ` Johannes Schindelin
2019-03-25 20:23 ` [PATCH 2/2] parse-options: don't emit "ambiguous option" for aliases Ævar Arnfjörð Bjarmason
2019-04-17 12:44   ` Ævar Arnfjörð Bjarmason [this message]
2019-04-17 16:04     ` [PATCH v2] " Duy Nguyen
2019-04-18  0:48       ` Junio C Hamano
2019-04-18  9:29         ` Duy Nguyen
2019-04-19  4:39           ` Junio C Hamano
2019-04-22 12:22             ` [PATCH] " Nguyễn Thái Ngọc Duy
2019-04-22 12:34               ` Duy Nguyen
2019-04-29 10:05               ` [PATCH v2] " Nguyễn Thái Ngọc Duy
2019-05-07  3:43                 ` Junio C Hamano
2019-05-07 11:58                   ` Duy Nguyen
2019-04-02  0:58 ` [PATCH 0/8] Do not use abbreviated options in tests Junio C Hamano
2019-04-12  9:37 ` [PATCH v2 " Johannes Schindelin via GitGitGadget
2019-04-12  9:37   ` [PATCH v2 1/8] tests (rebase): spell out the `--keep-empty` option Johannes Schindelin via GitGitGadget
2019-04-12  9:37   ` [PATCH v2 2/8] tests (rebase): spell out the `--force-rebase` option Johannes Schindelin via GitGitGadget
2019-04-12  9:37   ` [PATCH v2 3/8] t7810: do not abbreviate `--no-exclude-standard` nor `--invert-match` Johannes Schindelin via GitGitGadget
2019-04-12  9:37   ` [PATCH v2 4/8] t5531: avoid using an abbreviated option Johannes Schindelin via GitGitGadget
2019-04-12  9:37   ` [PATCH v2 5/8] tests (push): do not abbreviate the `--follow-tags` option Johannes Schindelin via GitGitGadget
2019-04-12  9:37   ` [PATCH v2 6/8] tests (status): spell out the `--find-renames` option in full Johannes Schindelin via GitGitGadget
2019-04-12  9:37   ` [PATCH v2 7/8] tests (pack-objects): use the full, unabbreviated `--revs` option Johannes Schindelin via GitGitGadget
2019-04-12  9:37   ` [PATCH v2 8/8] tests: disallow the use of abbreviated options (by default) Johannes Schindelin via GitGitGadget
2019-04-14  2:35     ` Junio C Hamano
2019-04-15  2:55       ` Junio C Hamano
2019-04-15 13:09         ` Johannes Schindelin
2019-04-15 13:45           ` Junio C Hamano
2019-04-17 12:07             ` Johannes Schindelin
2019-04-15 13:08       ` Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190417124438.8191-1-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=liu.denton@gmail.com \
    --cc=pclouds@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.