All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Martin Ågren" <martin.agren@gmail.com>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>
Subject: [PATCH 5/8] builtin/config: canonicalize "value_regex" with `--type=bool`
Date: Wed, 13 Nov 2019 19:55:04 +0100	[thread overview]
Message-ID: <beb0cfbb78e3ae69e64ff2ed57c8e628ccb187ce.1573670565.git.martin.agren@gmail.com> (raw)
In-Reply-To: <cover.1573670565.git.martin.agren@gmail.com>

With `--type=bool`, we use the "value_regex" as a normal regex and do
not use our knowledge about the different well-defined ways we have of
spelling the boolean values. Let's consider a few examples.

These output "true":
	git -c foo.bar=true config --type=bool --get foo.bar true
	git -c foo.bar=true config --type=bool --get foo.bar t

These produce no output:
	git -c foo.bar=true config --type=bool --get foo.bar True
	git -c foo.bar=true config --type=bool --get foo.bar 1

Canonicalize the provided "value_regex", then canonicalize candidate
values as we go through the config and compare the canonicalized values.
If the parameter doesn't canonicalize, fall back to handling it as a
regex, as before. This would happen in the second example above, but
also if someone has hand-rolled their own canonicalisation with, e.g.,
something like "^(on|On|ON|true|1|42)$".

This commit affects all modes that take a "value_regex", e.g.,
`--get-regexp` which can be used in a more useful way than the examples
above might at first suggest:

	git config --type=bool --name-only --get-regexp '^foo\.' true

This commit does change the behavior for certain usages, but it almost
certainly does so for the better: We don't exclude config items based on
how the config happens to spell "true" or "false". This commit would
cause a regression if someone uses different synonyms for "true",
knowing that Git handles them all the same in day-to-day operation, but
relying on the encoding (using, say, integers 1-100) to signal some sort
of confidence and using `git config` to query for various such "levels".
I'm tempted to bet no-one will be hurt by this change.

Do not rename the "value_regex" in the documentation. This commit can be
seen as teaching `git config --type=bool` about a particular type of
regex, where "true" matches "yes", but not "no". So arguably,
"value_regex" still describes quite well what is going on.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 Documentation/git-config.txt |  4 ++++
 builtin/config.c             | 15 +++++++++++-
 t/t1300-config.sh            | 45 ++++++++++++++++++++++++++++++++++++
 3 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 899e92a1c9..139750bbda 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -43,6 +43,10 @@ outgoing values are canonicalize-able under the given <type>.  If no
 `--type=<type>` is given, no canonicalization will be performed. Callers may
 unset an existing `--type` specifier with `--no-type`.
 
+With `--type=bool`, if `value_regex` is given
+and canonicalizes to a boolean value, it matches all entries
+that canonicalize to the same boolean value.
+
 When reading, the values are read from the system, global and
 repository local configuration files by default, and options
 `--system`, `--global`, `--local`, `--worktree` and
diff --git a/builtin/config.c b/builtin/config.c
index d812e73e2b..c9fe0c5752 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -15,9 +15,10 @@ static const char *const builtin_config_usage[] = {
 static char *key;
 static regex_t *key_regexp;
 static struct {
-	enum { none, regexp } mode;
+	enum { none, regexp, boolean } mode;
 	regex_t *regexp;
 	int do_not_match; /* used with `regexp` */
+	int boolean;
 } cmd_line_value;
 static int show_keys;
 static int omit_values;
@@ -278,6 +279,9 @@ static int collect_config(const char *key_, const char *value_, void *cb)
 	     !!regexec(cmd_line_value.regexp, value_ ? value_ : "",
 		       0, NULL, 0)))
 		return 0;
+	if (cmd_line_value.mode == boolean &&
+	    git_parse_maybe_bool(value_) != cmd_line_value.boolean)
+		return 0;
 
 	ALLOC_GROW(values->items, values->nr + 1, values->alloc);
 	strbuf_init(&values->items[values->nr], 0);
@@ -292,6 +296,15 @@ static int handle_value_regex(const char *regex_)
 		return 0;
 	}
 
+	if (type == TYPE_BOOL) {
+		int boolval = git_parse_maybe_bool(regex_);
+		if (boolval >= 0) {
+			cmd_line_value.mode = boolean;
+			cmd_line_value.boolean = boolval;
+			return 0;
+		}
+	}
+
 	cmd_line_value.mode = regexp;
 
 	if (regex_[0] == '!') {
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index a38cc143a1..e4906a893e 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -427,6 +427,51 @@ test_expect_success 'no arguments, but no crash' '
 	test_i18ngrep usage output
 '
 
+test_expect_success 'setup config file with several boolean values' '
+	cat >.git/config <<-\EOF
+	[foo]
+		n1 = no
+		n2 = NO
+		n3 = off
+		n4 = false
+		n5 = 0
+		n6 =
+		y1 = yes
+		y2 = YES
+		y3 = on
+		y4 = true
+		y5 = 1
+		y6 = 42
+		y7
+	EOF
+'
+
+test_expect_success '--get-regexp canonicalizes value_regex with --type=bool (false)' '
+	git config --type=bool --get-regexp "foo\..*" OFF >output &&
+	test_line_count = 6 output &&
+	! grep -v "^foo.n" output
+'
+
+test_expect_success '--get-regexp canonicalizes value_regex with --type=bool (true)' '
+	git config --type=bool --get-regexp "foo\..*" ON >output &&
+	test_line_count = 7 output &&
+	! grep -v "^foo.y" output
+'
+
+test_expect_success '--get canonicalizes integer value_regex with --type=bool' '
+	echo true >expect &&
+	git config --type=bool --get foo.y2 1 >output &&
+	test_cmp expect output
+'
+
+test_expect_success '--type=bool with "non-bool" value_regex' '
+	echo true >expect &&
+	git config --type=bool --get foo.y4 "t.*" >output &&
+	test_cmp expect output &&
+	test_must_fail git config --type=bool --get foo.y4 "T.*" >output &&
+	test_must_be_empty output
+'
+
 test_expect_success 'setup simple config file' '
 	q_to_tab >.git/config <<-\EOF
 	[a.b]
-- 
2.24.0


  parent reply	other threads:[~2019-11-13 18:55 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-13 18:54 [PATCH 0/8] builtin/config: canonicalize "value_regex" with `--type=bool[-or-int]` Martin Ågren
2019-11-13 18:55 ` [PATCH 1/8] config: make `git_parse_maybe_bool_text()` public Martin Ågren
2019-11-13 18:55 ` [PATCH 2/8] t1300: modernize part of script Martin Ågren
2019-11-21  4:54   ` Junio C Hamano
2019-11-13 18:55 ` [PATCH 3/8] builtin/config: extract `handle_value_regex()` from `get_value()` Martin Ågren
2019-11-21  5:02   ` Junio C Hamano
2019-11-21 19:53     ` Martin Ågren
2019-11-13 18:55 ` [PATCH 4/8] builtin/config: collect "value_regexp" data in a struct Martin Ågren
2019-11-21  5:22   ` Junio C Hamano
2019-11-21 19:55     ` Martin Ågren
2019-11-22  6:30       ` Junio C Hamano
2019-11-13 18:55 ` Martin Ågren [this message]
2019-11-21  5:37   ` [PATCH 5/8] builtin/config: canonicalize "value_regex" with `--type=bool` Junio C Hamano
2019-11-13 18:55 ` [PATCH 6/8] builtin/config: canonicalize "value_regex" with `--type=bool-or-int` Martin Ågren
2019-11-13 18:55 ` [PATCH 7/8] builtin/config: warn if "value_regex" doesn't canonicalize as boolean Martin Ågren
2019-11-21  5:43   ` Junio C Hamano
2019-11-21 19:58     ` Martin Ågren
2019-11-13 18:55 ` [PATCH 8/8] builtin/config: die " Martin Ågren
2019-11-14  2:18 ` [PATCH 0/8] builtin/config: canonicalize "value_regex" with `--type=bool[-or-int]` Junio C Hamano
2019-11-14  6:40   ` Martin Ågren
2019-11-14  6:29 ` Jeff King
2019-11-14  6:54   ` Martin Ågren
2019-11-14  7:37     ` Jeff King

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=beb0cfbb78e3ae69e64ff2ed57c8e628ccb187ce.1573670565.git.martin.agren@gmail.com \
    --to=martin.agren@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.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.