[BUG 1.7.6.1] `git config --bool --get-regexp' omits separating space... sometimes!

Hello,

# Script to illustrate the problem:
rm -f Config
cat <<\EOF >Config
[Example]
	Boolean
	Other = yes
EOF
git_Config() { git config --file Config "$@"; }

git version
git_Config        --get-regexp '.*\.Boolean'	#1. ✓ Ok: example.boolean
git_Config --bool --get-regexp '.*\.Boolean'	#2. ✗ NO: example.booleantrue
git_Config        --get-regexp '.*\.Other'	#3. ✓ Ok: example.other yes
git_Config --bool --get-regexp '.*\.Other'	#4. ✓ Ok: example.other true
exit

# Output:
git version 1.7.6.1
example.boolean
example.booleantrue
example.other yes
example.other true

 Case 2 omits the space between the key-name and (generated) value,
 making the output difficult to parse/process.  Without checking,
 I assume --int (and friends) have a similar bug?

cheers!
	-blf-

-- 
Brian FOSTER
Principal MTS, Software
Maxim Integrated Products (Microcontroller BU), formerly Innova Card
Web    : http://www.maxim-ic.com/

The previous logic in show_config was to print the delimiter when the
value was set, but Boolean variables have an implicit value "true" when
they appear with no value in the config file. As a result, we got:

git_Config        --get-regexp '.*\.Boolean'	#1. Ok: example.boolean
git_Config --bool --get-regexp '.*\.Boolean'	#2. NO: example.booleantrue

Fix this by defering the display of the separator until after the value
to display has been computed.

Reported-by: Brian Foster <redacted>
Signed-off-by: Matthieu Moy <redacted>
---
 builtin/config.c       |   20 +++++++++++++-------
 t/t1300-repo-config.sh |    6 ++++++
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 0b4ecac..0315ad7 100644
--- a/builtin/config.c
+++ b/builtin/config.c

@@ -99,6 +99,7 @@ static int show_config(const char *key_, const char *value_, void *cb)
 	const char *vptr = value;
 	int must_free_vptr = 0;
 	int dup_error = 0;
+	int must_print_delim = 0;
 
 	if (!use_key_regexp && strcmp(key_, key))
 		return 0;

@@ -109,10 +110,8 @@ static int show_config(const char *key_, const char *value_, void *cb)
 		return 0;
 
 	if (show_keys) {
-		if (value_)
-			printf("%s%c", key_, key_delim);
-		else
-			printf("%s", key_);
+		printf("%s", key_);
+		must_print_delim = 1;
 	}
 	if (seen && !do_all)
 		dup_error = 1;

@@ -130,16 +129,23 @@ static int show_config(const char *key_, const char *value_, void *cb)
 	} else if (types == TYPE_PATH) {
 		git_config_pathname(&vptr, key_, value_);
 		must_free_vptr = 1;
+	} else if (value_) {
+		vptr = value_;
+	} else {
+		/* Just show the key name */
+		vptr = "";
+		must_print_delim = 0;
 	}
-	else
-		vptr = value_?value_:"";
 	seen++;
 	if (dup_error) {
 		error("More than one value for the key %s: %s",
 				key_, vptr);
 	}
-	else
+	else {
+		if (must_print_delim)
+			printf("%c", key_delim);
 		printf("%s%c", vptr, term);
+	}
 	if (must_free_vptr)
 		/* If vptr must be freed, it's a pointer to a
 		 * dynamically allocated buffer, it's safe to cast to

diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 3e140c1..dffccf8 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh

@@ -333,6 +333,12 @@ test_expect_success 'get-regexp variable with no value' \
 	'git config --get-regexp novalue > output &&
 	 cmp output expect'
 
+echo 'novalue.variable true' > expect
+
+test_expect_success 'get-regexp --bool variable with no value' \
+	'git config --bool --get-regexp novalue > output &&
+	 cmp output expect'
+
 echo 'emptyvalue.variable ' > expect
 
 test_expect_success 'get-regexp variable with empty value' \

-- 
1.7.7.140.ge3099

Matthieu Moy [off-list ref] writes:

The previous logic in show_config was to print the delimiter when the
value was set, but Boolean variables have an implicit value "true" when
they appear with no value in the config file. As a result, we got:

git_Config        --get-regexp '.*\.Boolean'	#1. Ok: example.boolean
git_Config --bool --get-regexp '.*\.Boolean'	#2. NO: example.booleantrue

Fix this by defering the display of the separator until after the value
to display has been computed.

Reported-by: Brian Foster <redacted>
Signed-off-by: Matthieu Moy <redacted>
---

Thanks. Will queue for maintenance track.

This matches the style of the surrounding code, but we may want to update
this to a more modern style.