[add-default-config 1/5] add --color option to git config

[add-default-config 1/5] add --color option to git config

[Soukaina NAIT HMID]

From: Soukaina NAIT HMID <snaithmid@bloomberg.net>

Signed-off-by: Soukaina NAIT HMID <snaithmid@bloomberg.net>
---
 Documentation/git-config.txt | 4 ++++
 builtin/config.c             | 1 +
 2 files changed, 5 insertions(+)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 4edd09fc6b074..5d5cd58fdae37 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -179,6 +179,10 @@ See also <<FILES>>.
 	specified user.  This option has no effect when setting the
 	value (but you can use `git config section.variable ~/`
 	from the command line to let your shell do the expansion).
+--color::
+	Find the color configured for `name` (e.g. `color.diff.new`) and
+	output it as the ANSI color escape sequence to the standard
+	output. 
 
 -z::
 --null::
diff --git a/builtin/config.c b/builtin/config.c
index d13daeeb55927..124a682d50fa8 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -80,6 +80,7 @@ static struct option builtin_config_options[] = {
 	OPT_BIT(0, "int", &types, N_("value is decimal number"), TYPE_INT),
 	OPT_BIT(0, "bool-or-int", &types, N_("value is --bool or --int"), TYPE_BOOL_OR_INT),
 	OPT_BIT(0, "path", &types, N_("value is a path (file or directory name)"), TYPE_PATH),
+	OPT_BIT(0, "color", &actions, N_("find the color configured"), ACTION_GET_COLOR),
 	OPT_GROUP(N_("Other")),
 	OPT_BOOL('z', "null", &end_null, N_("terminate values with NUL byte")),
 	OPT_BOOL(0, "name-only", &omit_values, N_("show variable names only")),

--
https://github.com/git/git/pull/431

[add-default-config 5/5] fix return code on default + add tests

[Soukaina NAIT HMID]

From: Soukaina NAIT HMID <snaithmid@bloomberg.net>

Signed-off-by: Soukaina NAIT HMID <snaithmid@bloomberg.net>
---
 builtin/config.c   |   9 ++-
 t/t9904-default.sh | 232 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 238 insertions(+), 3 deletions(-)
 create mode 100755 t/t9904-default.sh

diff --git a/builtin/config.c b/builtin/config.c
index eab81c5627091..29c5f55f27a57 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -261,9 +261,12 @@ static int get_value(const char *key_, const char *regex_)
 
 	if (values.nr == 0 && default_value) {
 		if(types == TYPE_INT || types == TYPE_BOOL || types == TYPE_BOOL_OR_INT || types == TYPE_PATH ) {
-			char* xstr = normalize_value(key, default_value);
-			fwrite(xstr, 1, strlen(xstr), stdout);
-			fwrite("\n", 1, 1, stdout);
+			if(strlen(default_value)) {
+				char* xstr = normalize_value(key, default_value);
+				fwrite(xstr, 1, strlen(xstr), stdout);
+				fwrite("\n", 1, 1, stdout);
+				ret = 0;
+			}
 		}
 	}
 	free(values.items);
diff --git a/t/t9904-default.sh b/t/t9904-default.sh
new file mode 100755
index 0000000000000..8e838f512298b
--- /dev/null
+++ b/t/t9904-default.sh
@@ -0,0 +1,232 @@
+#!/bin/sh
+
+test_description='Test default color/boolean/int/path'
+. ./test-lib.sh
+
+boolean()
+{
+	slot=$([ "$#" == 3 ] && echo $3 || echo "no.such.slot") &&
+	actual=$(git config --default "$1" --bool "$slot") &&
+	test "$actual" = "$2"
+}
+
+invalid_boolean()
+{
+	slot=$([ "$#" == 2 ] && echo $2 || echo "no.such.slot") &&
+	test_must_fail git config --default "$1" --bool "$slot"
+}
+
+test_expect_success 'empty value for boolean' '
+	invalid_boolean ""
+'
+
+test_expect_success 'true' '
+	boolean "true" "true"
+'
+
+test_expect_success '1 is true' '
+	boolean "1" "true"
+'
+
+test_expect_success 'non-zero is true' '
+	boolean "5312" "true"
+'
+
+test_expect_success 'false' '
+	boolean "false" "false"
+'
+
+test_expect_success '0 is false' '
+	boolean "0" "false"
+'
+
+test_expect_success 'invalid value' '
+	invalid_boolean "ab"
+'
+
+test_expect_success 'existing slot has priority = true' '
+	git config bool.value true &&
+	boolean "false" "true" "bool.value"
+'
+
+test_expect_success 'existing slot has priority = false' '
+	git config bool.value false &&
+	boolean "true" "false" "bool.value"
+'
+
+int()
+{
+	slot=$([ "$#" == 3 ] && echo $3 || echo "no.such.slot") &&
+	actual=$(git config --default "$1" --int "$slot") &&
+	test "$actual" = "$2"
+}
+
+invalid_int()
+{
+	slot=$([ "$#" == 2 ] && echo $2 || echo "no.such.slot") &&
+	test_must_fail git config "$1" --int "$slot"
+}
+
+test_expect_success 'empty value for int' '
+	invalid_int "" ""
+'
+
+test_expect_success 'positive' '
+	int "12345" "12345"
+'
+
+test_expect_success 'negative' '
+	int "-679032" "-679032"
+'
+
+test_expect_success 'invalid value' '
+	invalid_int "abc"
+'
+test_expect_success 'existing slot has priority = 123' '
+	git config int.value 123 &&
+	int "666" "123" "int.value"
+'
+
+test_expect_success 'existing slot with bad value' '
+	git config int.value abc &&
+	invalid_int "123" "int.value"
+'
+
+path()
+{
+	slot=$([ "$#" == 3 ] && echo $3 || echo "no.such.slot") &&
+	actual=$(git config --default "$1" --path "$slot") &&
+	test "$actual" = "$2"
+}
+
+invalid_path()
+{
+	slot=$([ "$#" == 2 ] && echo $2 || echo "no.such.slot") &&
+	test_must_fail git config "$1" --path "$slot"
+}
+
+test_expect_success 'empty path is invalid' '
+	invalid_path "" ""
+'
+
+test_expect_success 'valid path' '
+	path "/aa/bb/cc" "/aa/bb/cc"
+'
+
+test_expect_success 'existing slot has priority = /to/the/moon' '
+	git config path.value /to/the/moon &&
+	path "/to/the/sun" "/to/the/moon" "path.value"
+'
+ESC=$(printf '\033')
+
+color()
+{
+	slot=$([ "$#" == 3 ] && echo $3 || echo "no.such.slot") &&
+	actual=$(git config --default "$1" --color "$slot" ) &&
+	test "$actual" = "${2:+$ESC}$2"
+}
+
+invalid_color()
+{
+	slot=$([ "$#" == 2 ] && echo $2 || echo "no.such.slot") &&
+	test_must_fail git config --default "$1" --color "$slot"
+}
+
+test_expect_success 'reset' '
+	color "reset" "[m"
+'
+
+test_expect_success 'empty color is empty' '
+	color "" ""
+'
+
+test_expect_success 'attribute before color name' '
+	color "bold red" "[1;31m"
+'
+
+test_expect_success 'color name before attribute' '
+	color "red bold" "[1;31m"
+'
+
+test_expect_success 'attr fg bg' '
+	color "ul blue red" "[4;34;41m"
+'
+
+test_expect_success 'fg attr bg' '
+	color "blue ul red" "[4;34;41m"
+'
+
+test_expect_success 'fg bg attr' '
+	color "blue red ul" "[4;34;41m"
+'
+
+test_expect_success 'fg bg attr...' '
+	color "blue bold dim ul blink reverse" "[1;2;4;5;7;34m"
+'
+
+# note that nobold and nodim are the same code (22)
+test_expect_success 'attr negation' '
+	color "nobold nodim noul noblink noreverse" "[22;24;25;27m"
+'
+
+test_expect_success '"no-" variant of negation' '
+	color "no-bold no-blink" "[22;25m"
+'
+
+test_expect_success 'long color specification' '
+	color "254 255 bold dim ul blink reverse" "[1;2;4;5;7;38;5;254;48;5;255m"
+'
+
+test_expect_success 'absurdly long color specification' '
+	color \
+	  "#ffffff #ffffff bold nobold dim nodim italic noitalic
+	   ul noul blink noblink reverse noreverse strike nostrike" \
+	  "[1;2;3;4;5;7;9;22;23;24;25;27;29;38;2;255;255;255;48;2;255;255;255m"
+'
+
+test_expect_success '0-7 are aliases for basic ANSI color names' '
+	color "0 7" "[30;47m"
+'
+
+test_expect_success '256 colors' '
+	color "254 bold 255" "[1;38;5;254;48;5;255m"
+'
+
+test_expect_success '24-bit colors' '
+	color "#ff00ff black" "[38;2;255;0;255;40m"
+'
+
+test_expect_success '"normal" yields no color at all"' '
+	color "normal black" "[40m"
+'
+
+test_expect_success '-1 is a synonym for "normal"' '
+	color "-1 black" "[40m"
+'
+
+test_expect_success 'color too small' '
+	invalid_color "-2"
+'
+
+test_expect_success 'color too big' '
+	invalid_color "256"
+'
+
+test_expect_success 'extra character after color number' '
+	invalid_color "3X"
+'
+
+test_expect_success 'extra character after color name' '
+	invalid_color "redX"
+'
+
+test_expect_success 'extra character after attribute' '
+	invalid_color "dimX"
+'
+
+test_expect_success 'existing slot has priority = red' '
+	git config color.value red &&
+	path "blue" "red" "color.value"
+'
+
+test_done

--
https://github.com/git/git/pull/431

Re: [add-default-config 5/5] fix return code on default + add tests

[Jeff King]

On Sun, Nov 12, 2017 at 03:00:40PM +0000, Soukaina NAIT HMID wrote:

> diff --git a/builtin/config.c b/builtin/config.c
> index eab81c5627091..29c5f55f27a57 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -261,9 +261,12 @@ static int get_value(const char *key_, const char *regex_)
>  
>  	if (values.nr == 0 && default_value) {
>  		if(types == TYPE_INT || types == TYPE_BOOL || types == TYPE_BOOL_OR_INT || types == TYPE_PATH ) {
> -			char* xstr = normalize_value(key, default_value);
> -			fwrite(xstr, 1, strlen(xstr), stdout);
> -			fwrite("\n", 1, 1, stdout);
> +			if(strlen(default_value)) {
> +				char* xstr = normalize_value(key, default_value);
> +				fwrite(xstr, 1, strlen(xstr), stdout);
> +				fwrite("\n", 1, 1, stdout);
> +				ret = 0;
> +			}
>  		}

OK, fixing up the return value is a good thing (though again, I think if
we place our default_value in the list earlier that will just fall out
naturally).

I'm not sure why we care if the default_value string is empty or not. It
should be allowed to default to an empty string, I'd think.

> diff --git a/t/t9904-default.sh b/t/t9904-default.sh
> new file mode 100755
> index 0000000000000..8e838f512298b
> --- /dev/null
> +++ b/t/t9904-default.sh

We usually try to group tests with similar-numbered ones. Most of the
config tests are in the t13xx area. Probably "t1310-config-default.sh"
would be the right place (or if there really are just a few tests, which
I think may be all we need, they can just go into t1300).

> +boolean()
> +{
> +	slot=$([ "$#" == 3 ] && echo $3 || echo "no.such.slot") &&
> +	actual=$(git config --default "$1" --bool "$slot") &&
> +	test "$actual" = "$2"
> +}

A minor style nit, but we usually prefer "test" instead of "[" for
conditionals. It took me a while to figure out how this function was
meant to be used. It might be worth adding a comment. Though most of it
was due to the first line, which I think can just be written as:

  slot=${3:-no.such.slot}

(or you could even just write that directly in the second line).

That's a bit more idiomatic for our shell scripts.

> +test_expect_success 'empty value for boolean' '
> +	invalid_boolean ""
> +'

There are a lot of tests here about type interpretation, but I think
that should be largely orthogonal to the --default feature. Once it's
written in a way that's independent of the type, I think we can assume
that if "--default" works for one type, it should work with others
without being exhaustive.

So I think what we really want to test from this series is:

  1. --default kicks in when no matching config is found

  2. --default does not kick in when config _is_ found

  3. (optional) we complain about --default with non-get actions

  4. --color works as a type for "get" operations

  5. --color is not normalized for "set" operations; if you do:

       git config --color some.key red

     we should write "red" into the config file, not the ANSI codes.

I know the reason you were looking into t4026 originally because it was
the only spot that used --get-color in the whole test suite. But its use
of "--get-color" is largely orthogonal to what it's testing. It cares
about parsing the specific colors, but just didn't have another easy way
to convince Git to parse a bunch of colors without having to pick the
results out of diff or log output.

I'd be OK with converting that to use "--color --default" instead of
--get-color, but if we do so we should make sure that there's some
coverage of "--get-color" elsewhere in the config tests (not checking
every possible color variation, but just making sure that it can
actually look up any color with it).

-Peff

[add-default-config 4/5] add defaults for path/int/bool

[Soukaina NAIT HMID]

From: Soukaina NAIT HMID <snaithmid@bloomberg.net>

Signed-off-by: Soukaina NAIT HMID <snaithmid@bloomberg.net>
---
 builtin/config.c  |  12 ++++-
 t/t4026-color2.sh | 129 ------------------------------------------------------
 2 files changed, 11 insertions(+), 130 deletions(-)
 delete mode 100755 t/t4026-color2.sh

diff --git a/builtin/config.c b/builtin/config.c
index 9df2d9c43bcad..eab81c5627091 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -55,6 +55,8 @@ static const char *default_value;
 #define TYPE_BOOL_OR_INT (1<<2)
 #define TYPE_PATH (1<<3)
 
+static char *normalize_value(const char *key, const char *value);
+
 static struct option builtin_config_options[] = {
 	OPT_GROUP(N_("Config file location")),
 	OPT_BOOL(0, "global", &use_global_config, N_("use global config file")),
@@ -256,8 +258,15 @@ static int get_value(const char *key_, const char *regex_)
 			fwrite(buf->buf, 1, buf->len, stdout);
 		strbuf_release(buf);
 	}
-	free(values.items);
 
+	if (values.nr == 0 && default_value) {
+		if(types == TYPE_INT || types == TYPE_BOOL || types == TYPE_BOOL_OR_INT || types == TYPE_PATH ) {
+			char* xstr = normalize_value(key, default_value);
+			fwrite(xstr, 1, strlen(xstr), stdout);
+			fwrite("\n", 1, 1, stdout);
+		}
+	}
+	free(values.items);
 free_strings:
 	free(key);
 	if (key_regexp) {
@@ -272,6 +281,7 @@ static int get_value(const char *key_, const char *regex_)
 	return ret;
 }
 
+
 static char *normalize_value(const char *key, const char *value)
 {
 	if (!value)
diff --git a/t/t4026-color2.sh b/t/t4026-color2.sh
deleted file mode 100755
index 695ce9dd6f8d4..0000000000000
--- a/t/t4026-color2.sh
+++ /dev/null
@@ -1,129 +0,0 @@
-#!/bin/sh
-#
-# Copyright (c) 2008 Timo Hirvonen
-#
-
-test_description='Test diff/status color escape codes'
-. ./test-lib.sh
-
-ESC=$(printf '\033')
-color()
-{
-	actual=$(git config --default "$1" --color no.such.slot) &&
-	test "$actual" = "${2:+$ESC}$2"
-}
-
-invalid_color()
-{
-	test_must_fail git config --get-color no.such.slot "$1"
-}
-
-test_expect_success 'reset' '
-	color "reset" "[m"
-'
-
-test_expect_success 'empty color is empty' '
-	color "" ""
-'
-
-test_expect_success 'attribute before color name' '
-	color "bold red" "[1;31m"
-'
-
-test_expect_success 'color name before attribute' '
-	color "red bold" "[1;31m"
-'
-
-test_expect_success 'attr fg bg' '
-	color "ul blue red" "[4;34;41m"
-'
-
-test_expect_success 'fg attr bg' '
-	color "blue ul red" "[4;34;41m"
-'
-
-test_expect_success 'fg bg attr' '
-	color "blue red ul" "[4;34;41m"
-'
-
-test_expect_success 'fg bg attr...' '
-	color "blue bold dim ul blink reverse" "[1;2;4;5;7;34m"
-'
-
-# note that nobold and nodim are the same code (22)
-test_expect_success 'attr negation' '
-	color "nobold nodim noul noblink noreverse" "[22;24;25;27m"
-'
-
-test_expect_success '"no-" variant of negation' '
-	color "no-bold no-blink" "[22;25m"
-'
-
-test_expect_success 'long color specification' '
-	color "254 255 bold dim ul blink reverse" "[1;2;4;5;7;38;5;254;48;5;255m"
-'
-
-test_expect_success 'absurdly long color specification' '
-	color \
-	  "#ffffff #ffffff bold nobold dim nodim italic noitalic
-	   ul noul blink noblink reverse noreverse strike nostrike" \
-	  "[1;2;3;4;5;7;9;22;23;24;25;27;29;38;2;255;255;255;48;2;255;255;255m"
-'
-
-test_expect_success '0-7 are aliases for basic ANSI color names' '
-	color "0 7" "[30;47m"
-'
-
-test_expect_success '256 colors' '
-	color "254 bold 255" "[1;38;5;254;48;5;255m"
-'
-
-test_expect_success '24-bit colors' '
-	color "#ff00ff black" "[38;2;255;0;255;40m"
-'
-
-test_expect_success '"normal" yields no color at all"' '
-	color "normal black" "[40m"
-'
-
-test_expect_success '-1 is a synonym for "normal"' '
-	color "-1 black" "[40m"
-'
-
-test_expect_success 'color too small' '
-	invalid_color "-2"
-'
-
-test_expect_success 'color too big' '
-	invalid_color "256"
-'
-
-test_expect_success 'extra character after color number' '
-	invalid_color "3X"
-'
-
-test_expect_success 'extra character after color name' '
-	invalid_color "redX"
-'
-
-test_expect_success 'extra character after attribute' '
-	invalid_color "dimX"
-'
-
-test_expect_success 'unknown color slots are ignored (diff)' '
-	git config color.diff.nosuchslotwilleverbedefined white &&
-	git diff --color
-'
-
-test_expect_success 'unknown color slots are ignored (branch)' '
-	git config color.branch.nosuchslotwilleverbedefined white &&
-	git branch -a
-'
-
-test_expect_success 'unknown color slots are ignored (status)' '
-	git config color.status.nosuchslotwilleverbedefined white &&
-	{ git status; ret=$?; } &&
-	case $ret in 0|1) : ok ;; *) false ;; esac
-'
-
-test_done

--
https://github.com/git/git/pull/431

Re: [add-default-config 4/5] add defaults for path/int/bool

[Jeff King]

On Sun, Nov 12, 2017 at 03:00:40PM +0000, Soukaina NAIT HMID wrote:

> @@ -256,8 +258,15 @@ static int get_value(const char *key_, const char *regex_)
>  			fwrite(buf->buf, 1, buf->len, stdout);
>  		strbuf_release(buf);
>  	}
> -	free(values.items);
>  
> +	if (values.nr == 0 && default_value) {
> +		if(types == TYPE_INT || types == TYPE_BOOL || types == TYPE_BOOL_OR_INT || types == TYPE_PATH ) {
> +			char* xstr = normalize_value(key, default_value);
> +			fwrite(xstr, 1, strlen(xstr), stdout);
> +			fwrite("\n", 1, 1, stdout);
> +		}
> +	}
> +	free(values.items);

OK, this location makes more sense to me for handling --default. I think
it's still a bit lower in the function than I'd expect, though.

The loop just above the code you added is showing all of the values we
found. So I think that's the spot where'd say "we didn't find any
values; pretend like we found default_value". Including, I think, making
sure that the return value from the function is still 0. So realy right
after config_with_options() returns, I'd expect to check something like:

  if (!values.nr && default_value) {
          /* insert default_value into values list */
  }

We'd also want to use format_config(), not normalize_config(). We do
format_config() to show values, whereas normalize_config() is usually
for values we're putting into a config file (so for example TYPE_PATH
doesn't get normalized, but we would want it converted here to show the
user).

I'm also not sure that we need to check "types" as you have here.
Wouldn't we want to apply the default regardless of type, and let
format_config() handle it?

> @@ -272,6 +281,7 @@ static int get_value(const char *key_, const char *regex_)
>  	return ret;
>  }
>  
> +
>  static char *normalize_value(const char *key, const char *value)

Watch out for stray changes like this one creeping into your commits.

> diff --git a/t/t4026-color2.sh b/t/t4026-color2.sh
> deleted file mode 100755
> index 695ce9dd6f8d4..0000000000000

This part is obviously good and rectifying the problem from patch 3.
Once they're squashed together, we won't see it at all. :)

-Peff

[add-default-config 3/5] add same test for new command format with --default and --color

[Soukaina NAIT HMID]

From: Soukaina NAIT HMID <snaithmid@bloomberg.net>

Signed-off-by: Soukaina NAIT HMID <snaithmid@bloomberg.net>
---
 t/t4026-color2.sh | 129 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 129 insertions(+)
 create mode 100755 t/t4026-color2.sh

diff --git a/t/t4026-color2.sh b/t/t4026-color2.sh
new file mode 100755
index 0000000000000..695ce9dd6f8d4
--- /dev/null
+++ b/t/t4026-color2.sh
@@ -0,0 +1,129 @@
+#!/bin/sh
+#
+# Copyright (c) 2008 Timo Hirvonen
+#
+
+test_description='Test diff/status color escape codes'
+. ./test-lib.sh
+
+ESC=$(printf '\033')
+color()
+{
+	actual=$(git config --default "$1" --color no.such.slot) &&
+	test "$actual" = "${2:+$ESC}$2"
+}
+
+invalid_color()
+{
+	test_must_fail git config --get-color no.such.slot "$1"
+}
+
+test_expect_success 'reset' '
+	color "reset" "[m"
+'
+
+test_expect_success 'empty color is empty' '
+	color "" ""
+'
+
+test_expect_success 'attribute before color name' '
+	color "bold red" "[1;31m"
+'
+
+test_expect_success 'color name before attribute' '
+	color "red bold" "[1;31m"
+'
+
+test_expect_success 'attr fg bg' '
+	color "ul blue red" "[4;34;41m"
+'
+
+test_expect_success 'fg attr bg' '
+	color "blue ul red" "[4;34;41m"
+'
+
+test_expect_success 'fg bg attr' '
+	color "blue red ul" "[4;34;41m"
+'
+
+test_expect_success 'fg bg attr...' '
+	color "blue bold dim ul blink reverse" "[1;2;4;5;7;34m"
+'
+
+# note that nobold and nodim are the same code (22)
+test_expect_success 'attr negation' '
+	color "nobold nodim noul noblink noreverse" "[22;24;25;27m"
+'
+
+test_expect_success '"no-" variant of negation' '
+	color "no-bold no-blink" "[22;25m"
+'
+
+test_expect_success 'long color specification' '
+	color "254 255 bold dim ul blink reverse" "[1;2;4;5;7;38;5;254;48;5;255m"
+'
+
+test_expect_success 'absurdly long color specification' '
+	color \
+	  "#ffffff #ffffff bold nobold dim nodim italic noitalic
+	   ul noul blink noblink reverse noreverse strike nostrike" \
+	  "[1;2;3;4;5;7;9;22;23;24;25;27;29;38;2;255;255;255;48;2;255;255;255m"
+'
+
+test_expect_success '0-7 are aliases for basic ANSI color names' '
+	color "0 7" "[30;47m"
+'
+
+test_expect_success '256 colors' '
+	color "254 bold 255" "[1;38;5;254;48;5;255m"
+'
+
+test_expect_success '24-bit colors' '
+	color "#ff00ff black" "[38;2;255;0;255;40m"
+'
+
+test_expect_success '"normal" yields no color at all"' '
+	color "normal black" "[40m"
+'
+
+test_expect_success '-1 is a synonym for "normal"' '
+	color "-1 black" "[40m"
+'
+
+test_expect_success 'color too small' '
+	invalid_color "-2"
+'
+
+test_expect_success 'color too big' '
+	invalid_color "256"
+'
+
+test_expect_success 'extra character after color number' '
+	invalid_color "3X"
+'
+
+test_expect_success 'extra character after color name' '
+	invalid_color "redX"
+'
+
+test_expect_success 'extra character after attribute' '
+	invalid_color "dimX"
+'
+
+test_expect_success 'unknown color slots are ignored (diff)' '
+	git config color.diff.nosuchslotwilleverbedefined white &&
+	git diff --color
+'
+
+test_expect_success 'unknown color slots are ignored (branch)' '
+	git config color.branch.nosuchslotwilleverbedefined white &&
+	git branch -a
+'
+
+test_expect_success 'unknown color slots are ignored (status)' '
+	git config color.status.nosuchslotwilleverbedefined white &&
+	{ git status; ret=$?; } &&
+	case $ret in 0|1) : ok ;; *) false ;; esac
+'
+
+test_done

--
https://github.com/git/git/pull/431

Re: [add-default-config 3/5] add same test for new command format with --default and --color

[Jeff King]

On Sun, Nov 12, 2017 at 03:00:40PM +0000, Soukaina NAIT HMID wrote:

> ---
>  t/t4026-color2.sh | 129 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 129 insertions(+)
>  create mode 100755 t/t4026-color2.sh

I'll skip reviewing this one, since the file goes away later in the
series.

-Peff

[add-default-config 2/5] adding default to color

[Soukaina NAIT HMID]

From: Soukaina NAIT HMID <snaithmid@bloomberg.net>

Signed-off-by: Soukaina NAIT HMID <snaithmid@bloomberg.net>
---
 builtin/config.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/builtin/config.c b/builtin/config.c
index 124a682d50fa8..9df2d9c43bcad 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -30,6 +30,7 @@ static int end_null;
 static int respect_includes_opt = -1;
 static struct config_options config_options;
 static int show_origin;
+static const char *default_value;
 
 #define ACTION_GET (1<<0)
 #define ACTION_GET_ALL (1<<1)
@@ -47,6 +48,7 @@ static int show_origin;
 #define ACTION_GET_COLOR (1<<13)
 #define ACTION_GET_COLORBOOL (1<<14)
 #define ACTION_GET_URLMATCH (1<<15)
+#define ACTION_GET_COLORORDEFAULT (1<<16)
 
 #define TYPE_BOOL (1<<0)
 #define TYPE_INT (1<<1)
@@ -80,12 +82,13 @@ static struct option builtin_config_options[] = {
 	OPT_BIT(0, "int", &types, N_("value is decimal number"), TYPE_INT),
 	OPT_BIT(0, "bool-or-int", &types, N_("value is --bool or --int"), TYPE_BOOL_OR_INT),
 	OPT_BIT(0, "path", &types, N_("value is a path (file or directory name)"), TYPE_PATH),
-	OPT_BIT(0, "color", &actions, N_("find the color configured"), ACTION_GET_COLOR),
+	OPT_BIT(0, "color", &actions, N_("find the color configured"), ACTION_GET_COLORORDEFAULT),
 	OPT_GROUP(N_("Other")),
 	OPT_BOOL('z', "null", &end_null, N_("terminate values with NUL byte")),
 	OPT_BOOL(0, "name-only", &omit_values, N_("show variable names only")),
 	OPT_BOOL(0, "includes", &respect_includes_opt, N_("respect include directives on lookup")),
 	OPT_BOOL(0, "show-origin", &show_origin, N_("show origin of config (file, standard input, blob, command line)")),
+	OPT_STRING(0, "default", &default_value, N_("default-value"), N_("sets default for bool/int/path/color when no value is returned from config")),
 	OPT_END(),
 };
 
@@ -331,6 +334,11 @@ static void get_color(const char *var, const char *def_color)
 	fputs(parsed_color, stdout);
 }
 
+static void get_color_default(const char *var)
+{
+	get_color(var, default_value);
+}
+
 static int get_colorbool_found;
 static int get_diff_color_found;
 static int get_color_ui_found;
@@ -726,6 +734,10 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		check_argc(argc, 1, 2);
 		get_color(argv[0], argv[1]);
 	}
+	else if (actions == ACTION_GET_COLORORDEFAULT) {
+		check_argc(argc, 1, 1);
+		get_color_default(argv[0]);
+	}
 	else if (actions == ACTION_GET_COLORBOOL) {
 		check_argc(argc, 1, 2);
 		if (argc == 2)

--
https://github.com/git/git/pull/431

Re: [add-default-config 2/5] adding default to color

[Jeff King]

On Sun, Nov 12, 2017 at 03:00:40PM +0000, Soukaina NAIT HMID wrote:

> diff --git a/builtin/config.c b/builtin/config.c
> index 124a682d50fa8..9df2d9c43bcad 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -30,6 +30,7 @@ static int end_null;
>  static int respect_includes_opt = -1;
>  static struct config_options config_options;
>  static int show_origin;
> +static const char *default_value;
> [...]
> +	OPT_STRING(0, "default", &default_value, N_("default-value"), N_("sets default for bool/int/path/color when no value is returned from config")),

These hunks make sense. We're adding a new "--default" option that would
kick in when you try to look up a key and it isn't present.

I think we can skip the "bool/int/path/color" thing in the help string.
We would want this to kick in for every type, right?  The only
constraint is that we are doing a "get" operation. It wouldn't make any
sense to use "--default" when setting a variable, listing, etc. Should
we catch these cases and return an error?

We'd also want to mention this in Documentation/git-config.txt.

> @@ -47,6 +48,7 @@ static int show_origin;
>  #define ACTION_GET_COLOR (1<<13)
>  #define ACTION_GET_COLORBOOL (1<<14)
>  #define ACTION_GET_URLMATCH (1<<15)
> +#define ACTION_GET_COLORORDEFAULT (1<<16)

I'm not sure I understand this part, though. Providing a default should
be something that goes along with a "get" action, but isn't its own
action.

> +static void get_color_default(const char *var)
> +{
> +	get_color(var, default_value);
> +}
> +

And here we're just applying --default to colors, but we'd eventually
want them for everything. I think that's fixed later in the series, so
I'll keep reading. But I'd expect a function like get_value() to be
detecting the case where we got no hits and filling in the default_value
there, as if we had read it from the config file.

-Peff

Re: [add-default-config 2/5] adding default to color

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

>> @@ -47,6 +48,7 @@ static int show_origin;
>>  #define ACTION_GET_COLOR (1<<13)
>>  #define ACTION_GET_COLORBOOL (1<<14)
>>  #define ACTION_GET_URLMATCH (1<<15)
>> +#define ACTION_GET_COLORORDEFAULT (1<<16)
>
> I'm not sure I understand this part, though. Providing a default should
> be something that goes along with a "get" action, but isn't its own
> action.

I agree that it is not.

As an aside.  Over time we accumulated quite a many actions that are
all mutually exclusive by nature.  I have a feeling that we might be
better off to move away from this implementation.  The only thing
that we are getting from the current one-bit-in-a-flag-word is that
we can name the variable "actions" (instead of "action") to pretend
as if we can be given more than one, and then having to check its
value with HAS_MULTI_BITS(actions) to confuse ourselves.

Instead, perhaps we should introduce an "enum action" that includes
ACTION_UNSPECIFIED that is the initial value for the "action"
variable, which gets set to ACTION_GET, etc. with OPT_SET_INT().  If
we really care about erroring out when given

	$ git config --add --get foo.bar

instead of the "last one wins" semantics, we can use OPT_CMDMODE.

The above is of course outside the scope of this series, and I am
not sure if it should be done as a preparatory or a follow-up
clean-up.

Re: [add-default-config 2/5] adding default to color

[Jeff King]

On Mon, Nov 13, 2017 at 12:40:16PM +0900, Junio C Hamano wrote:

> As an aside.  Over time we accumulated quite a many actions that are
> all mutually exclusive by nature.  I have a feeling that we might be
> better off to move away from this implementation.  The only thing
> that we are getting from the current one-bit-in-a-flag-word is that
> we can name the variable "actions" (instead of "action") to pretend
> as if we can be given more than one, and then having to check its
> value with HAS_MULTI_BITS(actions) to confuse ourselves.
> 
> Instead, perhaps we should introduce an "enum action" that includes
> ACTION_UNSPECIFIED that is the initial value for the "action"
> variable, which gets set to ACTION_GET, etc. with OPT_SET_INT().  If
> we really care about erroring out when given
> 
> 	$ git config --add --get foo.bar
> 
> instead of the "last one wins" semantics, we can use OPT_CMDMODE.
> 
> The above is of course outside the scope of this series, and I am
> not sure if it should be done as a preparatory or a follow-up
> clean-up.

Yes, I agree that it's a little confusing, and that an enum is a better
representation.  The TYPE constants have the same problem.

I _think_ we could use OPT_CMDMODE() for those, too. Despite the name,
there is nothing in the parse-options error message that would be
inappropriate for something that isn't a cmdmode. Though I care a lot
less about "--bool --int" reporting an error (instead of last-one-wins)
than I do about "--get --set".

-Peff

Re: [add-default-config 1/5] add --color option to git config

[Jeff King]

On Sun, Nov 12, 2017 at 03:00:40PM +0000, Soukaina NAIT HMID wrote:

> From: Soukaina NAIT HMID <snaithmid@bloomberg.net>
> 
> Signed-off-by: Soukaina NAIT HMID <snaithmid@bloomberg.net>

Hi Soukaina, and welcome to the list! Thanks for working on these
patches.

I'll go through and make inline comments on your patches, but first a
few overall issues:

 - you have five patches in your series, some of which are backing out
   changes made by the other patches. It's normal in the Git community
   to use "git rebase -i" to squash down your changes to "clean" patches
   that form a sequence. For your topic, I'd expect to see two patches
   in the end:

     1. Add a "config --default" option (and documentation and tests).

     2. Add a "config --color" type (and documentation and tests).

 - your commit messages are mostly empty. :) This is a good place to
   describe the motivation for the patch, document any design decisions,
   discuss any alternatives, etc. This is helpful for reviewers to
   understand what you're trying to achieve, and for people later who
   discover your commit from "git log".

> diff --git a/builtin/config.c b/builtin/config.c
> index d13daeeb55927..124a682d50fa8 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -80,6 +80,7 @@ static struct option builtin_config_options[] = {
>  	OPT_BIT(0, "int", &types, N_("value is decimal number"), TYPE_INT),
>  	OPT_BIT(0, "bool-or-int", &types, N_("value is --bool or --int"), TYPE_BOOL_OR_INT),
>  	OPT_BIT(0, "path", &types, N_("value is a path (file or directory name)"), TYPE_PATH),
> +	OPT_BIT(0, "color", &actions, N_("find the color configured"), ACTION_GET_COLOR),

I think we'd want this "--color" to actually be a type, not a separate
action. I.e., so that it behaves --int, --bool, etc. Part of the goal
(well, my goal) was to make accessing color config more like other types
of config.

-Peff

[add-default-config] add --default option to git config.

[Soukaina NAIT HMID]

From: Soukaina NAIT HMID <snaithmid@bloomberg.net>

Signed-off-by: Soukaina NAIT HMID <snaithmid@bloomberg.net>
---
 Documentation/git-config.txt |   4 ++
 builtin/config.c             |  34 ++++++++-
 config.c                     |  10 +++
 config.h                     |   1 +
 t/t1300-repo-config.sh       | 161 +++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 209 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 4edd09fc6b074..5d5cd58fdae37 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -179,6 +179,10 @@ See also <<FILES>>.
 	specified user.  This option has no effect when setting the
 	value (but you can use `git config section.variable ~/`
 	from the command line to let your shell do the expansion).
+--color::
+	Find the color configured for `name` (e.g. `color.diff.new`) and
+	output it as the ANSI color escape sequence to the standard
+	output. 
 
 -z::
 --null::
diff --git a/builtin/config.c b/builtin/config.c
index d13daeeb55927..5e5b998b7c892 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -30,6 +30,7 @@ static int end_null;
 static int respect_includes_opt = -1;
 static struct config_options config_options;
 static int show_origin;
+static const char *default_value;
 
 #define ACTION_GET (1<<0)
 #define ACTION_GET_ALL (1<<1)
@@ -52,6 +53,8 @@ static int show_origin;
 #define TYPE_INT (1<<1)
 #define TYPE_BOOL_OR_INT (1<<2)
 #define TYPE_PATH (1<<3)
+#define TYPE_COLOR (1<<4)
+
 
 static struct option builtin_config_options[] = {
 	OPT_GROUP(N_("Config file location")),
@@ -80,11 +83,13 @@ static struct option builtin_config_options[] = {
 	OPT_BIT(0, "int", &types, N_("value is decimal number"), TYPE_INT),
 	OPT_BIT(0, "bool-or-int", &types, N_("value is --bool or --int"), TYPE_BOOL_OR_INT),
 	OPT_BIT(0, "path", &types, N_("value is a path (file or directory name)"), TYPE_PATH),
+	OPT_BIT(0, "color", &types, N_("find the color configured"), TYPE_COLOR),
 	OPT_GROUP(N_("Other")),
 	OPT_BOOL('z', "null", &end_null, N_("terminate values with NUL byte")),
 	OPT_BOOL(0, "name-only", &omit_values, N_("show variable names only")),
 	OPT_BOOL(0, "includes", &respect_includes_opt, N_("respect include directives on lookup")),
 	OPT_BOOL(0, "show-origin", &show_origin, N_("show origin of config (file, standard input, blob, command line)")),
+	OPT_STRING(0, "default", &default_value, N_("default-value"), N_("sets default value when no value is returned from config")),
 	OPT_END(),
 };
 
@@ -159,6 +164,13 @@ static int format_config(struct strbuf *buf, const char *key_, const char *value
 				return -1;
 			strbuf_addstr(buf, v);
 			free((char *)v);
+		}
+		else if (types == TYPE_COLOR) {
+			char *v = xmalloc(COLOR_MAXLEN);
+			if (git_config_color(&v, key_, value_) < 0)
+				return -1;
+			strbuf_addstr(buf, v);
+			free((char *)v);
 		} else if (value_) {
 			strbuf_addstr(buf, value_);
 		} else {
@@ -244,8 +256,16 @@ static int get_value(const char *key_, const char *regex_)
 	config_with_options(collect_config, &values,
 			    &given_config_source, &config_options);
 
-	ret = !values.nr;
+	if (!values.nr && default_value && types) {
+		struct strbuf *item;
+		ALLOC_GROW(values.items, values.nr + 1, values.alloc);
+		item = &values.items[values.nr++];
+		if(format_config(item, key_, default_value) < 0){
+			values.nr = 0;
+		}
+	}
 
+	ret = !values.nr;
 	for (i = 0; i < values.nr; i++) {
 		struct strbuf *buf = values.items + i;
 		if (do_all || i == values.nr - 1)
@@ -268,6 +288,7 @@ static int get_value(const char *key_, const char *regex_)
 	return ret;
 }
 
+
 static char *normalize_value(const char *key, const char *value)
 {
 	if (!value)
@@ -281,6 +302,17 @@ static char *normalize_value(const char *key, const char *value)
 		 * when retrieving the value.
 		 */
 		return xstrdup(value);
+	if (types == TYPE_COLOR)
+	{
+		char *v = xmalloc(COLOR_MAXLEN);
+		if (git_config_color(&v, key, value) == 0)
+		{
+			free((char *)v);
+			return xstrdup(value);
+		}
+		free((char *)v);
+		die("cannot parse color '%s'", value);
+	}
 	if (types == TYPE_INT)
 		return xstrfmt("%"PRId64, git_config_int64(key, value));
 	if (types == TYPE_BOOL)
diff --git a/config.c b/config.c
index 903abf9533b18..5c5daffeb6723 100644
--- a/config.c
+++ b/config.c
@@ -16,6 +16,7 @@
 #include "string-list.h"
 #include "utf8.h"
 #include "dir.h"
+#include "color.h"
 
 struct config_source {
 	struct config_source *prev;
@@ -990,6 +991,15 @@ int git_config_pathname(const char **dest, const char *var, const char *value)
 	return 0;
 }
 
+int git_config_color(char **dest, const char *var, const char *value)
+{
+	if (!value)
+		return config_error_nonbool(var);
+	if (color_parse(value, *dest) < 0)
+		return -1;
+	return 0;
+}
+
 static int git_default_core_config(const char *var, const char *value)
 {
 	/* This needs a better name */
diff --git a/config.h b/config.h
index a49d264416225..8f8ca6a9b0741 100644
--- a/config.h
+++ b/config.h
@@ -72,6 +72,7 @@ extern int git_config_rename_section(const char *, const char *);
 extern int git_config_rename_section_in_file(const char *, const char *, const char *);
 extern int git_config_copy_section(const char *, const char *);
 extern int git_config_copy_section_in_file(const char *, const char *, const char *);
+extern int git_config_color(char ** dest, const char *var, const char *value);
 extern const char *git_etc_gitconfig(void);
 extern int git_env_bool(const char *, int);
 extern unsigned long git_env_ulong(const char *, unsigned long);
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 364a537000bbb..b5804ab05ee3a 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -1558,4 +1558,165 @@ test_expect_success '--local requires a repo' '
 	test_expect_code 128 nongit git config --local foo.bar
 '
 
+boolean()
+{
+	slot=${3:-no.such.slot} &&
+	actual=$(git config --default "$1" --bool "$slot") &&
+	test "$actual" = "$2"
+}
+
+invalid_boolean()
+{
+	slot=${2:-no.such.slot} &&
+	test_must_fail git config --default "$1" --bool "$slot"
+}
+
+test_expect_success 'empty value for boolean' '
+	boolean "" "false"
+'
+
+test_expect_success 'true' '
+	boolean "true" "true"
+'
+
+test_expect_success '1 is true' '
+	boolean "1" "true"
+'
+
+test_expect_success 'non-zero is true' '
+	boolean "5312" "true"
+'
+
+test_expect_success 'false' '
+	boolean "false" "false"
+'
+
+test_expect_success '0 is false' '
+	boolean "0" "false"
+'
+
+test_expect_success 'invalid value' '
+	invalid_boolean "ab"
+'
+
+test_expect_success 'existing slot has priority = true' '
+	git config bool.value true &&
+	boolean "false" "true" "bool.value"
+'
+
+test_expect_success 'existing slot has priority = false' '
+	git config bool.value false &&
+	boolean "true" "false" "bool.value"
+'
+
+int()
+{
+	slot=${3:-no.such.slot} &&
+	actual=$(git config --default "$1" --int "$slot") &&
+	test "$actual" = "$2"
+}
+
+invalid_int()
+{
+	slot=${2:-no.such.slot} &&
+	test_must_fail git config "$1" --int "$slot"
+}
+
+test_expect_success 'empty value for int' '
+	invalid_int "" ""
+'
+
+test_expect_success 'positive' '
+	int "12345" "12345"
+'
+
+test_expect_success 'negative' '
+	int "-679032" "-679032"
+'
+
+test_expect_success 'invalid value' '
+	invalid_int "abc"
+'
+test_expect_success 'existing slot has priority = 123' '
+	git config int.value 123 &&
+	int "666" "123" "int.value"
+'
+
+test_expect_success 'existing slot with bad value' '
+	git config int.value abc &&
+	invalid_int "123" "int.value"
+'
+
+path()
+{
+	slot=${3:-no.such.slot} &&
+	actual=$(git config --default "$1" --path "$slot") &&
+	test "$actual" = "$2"
+}
+
+invalid_path()
+{
+	slot=${2:-no.such.slot} &&
+	test_must_fail git config "$1" --path "$slot"
+}
+
+test_expect_success 'empty path is invalid' '
+	invalid_path "" ""
+'
+
+test_expect_success 'valid path' '
+	path "/aa/bb/cc" "/aa/bb/cc"
+'
+
+test_expect_success 'existing slot has priority = /to/the/moon' '
+	git config path.value /to/the/moon &&
+	path "/to/the/sun" "/to/the/moon" "path.value"
+'
+ESC=$(printf '\033')
+
+color()
+{
+	slot=${3:-no.such.slot} &&
+	actual=$(git config --default "$1" --color "$slot" ) &&
+	test "$actual" = "${2:+$ESC}$2"
+}
+
+invalid_color()
+{
+	slot=${2:-no.such.slot} &&
+	test_must_fail git config --default "$1" --color "$slot"
+}
+
+test_expect_success 'reset' '
+	color "reset" "[m"
+'
+
+test_expect_success 'empty color is empty' '
+	color "" ""
+'
+
+test_expect_success 'absurdly long color specification' '
+	color \
+	  "#ffffff #ffffff bold nobold dim nodim italic noitalic
+	   ul noul blink noblink reverse noreverse strike nostrike" \
+	  "[1;2;3;4;5;7;9;22;23;24;25;27;29;38;2;255;255;255;48;2;255;255;255m"
+'
+
+test_expect_success 'color too small' '
+	invalid_color "-2"
+'
+
+test_expect_success 'color too big' '
+	invalid_color "256"
+'
+
+test_expect_success 'extra character after color name' '
+	invalid_color "redX"
+'
+
+test_expect_success 'existing slot has priority = red' '
+	git config color.value red &&
+	color "blue" "[31m" "color.value"
+'
+
 test_done

--
https://github.com/git/git/pull/431

Re: [add-default-config] add --default option to git config.

[Philip Oakley]

From: "Soukaina NAIT HMID" <nhsoukaina@gmail.com>
> From: Soukaina NAIT HMID <snaithmid@bloomberg.net>
>

From a coursory read, there does need a bit more explanation.

I see you also add a --color description and code, and don't say what the 
problem being solved is.

If it is trickty to explain, then a two patch series may tease apart the 
issues. perhaps add the --color option first (noting you'll use it in the 
next patch), then a second patch that explains about the --default problem.

The patch title should be something like "[PATCH 1/n] config: add --default 
option"

You may also want to explain the test rationale, and maybe split them if 
appropriate.

--
Philip


> Signed-off-by: Soukaina NAIT HMID <snaithmid@bloomberg.net>
> ---
> Documentation/git-config.txt |   4 ++
> builtin/config.c             |  34 ++++++++-
> config.c                     |  10 +++
> config.h                     |   1 +
> t/t1300-repo-config.sh       | 161 
> +++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 209 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
> index 4edd09fc6b074..5d5cd58fdae37 100644
> --- a/Documentation/git-config.txt
> +++ b/Documentation/git-config.txt
> @@ -179,6 +179,10 @@ See also <<FILES>>.
>  specified user.  This option has no effect when setting the
>  value (but you can use `git config section.variable ~/`
>  from the command line to let your shell do the expansion).
> +--color::
> + Find the color configured for `name` (e.g. `color.diff.new`) and
> + output it as the ANSI color escape sequence to the standard
> + output.
>
> -z::
> --null::
> diff --git a/builtin/config.c b/builtin/config.c
> index d13daeeb55927..5e5b998b7c892 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -30,6 +30,7 @@ static int end_null;
> static int respect_includes_opt = -1;
> static struct config_options config_options;
> static int show_origin;
> +static const char *default_value;
>
> #define ACTION_GET (1<<0)
> #define ACTION_GET_ALL (1<<1)
> @@ -52,6 +53,8 @@ static int show_origin;
> #define TYPE_INT (1<<1)
> #define TYPE_BOOL_OR_INT (1<<2)
> #define TYPE_PATH (1<<3)
> +#define TYPE_COLOR (1<<4)
> +
>
> static struct option builtin_config_options[] = {
>  OPT_GROUP(N_("Config file location")),
> @@ -80,11 +83,13 @@ static struct option builtin_config_options[] = {
>  OPT_BIT(0, "int", &types, N_("value is decimal number"), TYPE_INT),
>  OPT_BIT(0, "bool-or-int", &types, N_("value is --bool or --int"), 
> TYPE_BOOL_OR_INT),
>  OPT_BIT(0, "path", &types, N_("value is a path (file or directory 
> name)"), TYPE_PATH),
> + OPT_BIT(0, "color", &types, N_("find the color configured"), 
> TYPE_COLOR),
>  OPT_GROUP(N_("Other")),
>  OPT_BOOL('z', "null", &end_null, N_("terminate values with NUL byte")),
>  OPT_BOOL(0, "name-only", &omit_values, N_("show variable names only")),
>  OPT_BOOL(0, "includes", &respect_includes_opt, N_("respect include 
> directives on lookup")),
>  OPT_BOOL(0, "show-origin", &show_origin, N_("show origin of config (file, 
> standard input, blob, command line)")),
> + OPT_STRING(0, "default", &default_value, N_("default-value"), N_("sets 
> default value when no value is returned from config")),
>  OPT_END(),
> };
>
> @@ -159,6 +164,13 @@ static int format_config(struct strbuf *buf, const 
> char *key_, const char *value
>  return -1;
>  strbuf_addstr(buf, v);
>  free((char *)v);
> + }
> + else if (types == TYPE_COLOR) {
> + char *v = xmalloc(COLOR_MAXLEN);
> + if (git_config_color(&v, key_, value_) < 0)
> + return -1;
> + strbuf_addstr(buf, v);
> + free((char *)v);
>  } else if (value_) {
>  strbuf_addstr(buf, value_);
>  } else {
> @@ -244,8 +256,16 @@ static int get_value(const char *key_, const char 
> *regex_)
>  config_with_options(collect_config, &values,
>      &given_config_source, &config_options);
>
> - ret = !values.nr;
> + if (!values.nr && default_value && types) {
> + struct strbuf *item;
> + ALLOC_GROW(values.items, values.nr + 1, values.alloc);
> + item = &values.items[values.nr++];
> + if(format_config(item, key_, default_value) < 0){
> + values.nr = 0;
> + }
> + }
>
> + ret = !values.nr;
>  for (i = 0; i < values.nr; i++) {
>  struct strbuf *buf = values.items + i;
>  if (do_all || i == values.nr - 1)
> @@ -268,6 +288,7 @@ static int get_value(const char *key_, const char 
> *regex_)
>  return ret;
> }
>
> +
> static char *normalize_value(const char *key, const char *value)
> {
>  if (!value)
> @@ -281,6 +302,17 @@ static char *normalize_value(const char *key, const 
> char *value)
>  * when retrieving the value.
>  */
>  return xstrdup(value);
> + if (types == TYPE_COLOR)
> + {
> + char *v = xmalloc(COLOR_MAXLEN);
> + if (git_config_color(&v, key, value) == 0)
> + {
> + free((char *)v);
> + return xstrdup(value);
> + }
> + free((char *)v);
> + die("cannot parse color '%s'", value);
> + }
>  if (types == TYPE_INT)
>  return xstrfmt("%"PRId64, git_config_int64(key, value));
>  if (types == TYPE_BOOL)
> diff --git a/config.c b/config.c
> index 903abf9533b18..5c5daffeb6723 100644
> --- a/config.c
> +++ b/config.c
> @@ -16,6 +16,7 @@
> #include "string-list.h"
> #include "utf8.h"
> #include "dir.h"
> +#include "color.h"
>
> struct config_source {
>  struct config_source *prev;
> @@ -990,6 +991,15 @@ int git_config_pathname(const char **dest, const char 
> *var, const char *value)
>  return 0;
> }
>
> +int git_config_color(char **dest, const char *var, const char *value)
> +{
> + if (!value)
> + return config_error_nonbool(var);
> + if (color_parse(value, *dest) < 0)
> + return -1;
> + return 0;
> +}
> +
> static int git_default_core_config(const char *var, const char *value)
> {
>  /* This needs a better name */
> diff --git a/config.h b/config.h
> index a49d264416225..8f8ca6a9b0741 100644
> --- a/config.h
> +++ b/config.h
> @@ -72,6 +72,7 @@ extern int git_config_rename_section(const char *, const 
> char *);
> extern int git_config_rename_section_in_file(const char *, const char *, 
> const char *);
> extern int git_config_copy_section(const char *, const char *);
> extern int git_config_copy_section_in_file(const char *, const char *, 
> const char *);
> +extern int git_config_color(char ** dest, const char *var, const char 
> *value);
> extern const char *git_etc_gitconfig(void);
> extern int git_env_bool(const char *, int);
> extern unsigned long git_env_ulong(const char *, unsigned long);
> diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
> index 364a537000bbb..b5804ab05ee3a 100755
> --- a/t/t1300-repo-config.sh
> +++ b/t/t1300-repo-config.sh
> @@ -1558,4 +1558,165 @@ test_expect_success '--local requires a repo' '
>  test_expect_code 128 nongit git config --local foo.bar
> '
>
> +boolean()
> +{
> + slot=${3:-no.such.slot} &&
> + actual=$(git config --default "$1" --bool "$slot") &&
> + test "$actual" = "$2"
> +}
> +
> +invalid_boolean()
> +{
> + slot=${2:-no.such.slot} &&
> + test_must_fail git config --default "$1" --bool "$slot"
> +}
> +
> +test_expect_success 'empty value for boolean' '
> + boolean "" "false"
> +'
> +
> +test_expect_success 'true' '
> + boolean "true" "true"
> +'
> +
> +test_expect_success '1 is true' '
> + boolean "1" "true"
> +'
> +
> +test_expect_success 'non-zero is true' '
> + boolean "5312" "true"
> +'
> +
> +test_expect_success 'false' '
> + boolean "false" "false"
> +'
> +
> +test_expect_success '0 is false' '
> + boolean "0" "false"
> +'
> +
> +test_expect_success 'invalid value' '
> + invalid_boolean "ab"
> +'
> +
> +test_expect_success 'existing slot has priority = true' '
> + git config bool.value true &&
> + boolean "false" "true" "bool.value"
> +'
> +
> +test_expect_success 'existing slot has priority = false' '
> + git config bool.value false &&
> + boolean "true" "false" "bool.value"
> +'
> +
> +int()
> +{
> + slot=${3:-no.such.slot} &&
> + actual=$(git config --default "$1" --int "$slot") &&
> + test "$actual" = "$2"
> +}
> +
> +invalid_int()
> +{
> + slot=${2:-no.such.slot} &&
> + test_must_fail git config "$1" --int "$slot"
> +}
> +
> +test_expect_success 'empty value for int' '
> + invalid_int "" ""
> +'
> +
> +test_expect_success 'positive' '
> + int "12345" "12345"
> +'
> +
> +test_expect_success 'negative' '
> + int "-679032" "-679032"
> +'
> +
> +test_expect_success 'invalid value' '
> + invalid_int "abc"
> +'
> +test_expect_success 'existing slot has priority = 123' '
> + git config int.value 123 &&
> + int "666" "123" "int.value"
> +'
> +
> +test_expect_success 'existing slot with bad value' '
> + git config int.value abc &&
> + invalid_int "123" "int.value"
> +'
> +
> +path()
> +{
> + slot=${3:-no.such.slot} &&
> + actual=$(git config --default "$1" --path "$slot") &&
> + test "$actual" = "$2"
> +}
> +
> +invalid_path()
> +{
> + slot=${2:-no.such.slot} &&
> + test_must_fail git config "$1" --path "$slot"
> +}
> +
> +test_expect_success 'empty path is invalid' '
> + invalid_path "" ""
> +'
> +
> +test_expect_success 'valid path' '
> + path "/aa/bb/cc" "/aa/bb/cc"
> +'
> +
> +test_expect_success 'existing slot has priority = /to/the/moon' '
> + git config path.value /to/the/moon &&
> + path "/to/the/sun" "/to/the/moon" "path.value"
> +'
> +ESC=$(printf '\033')
> +
> +color()
> +{
> + slot=${3:-no.such.slot} &&
> + actual=$(git config --default "$1" --color "$slot" ) &&
> + test "$actual" = "${2:+$ESC}$2"
> +}
> +
> +invalid_color()
> +{
> + slot=${2:-no.such.slot} &&
> + test_must_fail git config --default "$1" --color "$slot"
> +}
> +
> +test_expect_success 'reset' '
> + color "reset" "[m"
> +'
> +
> +test_expect_success 'empty color is empty' '
> + color "" ""
> +'
> +
> +test_expect_success 'absurdly long color specification' '
> + color \
> +   "#ffffff #ffffff bold nobold dim nodim italic noitalic
> +    ul noul blink noblink reverse noreverse strike nostrike" \
> +   "[1;2;3;4;5;7;9;22;23;24;25;27;29;38;2;255;255;255;48;2;255;255;255m"
> +'
> +
> +test_expect_success 'color too small' '
> + invalid_color "-2"
> +'
> +
> +test_expect_success 'color too big' '
> + invalid_color "256"
> +'
> +
> +test_expect_success 'extra character after color name' '
> + invalid_color "redX"
> +'
> +
> +test_expect_success 'existing slot has priority = red' '
> + git config color.value red &&
> + color "blue" "[31m" "color.value"
> +'
> +
> test_done
>
> --
> https://github.com/git/git/pull/431