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>,
	"Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
	"Denton Liu" <liu.denton@gmail.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH 1/2] parse-options: allow for configuring option abbreviation
Date: Mon, 25 Mar 2019 21:23:28 +0100	[thread overview]
Message-ID: <20190325202329.26033-2-avarab@gmail.com> (raw)
In-Reply-To: <pull.167.git.gitgitgadget@gmail.com>

Add a new core.abbreviatedOptions which corresponds to the newly added
GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS test mode. This allows
e.g. script authors to enact the same sort of paranoia about option
abbreviation as our own test suite now uses by default.

I'm renaming GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS to
GIT_TEST_ABBREVIATED_OPTIONS for consistency with the new
configuration variable, and I've picked the shorter
core.abbreviatedOptions instead of core.disallowAbbreviatedOptions
because it's easier to reason about a variable that's true by default
than about one that has a negation in its name.

While I'm at it, factor out the check for the environment variable
into a parse_options_config() which similar to the code added in
62c23938fa ("tests: add a special setup where rebase.useBuiltin is
off", 2018-11-14) will first check the environment variable, and then
the configuration.

Using git_env_bool() means it's a bit of a pain to distinguish between
the case where GIT_TEST_ABBREVIATED_OPTIONS is set to "true" or
"false" v.s. being empty, hence the
test_when_finished/export/sane_unset dance in the test that's being
added, but I think that's more sane than introducing a "bool but not
in the same way as the rest" GIT_TEST_* variable.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/config/core.txt | 12 ++++++++++++
 parse-options.c               | 19 +++++++++++++++----
 t/README                      |  4 ++--
 t/t0040-parse-options.sh      | 22 +++++++++++++++++-----
 t/test-lib.sh                 |  6 +++---
 5 files changed, 49 insertions(+), 14 deletions(-)

diff --git a/Documentation/config/core.txt b/Documentation/config/core.txt
index 7e9b6c8f4c..5a42ec3940 100644
--- a/Documentation/config/core.txt
+++ b/Documentation/config/core.txt
@@ -1,3 +1,15 @@
+core.abbreviatedOptions::
+	Defaults to `true` which allows options to be abbreviated as
+	long as they aren't ambiguous, e.g. for linkgit:git-init[1]
+	the `--bare` option can be abbreviated as `--bar`, `--ba` or
+	even `--b` since no other option starts with those
+	prefixes. However, if such an option were added in the future
+	any use of these abbreviations would break.
++
+By setting this to false (e.g. in scripts) you can defend against such
+future breakages by enforcing that options must always be fully
+provided.
+
 core.fileMode::
 	Tells Git if the executable bit of files in the working tree
 	is to be honored.
diff --git a/parse-options.c b/parse-options.c
index acc3a93660..d6a291f705 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -6,7 +6,7 @@
 #include "color.h"
 #include "utf8.h"
 
-static int disallow_abbreviated_options;
+static int abbreviated_options = 1;
 
 #define OPT_SHORT 1
 #define OPT_UNSET 2
@@ -346,7 +346,7 @@ static enum parse_opt_result parse_long_opt(
 		return get_value(p, options, all_opts, flags ^ opt_flags);
 	}
 
-	if (disallow_abbreviated_options && (ambiguous_option || abbrev_option))
+	if (!abbreviated_options && (ambiguous_option || abbrev_option))
 		die("disallowed abbreviated or ambiguous option '%.*s'",
 		    (int)(arg_end - arg), arg);
 
@@ -456,6 +456,17 @@ static void parse_options_check(const struct option *opts)
 		exit(128);
 }
 
+static void parse_options_config(void)
+{
+	int env = git_env_bool("GIT_TEST_ABBREVIATED_OPTIONS", -1);
+	if (env != -1) {
+		abbreviated_options = env;
+		return;
+	}
+	git_config_get_bool("core.abbreviatedoptions", &abbreviated_options);
+	return;
+}
+
 void parse_options_start(struct parse_opt_ctx_t *ctx,
 			 int argc, const char **argv, const char *prefix,
 			 const struct option *options, int flags)
@@ -480,6 +491,7 @@ void parse_options_start(struct parse_opt_ctx_t *ctx,
 	    (flags & PARSE_OPT_KEEP_ARGV0))
 		BUG("Can't keep argv0 if you don't have it");
 	parse_options_check(options);
+	parse_options_config();
 }
 
 static void show_negated_gitcomp(const struct option *opts, int nr_noopts)
@@ -714,9 +726,8 @@ int parse_options(int argc, const char **argv, const char *prefix,
 {
 	struct parse_opt_ctx_t ctx;
 
-	disallow_abbreviated_options =
-		git_env_bool("GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS", 0);
 
+	parse_options_config();
 	parse_options_start(&ctx, argc, argv, prefix, options, flags);
 	switch (parse_options_step(&ctx, options, usagestr)) {
 	case PARSE_OPT_HELP:
diff --git a/t/README b/t/README
index 9ed3051a1c..1d628baf31 100644
--- a/t/README
+++ b/t/README
@@ -399,9 +399,9 @@ GIT_TEST_SIDEBAND_ALL=<boolean>, when true, overrides the
 fetch-pack to not request sideband-all (even if the server advertises
 sideband-all).
 
-GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS=<boolean>, when true (which is
+GIT_TEST_ABBREVIATED_OPTIONS=<boolean>, when false (which is
 the default when running tests), errors out when an abbreviated option
-is used.
+is used. Overrides the core.abbreviatedOptions setting.
 
 Naming Tests
 ------------
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index 5f6a16336d..19685d1582 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -203,27 +203,39 @@ file: (not set)
 EOF
 
 test_expect_success 'unambiguously abbreviated option' '
-	GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS=false \
+	GIT_TEST_ABBREVIATED_OPTIONS=true \
 	test-tool parse-options --int 2 --boolean --no-bo >output 2>output.err &&
 	test_must_be_empty output.err &&
 	test_cmp expect output
 '
 
 test_expect_success 'unambiguously abbreviated option with "="' '
-	GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS=false \
+	GIT_TEST_ABBREVIATED_OPTIONS=true \
 	test-tool parse-options --expect="integer: 2" --int=2
 '
 
 test_expect_success 'ambiguously abbreviated option' '
-	test_expect_code 129 env GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS=false \
+	test_expect_code 129 env GIT_TEST_ABBREVIATED_OPTIONS=true \
 	test-tool parse-options --strin 123
 '
 
 test_expect_success 'non ambiguous option (after two options it abbreviates)' '
-	GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS=false \
+	GIT_TEST_ABBREVIATED_OPTIONS=true \
 	test-tool parse-options --expect="string: 123" --st 123
 '
 
+test_expect_success 'abbreviated options configured with core.abbreviatedOptions' '
+	test_when_finished "
+		rm -rf A B C &&
+		GIT_TEST_ABBREVIATED_OPTIONS=$GIT_TEST_ABBREVIATED_OPTIONS &&
+		export GIT_TEST_ABBREVIATED_OPTIONS
+	" &&
+	git init --bare A &&
+	test_must_fail git init --ba B &&
+	sane_unset GIT_TEST_ABBREVIATED_OPTIONS &&
+	git -c core.abbreviatedOptions=true init --ba C
+'
+
 cat >typo.err <<\EOF
 error: did you mean `--boolean` (with two dashes ?)
 EOF
@@ -329,7 +341,7 @@ file: (not set)
 EOF
 
 test_expect_success 'negation of OPT_NONEG flags is not ambiguous' '
-	GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS=false \
+	GIT_TEST_ABBREVIATED_OPTIONS=true \
 	test-tool parse-options --no-ambig >output 2>output.err &&
 	test_must_be_empty output.err &&
 	test_cmp expect output
diff --git a/t/test-lib.sh b/t/test-lib.sh
index e550009411..4b0dd8a1a9 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -58,9 +58,9 @@ fi
 export PERL_PATH SHELL_PATH
 
 # Disallow the use of abbreviated options in the test suite by default
-test -n "$GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS" || {
-	GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS=true
-	export GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS
+test -n "$GIT_TEST_ABBREVIATED_OPTIONS" || {
+	GIT_TEST_ABBREVIATED_OPTIONS=false
+	export GIT_TEST_ABBREVIATED_OPTIONS
 }
 
 ################################################################
-- 
2.21.0.360.g471c308f928


  parent reply	other threads:[~2019-03-25 20:23 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 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 18:14 ` [PATCH 7/8] tests (pack-objects): use the full, unabbreviated `--revs` option Johannes Schindelin via GitGitGadget
2019-03-25 20:23 ` [PATCH 0/2] allow for configuring option abbreviation + fix Ævar Arnfjörð Bjarmason
2019-03-25 20:23 ` Ævar Arnfjörð Bjarmason [this message]
2019-03-25 21:23   ` [PATCH 1/2] parse-options: allow for configuring option abbreviation 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   ` [PATCH v2] " Ævar Arnfjörð Bjarmason
2019-04-17 16:04     ` 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=20190325202329.26033-2-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 \
    /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.