threads / patch / 24871

patchTest the interaction of --bool and --get-regexp on a key with no explicit value

Subject: [PATCH] Test the interaction of --bool and --get-regexp on a key with no explicit value

## tl;dr

4 messages between Aug 26, 2010 and Aug 27, 2010. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Alex Vandiver· Aug 26, 2010, 17:45 UTC · lore
Signed-off-by: Alex Vandiver <alex@chmrr.net>
---

This test currently fails -- I don't have time to track down the fix, but I figured that at least I could provide a failing test. Note that --null isn't necessary for the failure; without it, one gets "section.sub=section.val5true", with no spacing between the key and value.

 t/t1300-repo-config.sh |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)
Show changes to t/t1300-repo-config.sh +10 −0
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 074f2f2..147e0e6 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -816,6 +816,16 @@ echo >>result
 
 test_expect_success '--null --get-regexp' 'cmp result expect'
 
+cat > expect <<\EOF
+section.sub=section.val5
+trueQ
+EOF
+
+git config --null --bool --get-regexp 'val5' | perl -pe 'y/\000/Q/' > result
+echo >>result
+
+test_expect_success '--null --get-regexp --bool' 'cmp result expect'
+
 test_expect_success 'inner whitespace kept verbatim' '
 	git config section.val "foo 	  bar" &&
 	test "z$(git config section.val)" = "zfoo 	  bar"

-- 
1.7.2.2.458.g8d9c8
alex@chmrr.net· Aug 26, 2010, 20:49 UTC · re: Alex Vandiver · lore

[PATCH] config: Print the delimeter of intuited --bool values under --get-regexp

From: Alex Vandiver <alex@chmrr.net>

Keys with no explicit value set may still have values that need to be printed, if used in conjunction with --bool, for example. Defer printing the key-value delimeter until we know what value, if any, we intend to output.

Signed-off-by: Alex Vandiver <alex@chmrr.net>
---

I found a couple tuits under the couch; perhaps something like the following would be correct.

 builtin/config.c |   20 ++++++++++++--------
 1 files changed, 12 insertions(+), 8 deletions(-)
Show changes to builtin/config.c +12 −8
diff --git a/builtin/config.c b/builtin/config.c
index ca4a0db..98fd1ba 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -108,12 +108,9 @@ static int show_config(const char *key_, const char *value_, void *cb)
 	    (do_not_match ^ !!regexec(regexp, (value_?value_:""), 0, NULL, 0)))
 		return 0;
 
-	if (show_keys) {
-		if (value_)
-			printf("%s%c", key_, key_delim);
-		else
-			printf("%s", key_);
-	}
+	if (show_keys)
+		printf("%s", key_);
+
 	if (seen && !do_all)
 		dup_error = 1;
 	if (types == TYPE_INT)
@@ -132,14 +129,21 @@ static int show_config(const char *key_, const char *value_, void *cb)
 		must_free_vptr = 1;
 	}
 	else
-		vptr = value_?value_:"";
+		vptr = value_;
+
+	if (show_keys && vptr)
+		printf("%c", key_delim);
+
 	seen++;
 	if (dup_error) {
 		error("More than one value for the key %s: %s",
 				key_, vptr);
 	}
-	else
+	else if (vptr)
 		printf("%s%c", vptr, term);
+	else
+		printf("%c", 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

-- 
1.7.2.2.458.g8d9c8
Jonathan Nieder· Aug 27, 2010, 03:17 UTC · re: alex@chmrr.net · lore

Re: [PATCH] config: Print the delimeter of intuited --bool values under --get-regexp

Alex Vandiver wrote:
Show 6 quoted lines
> @@ -132,14 +129,21 @@ static int show_config(const char *key_, const char *value_, void *cb)
>  		must_free_vptr = 1;
>  	}
>  	else
> -		vptr = value_?value_:"";
> +		vptr = value_;
[...]
>  	if (dup_error) {
>  		error("More than one value for the key %s: %s",
>  				key_, vptr);
Could this segfault?
Jonathan Nieder· Aug 27, 2010, 03:15 UTC · re: Alex Vandiver · lore

Re: [PATCH] Test the interaction of --bool and --get-regexp on a key with no explicit value

Alex Vandiver wrote:
Show 8 quoted lines
> Signed-off-by: Alex Vandiver <alex@chmrr.net>
> ---
> 
> This test currently fails -- I don't have time to track down the fix,
> but I figured that at least I could provide a failing test.  Note that
> --null isn't necessary for the failure; without it, one gets
> "section.sub=section.val5true", with no spacing between the key and
> value.

This is useful info (the motivation!) for the log mesage I think, though it might be even better to squash the patch with the fix. :)

Show 15 quoted lines
> --- a/t/t1300-repo-config.sh
> +++ b/t/t1300-repo-config.sh
> @@ -816,6 +816,16 @@ echo >>result
>  
>  test_expect_success '--null --get-regexp' 'cmp result expect'
>  
> +cat > expect <<\EOF
> +section.sub=section.val5
> +trueQ
> +EOF
> +
> +git config --null --bool --get-regexp 'val5' | perl -pe 'y/\000/Q/' > result
> +echo >>result
> +
> +test_expect_success '--null --get-regexp --bool' 'cmp result expect'
Style nitpicks (based on the "Do's, don'ts &" c section of t/README):
 - setup code (cat >expect) belongs in the body of tests.
   That way, it is clearer when each test begins and ends, and
   unexpected failures and output from setup code will be
   automatically noticed and suppressed, respectively.
 - there is no public nul_to_q () function but I still think it
   is clearer to use one;
 - test_cmp is guaranteed to work just as well as cmp and produces
   nicer output when tests run with "-v" fail.
So maybe:
 test_expect_failure '--get-regexp --bool still separates key and value' '
	echo 'section.sub=section.val5 true' >expected &&
	git config --bool --get-regexp val5 >actual &&
	test_cmp expected acutal
 '
 test_expect_failure '--get-regexp --bool --null' '
	printf "%s\n%s\0" "section.sub=section.val5" "true" >expected &&
	git config -z --bool --get-regexp val5 >actual &&
	test_cmp expected actual
 '
Thanks for reporting.

← back to recent threads