[PATCH 0/5] [RFC] Make 'git config list --type=' parse and filter types

[PATCH 0/5] [RFC] Make 'git config list --type=' parse and filter types

[Derrick Stolee via GitGitGadget]

I started down this road based on feedback on my 'git config-batch' RFC [1].

[1]
https://lore.kernel.org/git/pull.2033.git.1770214803.gitgitgadget@gmail.com/

I had described my intention to use 'git config-batch' as a single process
to load multiple config values one-by-one. Brian mentioned that 'git config
list -z' would probably suffice, so I started experimenting in that
direction [2].

[2]
https://github.com/git-ecosystem/git-credential-manager/compare/main...derrickstolee:config-list

However, I ran into a problem: the most critical performance bottleneck is
related to path-formatted config values that are queried with 'git config
get --type=path -z'. It wasn't hard to update things to lazily load the full
list of config values by type [3], but I then noticed a big problem!

[3]
https://github.com/git-ecosystem/git-credential-manager/commit/d403c8e24ce6f37da920cce23842dd5a6cf6481d

Problem: 'git config list' doesn't respect --type=<X>!

This boils down to the fact that the iterator function show_all_config()
doesn't call format_config(), which includes the type-parsing code.

This wasn't super trivial to update:

 1. format_config() uses git_config_parse_*() methods, which die() on a bad
    parse.
 2. The path parsing code didn't have a gentle version.
 3. The two paths ('git config list' and 'git config --list') needed to
    standardize their display options to work with format_config().
 4. Finally, we need to filter out key-value pairs that don't match the
    given type.

This is marked as an RFC because I need to add some more tests and because
this is a behavior change! If there are any tools currently passing the
--type=<X> argument to git config list then they will have a change of
behavior with this series. It's an easy workaround: drop the --type argument
or add --no-type to go back to the previous behavior.

Thanks for any and all feedback, -Stolee

Derrick Stolee (5):
  config: move show_all_config()
  parse: add git_parse_maybe_pathname()
  config: allow format_config() to filter
  config: create special init for list mode
  config: make 'git config list --type=<X>' work

 Documentation/git-config.adoc |   3 +
 builtin/config.c              | 130 ++++++++++++++++++++++++----------
 config.c                      |  14 +---
 parse.c                       |  24 +++++++
 parse.h                       |   2 +
 t/t1300-config.sh             |  26 ++++++-
 6 files changed, 147 insertions(+), 52 deletions(-)


base-commit: 67ad42147a7acc2af6074753ebd03d904476118f
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2044%2Fderrickstolee%2Fconfig-list-type-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2044/derrickstolee/config-list-type-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2044
-- 
gitgitgadget

[PATCH 1/5] config: move show_all_config()

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

In anticipation of using format_config() in this method, move
show_all_config() lower in the file without changes.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 builtin/config.c | 48 ++++++++++++++++++++++++------------------------
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 288ebdfdaa..237f7a934d 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -231,30 +231,6 @@ static void show_config_scope(const struct config_display_options *opts,
 	strbuf_addch(buf, term);
 }
 
-static int show_all_config(const char *key_, const char *value_,
-			   const struct config_context *ctx,
-			   void *cb)
-{
-	const struct config_display_options *opts = cb;
-	const struct key_value_info *kvi = ctx->kvi;
-
-	if (opts->show_origin || opts->show_scope) {
-		struct strbuf buf = STRBUF_INIT;
-		if (opts->show_scope)
-			show_config_scope(opts, kvi, &buf);
-		if (opts->show_origin)
-			show_config_origin(opts, kvi, &buf);
-		/* Use fwrite as "buf" can contain \0's if "end_null" is set. */
-		fwrite(buf.buf, 1, buf.len, stdout);
-		strbuf_release(&buf);
-	}
-	if (!opts->omit_values && value_)
-		printf("%s%c%s%c", key_, opts->delim, value_, opts->term);
-	else
-		printf("%s%c", key_, opts->term);
-	return 0;
-}
-
 struct strbuf_list {
 	struct strbuf *items;
 	int nr;
@@ -332,6 +308,30 @@ static int format_config(const struct config_display_options *opts,
 	return 0;
 }
 
+static int show_all_config(const char *key_, const char *value_,
+			   const struct config_context *ctx,
+			   void *cb)
+{
+	const struct config_display_options *opts = cb;
+	const struct key_value_info *kvi = ctx->kvi;
+
+	if (opts->show_origin || opts->show_scope) {
+		struct strbuf buf = STRBUF_INIT;
+		if (opts->show_scope)
+			show_config_scope(opts, kvi, &buf);
+		if (opts->show_origin)
+			show_config_origin(opts, kvi, &buf);
+		/* Use fwrite as "buf" can contain \0's if "end_null" is set. */
+		fwrite(buf.buf, 1, buf.len, stdout);
+		strbuf_release(&buf);
+	}
+	if (!opts->omit_values && value_)
+		printf("%s%c%s%c", key_, opts->delim, value_, opts->term);
+	else
+		printf("%s%c", key_, opts->term);
+	return 0;
+}
+
 #define GET_VALUE_ALL        (1 << 0)
 #define GET_VALUE_KEY_REGEXP (1 << 1)
 
-- 
gitgitgadget

[PATCH 2/5] parse: add git_parse_maybe_pathname()

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

This extraction of logic from config.c's git_config_pathname() allows
for parsing a fully-qualified path from a relative path along with
validation of the existence of the path without failing with a die().

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 config.c | 14 +-------------
 parse.c  | 24 ++++++++++++++++++++++++
 parse.h  |  2 ++
 3 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/config.c b/config.c
index 7f6d53b473..83257b7a97 100644
--- a/config.c
+++ b/config.c
@@ -1278,24 +1278,12 @@ int git_config_string(char **dest, const char *var, const char *value)
 
 int git_config_pathname(char **dest, const char *var, const char *value)
 {
-	bool is_optional;
-	char *path;
-
 	if (!value)
 		return config_error_nonbool(var);
 
-	is_optional = skip_prefix(value, ":(optional)", &value);
-	path = interpolate_path(value, 0);
-	if (!path)
+	if (git_parse_maybe_pathname(value, dest) < 0)
 		die(_("failed to expand user dir in: '%s'"), value);
 
-	if (is_optional && is_missing_file(path)) {
-		free(path);
-		*dest = NULL;
-		return 0;
-	}
-
-	*dest = path;
 	return 0;
 }
 
diff --git a/parse.c b/parse.c
index 48313571aa..3f37f0b93a 100644
--- a/parse.c
+++ b/parse.c
@@ -1,6 +1,7 @@
 #include "git-compat-util.h"
 #include "gettext.h"
 #include "parse.h"
+#include "path.h"
 
 static uintmax_t get_unit_factor(const char *end)
 {
@@ -209,3 +210,26 @@ unsigned long git_env_ulong(const char *k, unsigned long val)
 		die(_("failed to parse %s"), k);
 	return val;
 }
+
+int git_parse_maybe_pathname(const char *value, char **dest)
+{
+	bool is_optional;
+	char *path;
+
+	if (!value)
+		return -1;
+
+	is_optional = skip_prefix(value, ":(optional)", &value);
+	path = interpolate_path(value, 0);
+	if (!path)
+		return -1;
+
+	if (is_optional && is_missing_file(path)) {
+		free(path);
+		*dest = NULL;
+		return 0;
+	}
+
+	*dest = path;
+	return 0;
+}
diff --git a/parse.h b/parse.h
index ea32de9a91..4f97c3727a 100644
--- a/parse.h
+++ b/parse.h
@@ -19,4 +19,6 @@ int git_parse_maybe_bool_text(const char *value);
 int git_env_bool(const char *, int);
 unsigned long git_env_ulong(const char *, unsigned long);
 
+int git_parse_maybe_pathname(const char *value, char **dest);
+
 #endif /* PARSE_H */
-- 
gitgitgadget

[PATCH 3/5] config: allow format_config() to filter

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

The format_config() method in builtin/config.c currently only uses
git_config_*() methods for parsing. This allows parsing errors to result
in die() messages appropriate with keys in the error message.

In a future change we will want to use format_config() within 'git
config list' to help format the output, including when --type=<X>
arguments are provided. When the parsing fails in that case, that
key-value pair should be omitted instead of causing a failure across the
entire command.

This change is formatted in such a way that the if/else-if structure
allows the default die_on_error version to appear first and then be
followed by the gentle parsing mode immediately afterwards.

The only callers right now have die_on_parse set to 1.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 builtin/config.c | 75 +++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 62 insertions(+), 13 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 237f7a934d..5146abcfa3 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -3,6 +3,7 @@
 #include "abspath.h"
 #include "config.h"
 #include "color.h"
+#include "date.h"
 #include "editor.h"
 #include "environment.h"
 #include "gettext.h"
@@ -245,7 +246,8 @@ struct strbuf_list {
  */
 static int format_config(const struct config_display_options *opts,
 			 struct strbuf *buf, const char *key_,
-			 const char *value_, const struct key_value_info *kvi)
+			 const char *value_, const struct key_value_info *kvi,
+			 int die_on_parse)
 {
 	if (opts->show_scope)
 		show_config_scope(opts, kvi, buf);
@@ -257,27 +259,55 @@ static int format_config(const struct config_display_options *opts,
 		if (opts->show_keys)
 			strbuf_addch(buf, opts->key_delim);
 
-		if (opts->type == TYPE_INT)
+		if (opts->type == TYPE_INT && die_on_parse) {
 			strbuf_addf(buf, "%"PRId64,
 				    git_config_int64(key_, value_ ? value_ : "", kvi));
-		else if (opts->type == TYPE_BOOL)
+		} else if (opts->type == TYPE_INT) {
+			int64_t v;
+			int ret = git_parse_int64(value_, &v);
+
+			if (ret)
+				return -1;
+
+			strbuf_addf(buf, "%"PRId64, v);
+		}
+		else if (opts->type == TYPE_BOOL && die_on_parse) {
 			strbuf_addstr(buf, git_config_bool(key_, value_) ?
 				      "true" : "false");
-		else if (opts->type == TYPE_BOOL_OR_INT) {
-			int is_bool, v;
-			v = git_config_bool_or_int(key_, value_, kvi,
-						   &is_bool);
+		} else if (opts->type == TYPE_BOOL) {
+			int value = git_parse_maybe_bool(value_);
+
+			if (value < 0)
+				return -1;
+
+			strbuf_addstr(buf, value ? "true" : "false");
+		} else if (opts->type == TYPE_BOOL_OR_INT && die_on_parse) {
+			int is_bool = 0;
+			int v = git_config_bool_or_int(key_, value_, kvi,
+						       &is_bool);
+			if (is_bool)
+				strbuf_addstr(buf, v ? "true" : "false");
+			else
+				strbuf_addf(buf, "%d", v);
+		} else if (opts->type == TYPE_BOOL_OR_INT) {
+			int is_bool = 0;
+			int v = git_parse_maybe_bool_text(value_);
+
+			if (v < 0)
+				return -1;
+
 			if (is_bool)
 				strbuf_addstr(buf, v ? "true" : "false");
 			else
 				strbuf_addf(buf, "%d", v);
 		} else if (opts->type == TYPE_BOOL_OR_STR) {
+			/* Note: this can't fail to parse! */
 			int v = git_parse_maybe_bool(value_);
 			if (v < 0)
 				strbuf_addstr(buf, value_);
 			else
 				strbuf_addstr(buf, v ? "true" : "false");
-		} else if (opts->type == TYPE_PATH) {
+		} else if (opts->type == TYPE_PATH && die_on_parse) {
 			char *v;
 			if (git_config_pathname(&v, key_, value_) < 0)
 				return -1;
@@ -286,16 +316,35 @@ static int format_config(const struct config_display_options *opts,
 			else
 				return 1; /* :(optional)no-such-file */
 			free((char *)v);
-		} else if (opts->type == TYPE_EXPIRY_DATE) {
+		} else if (opts->type == TYPE_PATH) {
+			char *v;
+			if (git_parse_maybe_pathname(value_, &v) < 0)
+				return -1;
+			if (v)
+				strbuf_addstr(buf, v);
+			else
+				return 1; /* :(optional)no-such-file */
+			free((char *)v);
+		} else if (opts->type == TYPE_EXPIRY_DATE && die_on_parse) {
 			timestamp_t t;
 			if (git_config_expiry_date(&t, key_, value_) < 0)
 				return -1;
 			strbuf_addf(buf, "%"PRItime, t);
-		} else if (opts->type == TYPE_COLOR) {
+		} else if (opts->type == TYPE_EXPIRY_DATE) {
+			timestamp_t t;
+			if (parse_expiry_date(value_, &t) < 0)
+				return -1;
+			strbuf_addf(buf, "%"PRItime, t);
+		} else if (opts->type == TYPE_COLOR && die_on_parse) {
 			char v[COLOR_MAXLEN];
 			if (git_config_color(v, key_, value_) < 0)
 				return -1;
 			strbuf_addstr(buf, v);
+		} else if (opts->type == TYPE_COLOR) {
+			char v[COLOR_MAXLEN];
+			if (color_parse(value_, v) < 0)
+				return -1;
+			strbuf_addstr(buf, v);
 		} else if (value_) {
 			strbuf_addstr(buf, value_);
 		} else {
@@ -372,7 +421,7 @@ static int collect_config(const char *key_, const char *value_,
 	strbuf_init(&values->items[values->nr], 0);
 
 	status = format_config(data->display_opts, &values->items[values->nr++],
-			       key_, value_, kvi);
+			       key_, value_, kvi, 1);
 	if (status < 0)
 		return status;
 	if (status) {
@@ -463,7 +512,7 @@ static int get_value(const struct config_location_options *opts,
 		strbuf_init(item, 0);
 
 		status = format_config(display_opts, item, key_,
-				       display_opts->default_value, &kvi);
+				       display_opts->default_value, &kvi, 1);
 		if (status < 0)
 			die(_("failed to format default config value: %s"),
 			    display_opts->default_value);
@@ -743,7 +792,7 @@ static int get_urlmatch(const struct config_location_options *opts,
 
 		status = format_config(&display_opts, &buf, item->string,
 				       matched->value_is_null ? NULL : matched->value.buf,
-				       &matched->kvi);
+				       &matched->kvi, 1);
 		if (!status)
 			fwrite(buf.buf, 1, buf.len, stdout);
 		strbuf_release(&buf);
-- 
gitgitgadget

[PATCH 4/5] config: create special init for list mode

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

When listing multiple values, our initial settings for the output format
is different. Add a new init helper to specify the fact that keys should
be shown and also add the default delimiters as they were unset in some
cases.

There are two places, differing by the 'git config list' and 'git config
--list' modes.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 builtin/config.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/builtin/config.c b/builtin/config.c
index 5146abcfa3..e69b26af6a 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -917,6 +917,19 @@ static void display_options_init(struct config_display_options *opts)
 	}
 }
 
+static void display_options_init_list(struct config_display_options *opts)
+{
+	opts->show_keys = 1;
+
+	if (opts->end_nul) {
+		display_options_init(opts);
+	} else {
+		opts->term = '\n';
+		opts->delim = ' ';
+		opts->key_delim = '=';
+	}
+}
+
 static int cmd_config_list(int argc, const char **argv, const char *prefix,
 			   struct repository *repo UNUSED)
 {
@@ -935,7 +948,7 @@ static int cmd_config_list(int argc, const char **argv, const char *prefix,
 	check_argc(argc, 0, 0);
 
 	location_options_init(&location_opts, prefix);
-	display_options_init(&display_opts);
+	display_options_init_list(&display_opts);
 
 	setup_auto_pager("config", 1);
 
@@ -1366,6 +1379,7 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
 
 	if (actions == ACTION_LIST) {
 		check_argc(argc, 0, 0);
+		display_options_init_list(&display_opts);
 		if (config_with_options(show_all_config, &display_opts,
 					&location_opts.source, the_repository,
 					&location_opts.options) < 0) {
-- 
gitgitgadget

[PATCH 5/5] config: make 'git config list --type=<X>' work

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

Previously, the --type=<X> argument to 'git config list' was ignored and
did nothing. Now, we add the use of format_config() to the
show_all_config() function so each key-value pair is attempted to be
parsed.

If there is an error in parsing, then the row is not output.

This is a change in behavior! We are starting to respect an option that
was previously ignored, leading to potential user confusion. This is
probably still a good option, since the --type argument did not change
behavior at all previously, so users can get the behavior they expect by
removing the --type argument or adding the --no-type argument.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 Documentation/git-config.adoc |  3 +++
 builtin/config.c              | 19 +++++--------------
 t/t1300-config.sh             | 26 +++++++++++++++++++++++++-
 3 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/Documentation/git-config.adoc b/Documentation/git-config.adoc
index ac3b536a15..5300dd4c51 100644
--- a/Documentation/git-config.adoc
+++ b/Documentation/git-config.adoc
@@ -240,6 +240,9 @@ Valid `<type>`'s include:
   that the given value is canonicalize-able as an ANSI color, but it is written
   as-is.
 +
+If the command is in `list` mode, then the `--type <type>` argument will apply
+to each listed config value. If the value does not successfully parse in that
+format, then it will be omitted from the list.
 
 --bool::
 --int::
diff --git a/builtin/config.c b/builtin/config.c
index e69b26af6a..c83514b4ff 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -363,21 +363,12 @@ static int show_all_config(const char *key_, const char *value_,
 {
 	const struct config_display_options *opts = cb;
 	const struct key_value_info *kvi = ctx->kvi;
+	struct strbuf formatted = STRBUF_INIT;
 
-	if (opts->show_origin || opts->show_scope) {
-		struct strbuf buf = STRBUF_INIT;
-		if (opts->show_scope)
-			show_config_scope(opts, kvi, &buf);
-		if (opts->show_origin)
-			show_config_origin(opts, kvi, &buf);
-		/* Use fwrite as "buf" can contain \0's if "end_null" is set. */
-		fwrite(buf.buf, 1, buf.len, stdout);
-		strbuf_release(&buf);
-	}
-	if (!opts->omit_values && value_)
-		printf("%s%c%s%c", key_, opts->delim, value_, opts->term);
-	else
-		printf("%s%c", key_, opts->term);
+	if (format_config(opts, &formatted, key_, value_, kvi, 0) >= 0)
+		fwrite(formatted.buf, 1, formatted.len, stdout);
+
+	strbuf_release(&formatted);
 	return 0;
 }
 
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 9850fcd5b5..b5ce900126 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -2459,9 +2459,10 @@ done
 
 cat >.git/config <<-\EOF &&
 [section]
-foo = true
+foo = True
 number = 10
 big = 1M
+path = ~/dir
 EOF
 
 test_expect_success 'identical modern --type specifiers are allowed' '
@@ -2503,6 +2504,29 @@ test_expect_success 'unset type specifiers may be reset to conflicting ones' '
 	test_cmp_config 1048576 --type=bool --no-type --type=int section.big
 '
 
+test_expect_success 'list --type=bool shows only canonicalizable bool values' '
+	cat >expect <<-EOF &&
+	section.foo=true
+	section.number=true
+	section.big=true
+	EOF
+
+	git config ${mode_prefix}list --type=bool >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'list --type=path shows only canonicalizable path values' '
+	cat >expect <<-EOF &&
+	section.foo=True
+	section.number=10
+	section.big=1M
+	section.path=$HOME/dir
+	EOF
+
+	git config ${mode_prefix}list --type=path >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success '--type rejects unknown specifiers' '
 	test_must_fail git config --type=nonsense section.foo 2>error &&
 	test_grep "unrecognized --type argument" error
-- 
gitgitgadget

Re: [PATCH 0/5] [RFC] Make 'git config list --type=' parse and filter types

[Junio C Hamano]

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> Problem: 'git config list' doesn't respect --type=<X>!

;-).  

As there is no "inherent" type associated with each configuration
variable (in other words, type of a particular configuration
variable is something determined by the caller that wants the value
of that variable), "git config list/get --type=auto" would not work,
but it would not be too bad to allow "git config list --type=path"
to treat everything as if it is a path and having to filter nonsense
out of the result (like "core.bare = true/false" or even "core.bare"
without value that means true, which may make the "*force*
interpreting it as path" approach to barf), which is an inevitable
consequence.

> This boils down to the fact that the iterator function show_all_config()
> doesn't call format_config(), which includes the type-parsing code.
>
> This wasn't super trivial to update:
>
>  1. format_config() uses git_config_parse_*() methods, which die() on a bad
>     parse.
>  2. The path parsing code didn't have a gentle version.
>  3. The two paths ('git config list' and 'git config --list') needed to
>     standardize their display options to work with format_config().

Thanks for dealing with them.  These are what I would have expected
as part of the "inevitable consequence".

>  4. Finally, we need to filter out key-value pairs that don't match the
>     given type.

This one, however, I need to see the actual code before commenting,
as I do not think key-value pairs have inherent types.  The _only_
special case where you can tell what type the thing is is the
valueless true, which we can safely say is inherently boolean.
Everything else is text string, sometimes interpreted as boolean,
sometimes number, sometimes human-scaled number, sometimes path
(with possible tilde expansion), etc.

> This is marked as an RFC because I need to add some more tests and because
> this is a behavior change! If there are any tools currently passing the
> --type=<X> argument to git config list then they will have a change of
> behavior with this series. It's an easy workaround: drop the --type argument
> or add --no-type to go back to the previous behavior.
>
> Thanks for any and all feedback, -Stolee
>
> Derrick Stolee (5):
>   config: move show_all_config()
>   parse: add git_parse_maybe_pathname()
>   config: allow format_config() to filter
>   config: create special init for list mode
>   config: make 'git config list --type=<X>' work
>
>  Documentation/git-config.adoc |   3 +
>  builtin/config.c              | 130 ++++++++++++++++++++++++----------
>  config.c                      |  14 +---
>  parse.c                       |  24 +++++++
>  parse.h                       |   2 +
>  t/t1300-config.sh             |  26 ++++++-
>  6 files changed, 147 insertions(+), 52 deletions(-)
>
>
> base-commit: 67ad42147a7acc2af6074753ebd03d904476118f
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2044%2Fderrickstolee%2Fconfig-list-type-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2044/derrickstolee/config-list-type-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/2044

Re: [PATCH 3/5] config: allow format_config() to filter

[Junio C Hamano]

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Derrick Stolee <stolee@gmail.com>
>
> The format_config() method in builtin/config.c currently only uses
> git_config_*() methods for parsing. This allows parsing errors to result
> in die() messages appropriate with keys in the error message.
>
> In a future change we will want to use format_config() within 'git
> config list' to help format the output, including when --type=<X>
> arguments are provided. When the parsing fails in that case, that
> key-value pair should be omitted instead of causing a failure across the
> entire command.
>
> This change is formatted in such a way that the if/else-if structure
> allows the default die_on_error version to appear first and then be
> followed by the gentle parsing mode immediately afterwards.
>
> The only callers right now have die_on_parse set to 1.

Certainly you meant die-on-parse-errors, not unconditionally die
when asked to parse ;-).

I wonder if a "bool gently" like everybody else takes would be
easier to understand by more developers and readers, though.



> +		if (opts->type == TYPE_INT && die_on_parse) {
>  			strbuf_addf(buf, "%"PRId64,
>  				    git_config_int64(key_, value_ ? value_ : "", kvi));
> +		} else if (opts->type == TYPE_INT) {
> +			int64_t v;
> +			int ret = git_parse_int64(value_, &v);
> +
> +			if (ret)
> +				return -1;
> +
> +			strbuf_addf(buf, "%"PRId64, v);
> +		}

So, this follows the typical layout that was described in the
proposed log message.  I wonder if it is too much to break the set
of helper functions further down so that this part of the caller can
say something like:

	switch (opts->type) {
	case TYPE_INT:
		format_config_int(buf, key_, value_, kvi, gently);
		break;

and similar case arms for other types?

Re: [PATCH 3/5] config: allow format_config() to filter

[Derrick Stolee]

On 2/10/2026 12:04 AM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> From: Derrick Stolee <stolee@gmail.com>
>>
>> The format_config() method in builtin/config.c currently only uses
>> git_config_*() methods for parsing. This allows parsing errors to result
>> in die() messages appropriate with keys in the error message.
>>
>> In a future change we will want to use format_config() within 'git
>> config list' to help format the output, including when --type=<X>
>> arguments are provided. When the parsing fails in that case, that
>> key-value pair should be omitted instead of causing a failure across the
>> entire command.
>>
>> This change is formatted in such a way that the if/else-if structure
>> allows the default die_on_error version to appear first and then be
>> followed by the gentle parsing mode immediately afterwards.
>>
>> The only callers right now have die_on_parse set to 1.
> 
> Certainly you meant die-on-parse-errors, not unconditionally die
> when asked to parse ;-).
> 
> I wonder if a "bool gently" like everybody else takes would be
> easier to understand by more developers and readers, though.

'gently' makes a lot more sense.

>> +		if (opts->type == TYPE_INT && die_on_parse) {
>>  			strbuf_addf(buf, "%"PRId64,
>>  				    git_config_int64(key_, value_ ? value_ : "", kvi));
>> +		} else if (opts->type == TYPE_INT) {
>> +			int64_t v;
>> +			int ret = git_parse_int64(value_, &v);
>> +
>> +			if (ret)
>> +				return -1;
>> +
>> +			strbuf_addf(buf, "%"PRId64, v);
>> +		}
> 
> So, this follows the typical layout that was described in the
> proposed log message.  I wonder if it is too much to break the set
> of helper functions further down so that this part of the caller can
> say something like:
> 
> 	switch (opts->type) {
> 	case TYPE_INT:
> 		format_config_int(buf, key_, value_, kvi, gently);
> 		break;
> 
> and similar case arms for other types?

I had a similar feeling that such a refactor would be necessary.

I didn't want to go through that careful work if it wasn't
justified by positive reactions to the RFC. Thanks for calling it
out, and I'll definitely put in that effort if we find this worth
a v2.

Thanks,
-Stolee

Re: [PATCH 0/5] [RFC] Make 'git config list --type=' parse and filter types

[Derrick Stolee]

On 2/9/2026 11:59 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:...
>> This boils down to the fact that the iterator function show_all_config()
>> doesn't call format_config(), which includes the type-parsing code.
>>
>> This wasn't super trivial to update:
>>
>>  1. format_config() uses git_config_parse_*() methods, which die() on a bad
>>     parse.
>>  2. The path parsing code didn't have a gentle version.
>>  3. The two paths ('git config list' and 'git config --list') needed to
>>     standardize their display options to work with format_config().
> 
> Thanks for dealing with them.  These are what I would have expected
> as part of the "inevitable consequence".
> 
>>  4. Finally, we need to filter out key-value pairs that don't match the
>>     given type.
> 
> This one, however, I need to see the actual code before commenting,
> as I do not think key-value pairs have inherent types.  The _only_
> special case where you can tell what type the thing is is the
> valueless true, which we can safely say is inherently boolean.
> Everything else is text string, sometimes interpreted as boolean,
> sometimes number, sometimes human-scaled number, sometimes path
> (with possible tilde expansion), etc.
This is the crux of this series. If the caller asks for a type, then I
see a couple different ways to react:

 1. If the value fails to parse in that type, then don't list that
    result, allowing the caller to have confidence that every result
    is of the correct format.

 2. If the value fails to parse in that type, then list it in its
    base string. The caller would need to do extra parsing to check
    that the results match the correct format.

I chose option 1. It avoids showing results that would result in
'git config get --type=<X> <key>' to die().

I'd be interested to hear if there are reasons to go with option 2,
or if there exists an alternative option that I don't see.

Reordering your message somewhat:

> As there is no "inherent" type associated with each configuration
> variable (in other words, type of a particular configuration
> variable is something determined by the caller that wants the value
> of that variable), "git config list/get --type=auto" would not work,
> but it would not be too bad to allow "git config list --type=path"
> to treat everything as if it is a path and having to filter nonsense
> out of the result (like "core.bare = true/false" or even "core.bare"
> without value that means true, which may make the "*force*
> interpreting it as path" approach to barf), which is an inevitable
> consequence.

I agree that there is not inherent type, so the user can only
specify the type that they are expecting. To me, this is a request to
filter the results.

I don't think we'll have much success if we try to guess the type,
such as trying to parse an int, then a bool, then a path.

Thanks,
-Stolee

Re: [PATCH 0/5] [RFC] Make 'git config list --type=' parse and filter types

[Patrick Steinhardt]

On Tue, Feb 10, 2026 at 04:42:54AM +0000, Derrick Stolee via GitGitGadget wrote:
> This is marked as an RFC because I need to add some more tests and because
> this is a behavior change! If there are any tools currently passing the
> --type=<X> argument to git config list then they will have a change of
> behavior with this series. It's an easy workaround: drop the --type argument
> or add --no-type to go back to the previous behavior.

I think this is not a huge problem. It simply reads like a bug to me
that the command accepts the option, but doesn't honor it. Sure, it can
lead to different behaviour, but I think that's acceptable.

Patrick

Re: [PATCH 2/5] parse: add git_parse_maybe_pathname()

[Patrick Steinhardt]

On Tue, Feb 10, 2026 at 04:42:56AM +0000, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <stolee@gmail.com>
> 
> This extraction of logic from config.c's git_config_pathname() allows
> for parsing a fully-qualified path from a relative path along with
> validation of the existence of the path without failing with a die().

That sentence is quite something. I had to read it thrice to understand
what it wants to say :)

> diff --git a/parse.c b/parse.c
> index 48313571aa..3f37f0b93a 100644
> --- a/parse.c
> +++ b/parse.c
> @@ -209,3 +210,26 @@ unsigned long git_env_ulong(const char *k, unsigned long val)
>  		die(_("failed to parse %s"), k);
>  	return val;
>  }
> +
> +int git_parse_maybe_pathname(const char *value, char **dest)
> +{
> +	bool is_optional;
> +	char *path;
> +
> +	if (!value)
> +		return -1;
> +
> +	is_optional = skip_prefix(value, ":(optional)", &value);
> +	path = interpolate_path(value, 0);
> +	if (!path)
> +		return -1;
> +
> +	if (is_optional && is_missing_file(path)) {
> +		free(path);
> +		*dest = NULL;
> +		return 0;
> +	}
> +
> +	*dest = path;
> +	return 0;
> +}

Okay. So the difference is that this function here doesn't cause us to
die in case the path is not marked as optional and missing. Makes sense.

> diff --git a/parse.h b/parse.h
> index ea32de9a91..4f97c3727a 100644
> --- a/parse.h
> +++ b/parse.h
> @@ -19,4 +19,6 @@ int git_parse_maybe_bool_text(const char *value);
>  int git_env_bool(const char *, int);
>  unsigned long git_env_ulong(const char *, unsigned long);
>  
> +int git_parse_maybe_pathname(const char *value, char **dest);

I think this function could use some explanation what it actually does,
as the behaviour is non-trivial:

  - I think the ":(optional)" part needs to be documented properly to
    say that we return successfully with a NULL string in case the
    target path doesn't exist.

  - We should document that it expands "~" and "%(prefix)" (even though
    the latter feels somewhat coincidental to me).

  - The path is not resolved to an absolute path.

Thanks!

Patrick

Re: [PATCH 5/5] config: make 'git config list --type=<X>' work

[Patrick Steinhardt]

On Tue, Feb 10, 2026 at 04:42:59AM +0000, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <stolee@gmail.com>
> 
> Previously, the --type=<X> argument to 'git config list' was ignored and
> did nothing. Now, we add the use of format_config() to the
> show_all_config() function so each key-value pair is attempted to be
> parsed.
> 
> If there is an error in parsing, then the row is not output.

I was a bit surprised at first, but now that I think about it a bit more
I think this is sensible behaviour. If I ask for `git config list
--type=int`, then I don't want to see any non-int configuration. I
wouldn't even expect a warning, as the option essentially works like a
filter.

> This is a change in behavior! We are starting to respect an option that
> was previously ignored, leading to potential user confusion. This is
> probably still a good option, since the --type argument did not change
> behavior at all previously, so users can get the behavior they expect by
> removing the --type argument or adding the --no-type argument.

Yeah, I fully agree that this is a sensible change in behaviour. It is
obviously broken right now, so I would claim that this is simply a bug
fix.

> diff --git a/Documentation/git-config.adoc b/Documentation/git-config.adoc
> index ac3b536a15..5300dd4c51 100644
> --- a/Documentation/git-config.adoc
> +++ b/Documentation/git-config.adoc

The synopsis of `git config list` should also be amended.

> diff --git a/builtin/config.c b/builtin/config.c
> index e69b26af6a..c83514b4ff 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -363,21 +363,12 @@ static int show_all_config(const char *key_, const char *value_,
>  {
>  	const struct config_display_options *opts = cb;
>  	const struct key_value_info *kvi = ctx->kvi;
> +	struct strbuf formatted = STRBUF_INIT;
>  
> -	if (opts->show_origin || opts->show_scope) {
> -		struct strbuf buf = STRBUF_INIT;
> -		if (opts->show_scope)
> -			show_config_scope(opts, kvi, &buf);
> -		if (opts->show_origin)
> -			show_config_origin(opts, kvi, &buf);
> -		/* Use fwrite as "buf" can contain \0's if "end_null" is set. */
> -		fwrite(buf.buf, 1, buf.len, stdout);
> -		strbuf_release(&buf);
> -	}
> -	if (!opts->omit_values && value_)
> -		printf("%s%c%s%c", key_, opts->delim, value_, opts->term);
> -	else
> -		printf("%s%c", key_, opts->term);
> +	if (format_config(opts, &formatted, key_, value_, kvi, 0) >= 0)
> +		fwrite(formatted.buf, 1, formatted.len, stdout);
> +
> +	strbuf_release(&formatted);
>  	return 0;
>  }
>  

I wonder whether there is a good argument to be made here that we should
keep the old logic in case no "--type=" parameter was given. In that
case, for example the following output would remain the same:

> diff --git a/t/t1300-config.sh b/t/t1300-config.sh
> index 9850fcd5b5..b5ce900126 100755
> --- a/t/t1300-config.sh
> +++ b/t/t1300-config.sh
> @@ -2459,9 +2459,10 @@ done
>  
>  cat >.git/config <<-\EOF &&
>  [section]
> -foo = true
> +foo = True
>  number = 10
>  big = 1M
> +path = ~/dir
>  EOF
>  
>  test_expect_success 'identical modern --type specifiers are allowed' '

I'm not really sure whether we want that though. I actually like that
this also leads to some code duplication, so maybe this is fine?

Patrick

Re: [PATCH 5/5] config: make 'git config list --type=<X>' work

[Derrick Stolee]

On 2/11/2026 7:13 AM, Patrick Steinhardt wrote:
> On Tue, Feb 10, 2026 at 04:42:59AM +0000, Derrick Stolee via GitGitGadget wrote:

>> diff --git a/Documentation/git-config.adoc b/Documentation/git-config.adoc
>> index ac3b536a15..5300dd4c51 100644
>> --- a/Documentation/git-config.adoc
>> +++ b/Documentation/git-config.adoc
> 
> The synopsis of `git config list` should also be amended.

Good point. Will fix.
 
>> diff --git a/builtin/config.c b/builtin/config.c
>> index e69b26af6a..c83514b4ff 100644
>> --- a/builtin/config.c
>> +++ b/builtin/config.c
>> @@ -363,21 +363,12 @@ static int show_all_config(const char *key_, const char *value_,
>>  {
>>  	const struct config_display_options *opts = cb;
>>  	const struct key_value_info *kvi = ctx->kvi;
>> +	struct strbuf formatted = STRBUF_INIT;
>>  
>> -	if (opts->show_origin || opts->show_scope) {
>> -		struct strbuf buf = STRBUF_INIT;
>> -		if (opts->show_scope)
>> -			show_config_scope(opts, kvi, &buf);
>> -		if (opts->show_origin)
>> -			show_config_origin(opts, kvi, &buf);
>> -		/* Use fwrite as "buf" can contain \0's if "end_null" is set. */
>> -		fwrite(buf.buf, 1, buf.len, stdout);
>> -		strbuf_release(&buf);
>> -	}
>> -	if (!opts->omit_values && value_)
>> -		printf("%s%c%s%c", key_, opts->delim, value_, opts->term);
>> -	else
>> -		printf("%s%c", key_, opts->term);
>> +	if (format_config(opts, &formatted, key_, value_, kvi, 0) >= 0)
>> +		fwrite(formatted.buf, 1, formatted.len, stdout);
>> +
>> +	strbuf_release(&formatted);
>>  	return 0;
>>  }
>>  
> 
> I wonder whether there is a good argument to be made here that we should
> keep the old logic in case no "--type=" parameter was given. In that
> case, for example the following output would remain the same:

If no `--type=` parameter is given, then this new implementation does
the exact same thing as the display_options use a string format (which
does not mutate the config values).

>> diff --git a/t/t1300-config.sh b/t/t1300-config.sh
>> index 9850fcd5b5..b5ce900126 100755
>> --- a/t/t1300-config.sh
>> +++ b/t/t1300-config.sh
>> @@ -2459,9 +2459,10 @@ done
>>  
>>  cat >.git/config <<-\EOF &&
>>  [section]
>> -foo = true
>> +foo = True
>>  number = 10
>>  big = 1M
>> +path = ~/dir
>>  EOF
>>  
>>  test_expect_success 'identical modern --type specifiers are allowed' '
> 
> I'm not really sure whether we want that though. I actually like that
> this also leads to some code duplication, so maybe this is fine?

The change you highlight here is a difference in the config file _contents_
and not the expected output. These changes are to help demonstrate that the
bool and path types make meaningful conversions when listing these values.

The previous tests for getting bool values did not demonstrate the way it
modifies case, for example.

Thanks,
-Stolee

Re: [PATCH 5/5] config: make 'git config list --type=<X>' work

[Patrick Steinhardt]

On Wed, Feb 11, 2026 at 12:49:19PM -0500, Derrick Stolee wrote:
> On 2/11/2026 7:13 AM, Patrick Steinhardt wrote:
> > On Tue, Feb 10, 2026 at 04:42:59AM +0000, Derrick Stolee via GitGitGadget wrote:
> >> diff --git a/builtin/config.c b/builtin/config.c
> >> index e69b26af6a..c83514b4ff 100644
> >> --- a/builtin/config.c
> >> +++ b/builtin/config.c
> >> @@ -363,21 +363,12 @@ static int show_all_config(const char *key_, const char *value_,
> >>  {
> >>  	const struct config_display_options *opts = cb;
> >>  	const struct key_value_info *kvi = ctx->kvi;
> >> +	struct strbuf formatted = STRBUF_INIT;
> >>  
> >> -	if (opts->show_origin || opts->show_scope) {
> >> -		struct strbuf buf = STRBUF_INIT;
> >> -		if (opts->show_scope)
> >> -			show_config_scope(opts, kvi, &buf);
> >> -		if (opts->show_origin)
> >> -			show_config_origin(opts, kvi, &buf);
> >> -		/* Use fwrite as "buf" can contain \0's if "end_null" is set. */
> >> -		fwrite(buf.buf, 1, buf.len, stdout);
> >> -		strbuf_release(&buf);
> >> -	}
> >> -	if (!opts->omit_values && value_)
> >> -		printf("%s%c%s%c", key_, opts->delim, value_, opts->term);
> >> -	else
> >> -		printf("%s%c", key_, opts->term);
> >> +	if (format_config(opts, &formatted, key_, value_, kvi, 0) >= 0)
> >> +		fwrite(formatted.buf, 1, formatted.len, stdout);
> >> +
> >> +	strbuf_release(&formatted);
> >>  	return 0;
> >>  }
> >>  
> > 
> > I wonder whether there is a good argument to be made here that we should
> > keep the old logic in case no "--type=" parameter was given. In that
> > case, for example the following output would remain the same:
> 
> If no `--type=` parameter is given, then this new implementation does
> the exact same thing as the display_options use a string format (which
> does not mutate the config values).
> 
> >> diff --git a/t/t1300-config.sh b/t/t1300-config.sh
> >> index 9850fcd5b5..b5ce900126 100755
> >> --- a/t/t1300-config.sh
> >> +++ b/t/t1300-config.sh
> >> @@ -2459,9 +2459,10 @@ done
> >>  
> >>  cat >.git/config <<-\EOF &&
> >>  [section]
> >> -foo = true
> >> +foo = True
> >>  number = 10
> >>  big = 1M
> >> +path = ~/dir
> >>  EOF
> >>  
> >>  test_expect_success 'identical modern --type specifiers are allowed' '
> > 
> > I'm not really sure whether we want that though. I actually like that
> > this also leads to some code duplication, so maybe this is fine?
> 
> The change you highlight here is a difference in the config file _contents_
> and not the expected output. These changes are to help demonstrate that the
> bool and path types make meaningful conversions when listing these values.

Ooh, right. Completely missed that, thanks for the clarification.

Patrick

[PATCH v2 00/13] Make 'git config list --type=' parse and filter types

[Derrick Stolee via GitGitGadget]

I started down this road based on feedback on my 'git config-batch' RFC [1].

[1]
https://lore.kernel.org/git/pull.2033.git.1770214803.gitgitgadget@gmail.com/

I had described my intention to use 'git config-batch' as a single process
to load multiple config values one-by-one. Brian mentioned that 'git config
list -z' would probably suffice, so I started experimenting in that
direction [2].

[2]
https://github.com/git-ecosystem/git-credential-manager/compare/main...derrickstolee:config-list

However, I ran into a problem: the most critical performance bottleneck is
related to path-formatted config values that are queried with 'git config
get --type=path -z'. It wasn't hard to update things to lazily load the full
list of config values by type [3], but I then noticed a big problem!

[3]
https://github.com/git-ecosystem/git-credential-manager/commit/d403c8e24ce6f37da920cce23842dd5a6cf6481d

Problem: 'git config list' doesn't respect --type=<X>!

This boils down to the fact that the iterator function show_all_config()
doesn't call format_config(), which includes the type-parsing code.

This wasn't super trivial to update:

 1. format_config() uses git_config_parse_*() methods, which die() on a bad
    parse.
 2. The path parsing code didn't have a gentle version.
 3. The two paths ('git config list' and 'git config --list') needed to
    standardize their display options to work with format_config().
 4. Finally, we need to filter out key-value pairs that don't match the
    given type.


Updates in v2
=============

Based on the positive feedback in round one, this is no longer an RFC.

 * format_config() now uses a 'gently' parameter instead of 'die_on_parse'
   (flipped).
 * format_config() is more carefully updated with helper methods and a
   global refactor.
 * New gentle parsing code is introduced right before the format_config()
   helper is created to use it.
 * I squashed the change that updates the display_opts initial state into
   the patch that uses format_config() for the 'list' command. The initial
   state change on its own leads to test failures, so I am making a slightly
   bigger patch to keep things passing tests at every change.
 * More tests for 'git config list --type=<X>' are added.
 * I rearranged things so the 'git config list --type' integration follows
   the format_config() update immediately. The tests at that time show what
   such a trivial implementation would do, including failing on bool parsing
   and having several error messages for color and expiry-date parsing. The
   tests modify as these issues are fixed with gentle parsers.
 * I have a prototype implementation of GCM using this option in [4] and it
   gets the performance improvements I was hoping for. It requires polish
   and a compatibility check that uses the Git version to guarantee that
   this --type behavior change is recognized.

[4] https://github.com/git-ecosystem/git-credential-manager/pull/2268

Thanks for any and all feedback, -Stolee

Derrick Stolee (13):
  config: move show_all_config()
  config: add 'gently' parameter to format_config()
  config: make 'git config list --type=<X>' work
  config: format int64s gently
  config: format bools gently
  config: format bools or ints gently
  config: format bools or strings in helper
  parse: add git_parse_maybe_pathname()
  config: format paths gently
  config: format expiry dates gently
  color: add color_parse_gently()
  config: format colors gently
  config: restructure format_config()

 Documentation/git-config.adoc |   3 +
 builtin/config.c              | 283 +++++++++++++++++++++++++---------
 color.c                       |  25 ++-
 color.h                       |   1 +
 config.c                      |  14 +-
 parse.c                       |  24 +++
 parse.h                       |   2 +
 t/t1300-config.sh             |  58 ++++++-
 8 files changed, 318 insertions(+), 92 deletions(-)


base-commit: 67ad42147a7acc2af6074753ebd03d904476118f
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2044%2Fderrickstolee%2Fconfig-list-type-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2044/derrickstolee/config-list-type-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2044

Range-diff vs v1:

  1:  bca83d8ca8 =  1:  bca83d8ca8 config: move show_all_config()
  3:  d9e0424010 !  2:  93c94a1b25 config: allow format_config() to filter
     @@ Metadata
      Author: Derrick Stolee <stolee@gmail.com>
      
       ## Commit message ##
     -    config: allow format_config() to filter
     +    config: add 'gently' parameter to format_config()
      
     -    The format_config() method in builtin/config.c currently only uses
     -    git_config_*() methods for parsing. This allows parsing errors to result
     -    in die() messages appropriate with keys in the error message.
     -
     -    In a future change we will want to use format_config() within 'git
     -    config list' to help format the output, including when --type=<X>
     -    arguments are provided. When the parsing fails in that case, that
     -    key-value pair should be omitted instead of causing a failure across the
     -    entire command.
     -
     -    This change is formatted in such a way that the if/else-if structure
     -    allows the default die_on_error version to appear first and then be
     -    followed by the gentle parsing mode immediately afterwards.
     -
     -    The only callers right now have die_on_parse set to 1.
     +    This parameter is set to 0 for all current callers and is UNUSED.
     +    However, we will start using this option in future changes and in a
     +    critical change that requires gentle parsing (not using die()) to try
     +    parsing all values in a list.
      
          Signed-off-by: Derrick Stolee <stolee@gmail.com>
      
       ## builtin/config.c ##
     -@@
     - #include "abspath.h"
     - #include "config.h"
     - #include "color.h"
     -+#include "date.h"
     - #include "editor.h"
     - #include "environment.h"
     - #include "gettext.h"
      @@ builtin/config.c: struct strbuf_list {
     +  * append it into strbuf `buf`.  Returns a negative value on failure,
     +  * 0 on success, 1 on a missing optional value (i.e., telling the
     +  * caller to pretend that <key_,value_> did not exist).
     ++ *
     ++ * Note: 'gently' is currently ignored, but will be implemented in
     ++ * a future change.
        */
       static int format_config(const struct config_display_options *opts,
       			 struct strbuf *buf, const char *key_,
      -			 const char *value_, const struct key_value_info *kvi)
      +			 const char *value_, const struct key_value_info *kvi,
     -+			 int die_on_parse)
     ++			 int gently UNUSED)
       {
       	if (opts->show_scope)
       		show_config_scope(opts, kvi, buf);
     -@@ builtin/config.c: static int format_config(const struct config_display_options *opts,
     - 		if (opts->show_keys)
     - 			strbuf_addch(buf, opts->key_delim);
     - 
     --		if (opts->type == TYPE_INT)
     -+		if (opts->type == TYPE_INT && die_on_parse) {
     - 			strbuf_addf(buf, "%"PRId64,
     - 				    git_config_int64(key_, value_ ? value_ : "", kvi));
     --		else if (opts->type == TYPE_BOOL)
     -+		} else if (opts->type == TYPE_INT) {
     -+			int64_t v;
     -+			int ret = git_parse_int64(value_, &v);
     -+
     -+			if (ret)
     -+				return -1;
     -+
     -+			strbuf_addf(buf, "%"PRId64, v);
     -+		}
     -+		else if (opts->type == TYPE_BOOL && die_on_parse) {
     - 			strbuf_addstr(buf, git_config_bool(key_, value_) ?
     - 				      "true" : "false");
     --		else if (opts->type == TYPE_BOOL_OR_INT) {
     --			int is_bool, v;
     --			v = git_config_bool_or_int(key_, value_, kvi,
     --						   &is_bool);
     -+		} else if (opts->type == TYPE_BOOL) {
     -+			int value = git_parse_maybe_bool(value_);
     -+
     -+			if (value < 0)
     -+				return -1;
     -+
     -+			strbuf_addstr(buf, value ? "true" : "false");
     -+		} else if (opts->type == TYPE_BOOL_OR_INT && die_on_parse) {
     -+			int is_bool = 0;
     -+			int v = git_config_bool_or_int(key_, value_, kvi,
     -+						       &is_bool);
     -+			if (is_bool)
     -+				strbuf_addstr(buf, v ? "true" : "false");
     -+			else
     -+				strbuf_addf(buf, "%d", v);
     -+		} else if (opts->type == TYPE_BOOL_OR_INT) {
     -+			int is_bool = 0;
     -+			int v = git_parse_maybe_bool_text(value_);
     -+
     -+			if (v < 0)
     -+				return -1;
     -+
     - 			if (is_bool)
     - 				strbuf_addstr(buf, v ? "true" : "false");
     - 			else
     - 				strbuf_addf(buf, "%d", v);
     - 		} else if (opts->type == TYPE_BOOL_OR_STR) {
     -+			/* Note: this can't fail to parse! */
     - 			int v = git_parse_maybe_bool(value_);
     - 			if (v < 0)
     - 				strbuf_addstr(buf, value_);
     - 			else
     - 				strbuf_addstr(buf, v ? "true" : "false");
     --		} else if (opts->type == TYPE_PATH) {
     -+		} else if (opts->type == TYPE_PATH && die_on_parse) {
     - 			char *v;
     - 			if (git_config_pathname(&v, key_, value_) < 0)
     - 				return -1;
     -@@ builtin/config.c: static int format_config(const struct config_display_options *opts,
     - 			else
     - 				return 1; /* :(optional)no-such-file */
     - 			free((char *)v);
     --		} else if (opts->type == TYPE_EXPIRY_DATE) {
     -+		} else if (opts->type == TYPE_PATH) {
     -+			char *v;
     -+			if (git_parse_maybe_pathname(value_, &v) < 0)
     -+				return -1;
     -+			if (v)
     -+				strbuf_addstr(buf, v);
     -+			else
     -+				return 1; /* :(optional)no-such-file */
     -+			free((char *)v);
     -+		} else if (opts->type == TYPE_EXPIRY_DATE && die_on_parse) {
     - 			timestamp_t t;
     - 			if (git_config_expiry_date(&t, key_, value_) < 0)
     - 				return -1;
     - 			strbuf_addf(buf, "%"PRItime, t);
     --		} else if (opts->type == TYPE_COLOR) {
     -+		} else if (opts->type == TYPE_EXPIRY_DATE) {
     -+			timestamp_t t;
     -+			if (parse_expiry_date(value_, &t) < 0)
     -+				return -1;
     -+			strbuf_addf(buf, "%"PRItime, t);
     -+		} else if (opts->type == TYPE_COLOR && die_on_parse) {
     - 			char v[COLOR_MAXLEN];
     - 			if (git_config_color(v, key_, value_) < 0)
     - 				return -1;
     - 			strbuf_addstr(buf, v);
     -+		} else if (opts->type == TYPE_COLOR) {
     -+			char v[COLOR_MAXLEN];
     -+			if (color_parse(value_, v) < 0)
     -+				return -1;
     -+			strbuf_addstr(buf, v);
     - 		} else if (value_) {
     - 			strbuf_addstr(buf, value_);
     - 		} else {
      @@ builtin/config.c: static int collect_config(const char *key_, const char *value_,
       	strbuf_init(&values->items[values->nr], 0);
       
       	status = format_config(data->display_opts, &values->items[values->nr++],
      -			       key_, value_, kvi);
     -+			       key_, value_, kvi, 1);
     ++			       key_, value_, kvi, 0);
       	if (status < 0)
       		return status;
       	if (status) {
     @@ builtin/config.c: static int get_value(const struct config_location_options *opt
       
       		status = format_config(display_opts, item, key_,
      -				       display_opts->default_value, &kvi);
     -+				       display_opts->default_value, &kvi, 1);
     ++				       display_opts->default_value, &kvi, 0);
       		if (status < 0)
       			die(_("failed to format default config value: %s"),
       			    display_opts->default_value);
     @@ builtin/config.c: static int get_urlmatch(const struct config_location_options *
       		status = format_config(&display_opts, &buf, item->string,
       				       matched->value_is_null ? NULL : matched->value.buf,
      -				       &matched->kvi);
     -+				       &matched->kvi, 1);
     ++				       &matched->kvi, 0);
       		if (!status)
       			fwrite(buf.buf, 1, buf.len, stdout);
       		strbuf_release(&buf);
  5:  e27d52c4a5 !  3:  6d2a48a3b7 config: make 'git config list --type=<X>' work
     @@ Commit message
          Previously, the --type=<X> argument to 'git config list' was ignored and
          did nothing. Now, we add the use of format_config() to the
          show_all_config() function so each key-value pair is attempted to be
     -    parsed.
     +    parsed. This is our first use of the 'gently' parameter with a nonzero
     +    value.
     +
     +    When listing multiple values, our initial settings for the output format
     +    is different. Add a new init helper to specify the fact that keys should
     +    be shown and also add the default delimiters as they were unset in some
     +    cases.
      
          If there is an error in parsing, then the row is not output.
      
     @@ Commit message
          behavior at all previously, so users can get the behavior they expect by
          removing the --type argument or adding the --no-type argument.
      
     +    t1300-config.sh is updated with the current behavior of this formatting
     +    logic to justify the upcoming refactoring of format_config() that will
     +    incrementally fix some of these cases to be more user-friendly.
     +
          Signed-off-by: Derrick Stolee <stolee@gmail.com>
      
       ## Documentation/git-config.adoc ##
     @@ builtin/config.c: static int show_all_config(const char *key_, const char *value
      -		printf("%s%c%s%c", key_, opts->delim, value_, opts->term);
      -	else
      -		printf("%s%c", key_, opts->term);
     -+	if (format_config(opts, &formatted, key_, value_, kvi, 0) >= 0)
     ++	if (format_config(opts, &formatted, key_, value_, kvi, 1) >= 0)
      +		fwrite(formatted.buf, 1, formatted.len, stdout);
      +
      +	strbuf_release(&formatted);
       	return 0;
       }
       
     +@@ builtin/config.c: static void display_options_init(struct config_display_options *opts)
     + 	}
     + }
     + 
     ++static void display_options_init_list(struct config_display_options *opts)
     ++{
     ++	opts->show_keys = 1;
     ++
     ++	if (opts->end_nul) {
     ++		display_options_init(opts);
     ++	} else {
     ++		opts->term = '\n';
     ++		opts->delim = ' ';
     ++		opts->key_delim = '=';
     ++	}
     ++}
     ++
     + static int cmd_config_list(int argc, const char **argv, const char *prefix,
     + 			   struct repository *repo UNUSED)
     + {
     +@@ builtin/config.c: static int cmd_config_list(int argc, const char **argv, const char *prefix,
     + 	check_argc(argc, 0, 0);
     + 
     + 	location_options_init(&location_opts, prefix);
     +-	display_options_init(&display_opts);
     ++	display_options_init_list(&display_opts);
     + 
     + 	setup_auto_pager("config", 1);
     + 
     +@@ builtin/config.c: static int cmd_config_actions(int argc, const char **argv, const char *prefix)
     + 
     + 	if (actions == ACTION_LIST) {
     + 		check_argc(argc, 0, 0);
     ++		display_options_init_list(&display_opts);
     + 		if (config_with_options(show_all_config, &display_opts,
     + 					&location_opts.source, the_repository,
     + 					&location_opts.options) < 0) {
      
       ## t/t1300-config.sh ##
      @@ t/t1300-config.sh: done
     @@ t/t1300-config.sh: done
       number = 10
       big = 1M
      +path = ~/dir
     ++red = red
     ++blue = Blue
     ++date = Fri Jun 4 15:46:55 2010
       EOF
       
       test_expect_success 'identical modern --type specifiers are allowed' '
     @@ t/t1300-config.sh: test_expect_success 'unset type specifiers may be reset to co
      +	section.big=true
      +	EOF
      +
     -+	git config ${mode_prefix}list --type=bool >actual &&
     -+	test_cmp expect actual
     ++	test_must_fail git config ${mode_prefix}list --type=bool
      +'
      +
      +test_expect_success 'list --type=path shows only canonicalizable path values' '
     @@ t/t1300-config.sh: test_expect_success 'unset type specifiers may be reset to co
      +	section.number=10
      +	section.big=1M
      +	section.path=$HOME/dir
     ++	section.red=red
     ++	section.blue=Blue
     ++	section.date=Fri Jun 4 15:46:55 2010
     ++	EOF
     ++
     ++	git config ${mode_prefix}list --type=path >actual 2>err &&
     ++	test_cmp expect actual &&
     ++	test_must_be_empty err
     ++'
     ++
     ++test_expect_success 'list --type=expiry-date shows only canonicalizable dates' '
     ++	cat >expecterr <<-EOF &&
     ++	error: '\''True'\'' for '\''section.foo'\'' is not a valid timestamp
     ++	error: '\''~/dir'\'' for '\''section.path'\'' is not a valid timestamp
     ++	error: '\''red'\'' for '\''section.red'\'' is not a valid timestamp
     ++	error: '\''Blue'\'' for '\''section.blue'\'' is not a valid timestamp
     ++	EOF
     ++
     ++	git config ${mode_prefix}list --type=expiry-date >actual 2>err &&
     ++
     ++	# section.number and section.big parse as relative dates that could
     ++	# have clock skew in their results.
     ++	test_grep section.big actual &&
     ++	test_grep section.number actual &&
     ++	test_grep "section.date=$(git config --type=expiry-date section.$key)" actual &&
     ++	test_cmp expecterr err
     ++'
     ++
     ++test_expect_success 'list --type=color shows only canonicalizable color values' '
     ++	cat >expect <<-EOF &&
     ++	section.number=<>
     ++	section.red=<RED>
     ++	section.blue=<BLUE>
     ++	EOF
     ++
     ++	cat >expecterr <<-EOF &&
     ++	error: invalid color value: True
     ++	error: invalid color value: 1M
     ++	error: invalid color value: ~/dir
     ++	error: invalid color value: Fri Jun 4 15:46:55 2010
      +	EOF
      +
     -+	git config ${mode_prefix}list --type=path >actual &&
     -+	test_cmp expect actual
     ++	git config ${mode_prefix}list --type=color >actual.raw 2>err &&
     ++	test_decode_color <actual.raw >actual &&
     ++	test_cmp expect actual &&
     ++	test_cmp expecterr err
      +'
      +
       test_expect_success '--type rejects unknown specifiers' '
  4:  5601a5a84f !  4:  2bca4d2316 config: create special init for list mode
     @@ Metadata
      Author: Derrick Stolee <stolee@gmail.com>
      
       ## Commit message ##
     -    config: create special init for list mode
     +    config: format int64s gently
      
     -    When listing multiple values, our initial settings for the output format
     -    is different. Add a new init helper to specify the fact that keys should
     -    be shown and also add the default delimiters as they were unset in some
     -    cases.
     -
     -    There are two places, differing by the 'git config list' and 'git config
     -    --list' modes.
     +    Move the logic for formatting int64 config values into a helper method
     +    and use gentle parsing when needed.
      
          Signed-off-by: Derrick Stolee <stolee@gmail.com>
      
       ## builtin/config.c ##
     -@@ builtin/config.c: static void display_options_init(struct config_display_options *opts)
     - 	}
     - }
     +@@ builtin/config.c: struct strbuf_list {
     + 	int alloc;
     + };
       
     -+static void display_options_init_list(struct config_display_options *opts)
     ++static int format_config_int64(struct strbuf *buf,
     ++			       const char *key_,
     ++			       const char *value_,
     ++			       const struct key_value_info *kvi,
     ++			       int gently)
      +{
     -+	opts->show_keys = 1;
     -+
     -+	if (opts->end_nul) {
     -+		display_options_init(opts);
     ++	int64_t v = 0;
     ++	if (gently) {
     ++		if (git_parse_int64(value_, &v))
     ++			return -1;
      +	} else {
     -+		opts->term = '\n';
     -+		opts->delim = ' ';
     -+		opts->key_delim = '=';
     ++		/* may die() */
     ++		v = git_config_int64(key_, value_ ? value_ : "", kvi);
      +	}
     ++
     ++	strbuf_addf(buf, "%"PRId64, v);
     ++	return 0;
      +}
      +
     - static int cmd_config_list(int argc, const char **argv, const char *prefix,
     - 			   struct repository *repo UNUSED)
     + /*
     +  * Format the configuration key-value pair (`key_`, `value_`) and
     +  * append it into strbuf `buf`.  Returns a negative value on failure,
     +@@ builtin/config.c: struct strbuf_list {
     + static int format_config(const struct config_display_options *opts,
     + 			 struct strbuf *buf, const char *key_,
     + 			 const char *value_, const struct key_value_info *kvi,
     +-			 int gently UNUSED)
     ++			 int gently)
       {
     -@@ builtin/config.c: static int cmd_config_list(int argc, const char **argv, const char *prefix,
     - 	check_argc(argc, 0, 0);
     ++	int res = 0;
     + 	if (opts->show_scope)
     + 		show_config_scope(opts, kvi, buf);
     + 	if (opts->show_origin)
     +@@ builtin/config.c: static int format_config(const struct config_display_options *opts,
     + 			strbuf_addch(buf, opts->key_delim);
       
     - 	location_options_init(&location_opts, prefix);
     --	display_options_init(&display_opts);
     -+	display_options_init_list(&display_opts);
     - 
     - 	setup_auto_pager("config", 1);
     - 
     -@@ builtin/config.c: static int cmd_config_actions(int argc, const char **argv, const char *prefix)
     + 		if (opts->type == TYPE_INT)
     +-			strbuf_addf(buf, "%"PRId64,
     +-				    git_config_int64(key_, value_ ? value_ : "", kvi));
     ++			res = format_config_int64(buf, key_, value_, kvi, gently);
     + 		else if (opts->type == TYPE_BOOL)
     + 			strbuf_addstr(buf, git_config_bool(key_, value_) ?
     + 				      "true" : "false");
     +@@ builtin/config.c: static int format_config(const struct config_display_options *opts,
     + 		}
     + 	}
     + 	strbuf_addch(buf, opts->term);
     +-	return 0;
     ++	return res;
     + }
       
     - 	if (actions == ACTION_LIST) {
     - 		check_argc(argc, 0, 0);
     -+		display_options_init_list(&display_opts);
     - 		if (config_with_options(show_all_config, &display_opts,
     - 					&location_opts.source, the_repository,
     - 					&location_opts.options) < 0) {
     + static int show_all_config(const char *key_, const char *value_,
  -:  ---------- >  5:  f8e0b8304f config: format bools gently
  -:  ---------- >  6:  0a428d2ffe config: format bools or ints gently
  -:  ---------- >  7:  3fec3abbd6 config: format bools or strings in helper
  2:  8d3a6a8265 !  8:  fafafc5465 parse: add git_parse_maybe_pathname()
     @@ Metadata
       ## Commit message ##
          parse: add git_parse_maybe_pathname()
      
     -    This extraction of logic from config.c's git_config_pathname() allows
     -    for parsing a fully-qualified path from a relative path along with
     -    validation of the existence of the path without failing with a die().
     +    The git_config_pathname() method parses a config value as a path, but
     +    always die()s on an error. Move this logic into a gentler parsing
     +    algorithm that will return an error value instead of ending the process.
      
          Signed-off-by: Derrick Stolee <stolee@gmail.com>
      
  -:  ---------- >  9:  d1cfa0c5e1 config: format paths gently
  -:  ---------- > 10:  9221ca2352 config: format expiry dates gently
  -:  ---------- > 11:  ddf6131ac9 color: add color_parse_gently()
  -:  ---------- > 12:  d14937e6d1 config: format colors gently
  -:  ---------- > 13:  48fc882785 config: restructure format_config()

-- 
gitgitgadget

[PATCH v2 01/13] config: move show_all_config()

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

In anticipation of using format_config() in this method, move
show_all_config() lower in the file without changes.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 builtin/config.c | 48 ++++++++++++++++++++++++------------------------
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 288ebdfdaa..237f7a934d 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -231,30 +231,6 @@ static void show_config_scope(const struct config_display_options *opts,
 	strbuf_addch(buf, term);
 }
 
-static int show_all_config(const char *key_, const char *value_,
-			   const struct config_context *ctx,
-			   void *cb)
-{
-	const struct config_display_options *opts = cb;
-	const struct key_value_info *kvi = ctx->kvi;
-
-	if (opts->show_origin || opts->show_scope) {
-		struct strbuf buf = STRBUF_INIT;
-		if (opts->show_scope)
-			show_config_scope(opts, kvi, &buf);
-		if (opts->show_origin)
-			show_config_origin(opts, kvi, &buf);
-		/* Use fwrite as "buf" can contain \0's if "end_null" is set. */
-		fwrite(buf.buf, 1, buf.len, stdout);
-		strbuf_release(&buf);
-	}
-	if (!opts->omit_values && value_)
-		printf("%s%c%s%c", key_, opts->delim, value_, opts->term);
-	else
-		printf("%s%c", key_, opts->term);
-	return 0;
-}
-
 struct strbuf_list {
 	struct strbuf *items;
 	int nr;
@@ -332,6 +308,30 @@ static int format_config(const struct config_display_options *opts,
 	return 0;
 }
 
+static int show_all_config(const char *key_, const char *value_,
+			   const struct config_context *ctx,
+			   void *cb)
+{
+	const struct config_display_options *opts = cb;
+	const struct key_value_info *kvi = ctx->kvi;
+
+	if (opts->show_origin || opts->show_scope) {
+		struct strbuf buf = STRBUF_INIT;
+		if (opts->show_scope)
+			show_config_scope(opts, kvi, &buf);
+		if (opts->show_origin)
+			show_config_origin(opts, kvi, &buf);
+		/* Use fwrite as "buf" can contain \0's if "end_null" is set. */
+		fwrite(buf.buf, 1, buf.len, stdout);
+		strbuf_release(&buf);
+	}
+	if (!opts->omit_values && value_)
+		printf("%s%c%s%c", key_, opts->delim, value_, opts->term);
+	else
+		printf("%s%c", key_, opts->term);
+	return 0;
+}
+
 #define GET_VALUE_ALL        (1 << 0)
 #define GET_VALUE_KEY_REGEXP (1 << 1)
 
-- 
gitgitgadget

[PATCH v2 02/13] config: add 'gently' parameter to format_config()

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

This parameter is set to 0 for all current callers and is UNUSED.
However, we will start using this option in future changes and in a
critical change that requires gentle parsing (not using die()) to try
parsing all values in a list.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 builtin/config.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 237f7a934d..b4c4228311 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -242,10 +242,14 @@ struct strbuf_list {
  * append it into strbuf `buf`.  Returns a negative value on failure,
  * 0 on success, 1 on a missing optional value (i.e., telling the
  * caller to pretend that <key_,value_> did not exist).
+ *
+ * Note: 'gently' is currently ignored, but will be implemented in
+ * a future change.
  */
 static int format_config(const struct config_display_options *opts,
 			 struct strbuf *buf, const char *key_,
-			 const char *value_, const struct key_value_info *kvi)
+			 const char *value_, const struct key_value_info *kvi,
+			 int gently UNUSED)
 {
 	if (opts->show_scope)
 		show_config_scope(opts, kvi, buf);
@@ -372,7 +376,7 @@ static int collect_config(const char *key_, const char *value_,
 	strbuf_init(&values->items[values->nr], 0);
 
 	status = format_config(data->display_opts, &values->items[values->nr++],
-			       key_, value_, kvi);
+			       key_, value_, kvi, 0);
 	if (status < 0)
 		return status;
 	if (status) {
@@ -463,7 +467,7 @@ static int get_value(const struct config_location_options *opts,
 		strbuf_init(item, 0);
 
 		status = format_config(display_opts, item, key_,
-				       display_opts->default_value, &kvi);
+				       display_opts->default_value, &kvi, 0);
 		if (status < 0)
 			die(_("failed to format default config value: %s"),
 			    display_opts->default_value);
@@ -743,7 +747,7 @@ static int get_urlmatch(const struct config_location_options *opts,
 
 		status = format_config(&display_opts, &buf, item->string,
 				       matched->value_is_null ? NULL : matched->value.buf,
-				       &matched->kvi);
+				       &matched->kvi, 0);
 		if (!status)
 			fwrite(buf.buf, 1, buf.len, stdout);
 		strbuf_release(&buf);
-- 
gitgitgadget

[PATCH v2 03/13] config: make 'git config list --type=<X>' work

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

Previously, the --type=<X> argument to 'git config list' was ignored and
did nothing. Now, we add the use of format_config() to the
show_all_config() function so each key-value pair is attempted to be
parsed. This is our first use of the 'gently' parameter with a nonzero
value.

When listing multiple values, our initial settings for the output format
is different. Add a new init helper to specify the fact that keys should
be shown and also add the default delimiters as they were unset in some
cases.

If there is an error in parsing, then the row is not output.

This is a change in behavior! We are starting to respect an option that
was previously ignored, leading to potential user confusion. This is
probably still a good option, since the --type argument did not change
behavior at all previously, so users can get the behavior they expect by
removing the --type argument or adding the --no-type argument.

t1300-config.sh is updated with the current behavior of this formatting
logic to justify the upcoming refactoring of format_config() that will
incrementally fix some of these cases to be more user-friendly.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 Documentation/git-config.adoc |  3 ++
 builtin/config.c              | 35 ++++++++++--------
 t/t1300-config.sh             | 70 ++++++++++++++++++++++++++++++++++-
 3 files changed, 92 insertions(+), 16 deletions(-)

diff --git a/Documentation/git-config.adoc b/Documentation/git-config.adoc
index ac3b536a15..5300dd4c51 100644
--- a/Documentation/git-config.adoc
+++ b/Documentation/git-config.adoc
@@ -240,6 +240,9 @@ Valid `<type>`'s include:
   that the given value is canonicalize-able as an ANSI color, but it is written
   as-is.
 +
+If the command is in `list` mode, then the `--type <type>` argument will apply
+to each listed config value. If the value does not successfully parse in that
+format, then it will be omitted from the list.
 
 --bool::
 --int::
diff --git a/builtin/config.c b/builtin/config.c
index b4c4228311..4c4c791883 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -318,21 +318,12 @@ static int show_all_config(const char *key_, const char *value_,
 {
 	const struct config_display_options *opts = cb;
 	const struct key_value_info *kvi = ctx->kvi;
+	struct strbuf formatted = STRBUF_INIT;
 
-	if (opts->show_origin || opts->show_scope) {
-		struct strbuf buf = STRBUF_INIT;
-		if (opts->show_scope)
-			show_config_scope(opts, kvi, &buf);
-		if (opts->show_origin)
-			show_config_origin(opts, kvi, &buf);
-		/* Use fwrite as "buf" can contain \0's if "end_null" is set. */
-		fwrite(buf.buf, 1, buf.len, stdout);
-		strbuf_release(&buf);
-	}
-	if (!opts->omit_values && value_)
-		printf("%s%c%s%c", key_, opts->delim, value_, opts->term);
-	else
-		printf("%s%c", key_, opts->term);
+	if (format_config(opts, &formatted, key_, value_, kvi, 1) >= 0)
+		fwrite(formatted.buf, 1, formatted.len, stdout);
+
+	strbuf_release(&formatted);
 	return 0;
 }
 
@@ -872,6 +863,19 @@ static void display_options_init(struct config_display_options *opts)
 	}
 }
 
+static void display_options_init_list(struct config_display_options *opts)
+{
+	opts->show_keys = 1;
+
+	if (opts->end_nul) {
+		display_options_init(opts);
+	} else {
+		opts->term = '\n';
+		opts->delim = ' ';
+		opts->key_delim = '=';
+	}
+}
+
 static int cmd_config_list(int argc, const char **argv, const char *prefix,
 			   struct repository *repo UNUSED)
 {
@@ -890,7 +894,7 @@ static int cmd_config_list(int argc, const char **argv, const char *prefix,
 	check_argc(argc, 0, 0);
 
 	location_options_init(&location_opts, prefix);
-	display_options_init(&display_opts);
+	display_options_init_list(&display_opts);
 
 	setup_auto_pager("config", 1);
 
@@ -1321,6 +1325,7 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
 
 	if (actions == ACTION_LIST) {
 		check_argc(argc, 0, 0);
+		display_options_init_list(&display_opts);
 		if (config_with_options(show_all_config, &display_opts,
 					&location_opts.source, the_repository,
 					&location_opts.options) < 0) {
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 9850fcd5b5..362e580604 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -2459,9 +2459,13 @@ done
 
 cat >.git/config <<-\EOF &&
 [section]
-foo = true
+foo = True
 number = 10
 big = 1M
+path = ~/dir
+red = red
+blue = Blue
+date = Fri Jun 4 15:46:55 2010
 EOF
 
 test_expect_success 'identical modern --type specifiers are allowed' '
@@ -2503,6 +2507,70 @@ test_expect_success 'unset type specifiers may be reset to conflicting ones' '
 	test_cmp_config 1048576 --type=bool --no-type --type=int section.big
 '
 
+test_expect_success 'list --type=bool shows only canonicalizable bool values' '
+	cat >expect <<-EOF &&
+	section.foo=true
+	section.number=true
+	section.big=true
+	EOF
+
+	test_must_fail git config ${mode_prefix}list --type=bool
+'
+
+test_expect_success 'list --type=path shows only canonicalizable path values' '
+	cat >expect <<-EOF &&
+	section.foo=True
+	section.number=10
+	section.big=1M
+	section.path=$HOME/dir
+	section.red=red
+	section.blue=Blue
+	section.date=Fri Jun 4 15:46:55 2010
+	EOF
+
+	git config ${mode_prefix}list --type=path >actual 2>err &&
+	test_cmp expect actual &&
+	test_must_be_empty err
+'
+
+test_expect_success 'list --type=expiry-date shows only canonicalizable dates' '
+	cat >expecterr <<-EOF &&
+	error: '\''True'\'' for '\''section.foo'\'' is not a valid timestamp
+	error: '\''~/dir'\'' for '\''section.path'\'' is not a valid timestamp
+	error: '\''red'\'' for '\''section.red'\'' is not a valid timestamp
+	error: '\''Blue'\'' for '\''section.blue'\'' is not a valid timestamp
+	EOF
+
+	git config ${mode_prefix}list --type=expiry-date >actual 2>err &&
+
+	# section.number and section.big parse as relative dates that could
+	# have clock skew in their results.
+	test_grep section.big actual &&
+	test_grep section.number actual &&
+	test_grep "section.date=$(git config --type=expiry-date section.$key)" actual &&
+	test_cmp expecterr err
+'
+
+test_expect_success 'list --type=color shows only canonicalizable color values' '
+	cat >expect <<-EOF &&
+	section.number=<>
+	section.red=<RED>
+	section.blue=<BLUE>
+	EOF
+
+	cat >expecterr <<-EOF &&
+	error: invalid color value: True
+	error: invalid color value: 1M
+	error: invalid color value: ~/dir
+	error: invalid color value: Fri Jun 4 15:46:55 2010
+	EOF
+
+	git config ${mode_prefix}list --type=color >actual.raw 2>err &&
+	test_decode_color <actual.raw >actual &&
+	test_cmp expect actual &&
+	test_cmp expecterr err
+'
+
 test_expect_success '--type rejects unknown specifiers' '
 	test_must_fail git config --type=nonsense section.foo 2>error &&
 	test_grep "unrecognized --type argument" error
-- 
gitgitgadget

[PATCH v2 04/13] config: format int64s gently

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

Move the logic for formatting int64 config values into a helper method
and use gentle parsing when needed.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 builtin/config.c | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 4c4c791883..d259a91d53 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -237,6 +237,25 @@ struct strbuf_list {
 	int alloc;
 };
 
+static int format_config_int64(struct strbuf *buf,
+			       const char *key_,
+			       const char *value_,
+			       const struct key_value_info *kvi,
+			       int gently)
+{
+	int64_t v = 0;
+	if (gently) {
+		if (git_parse_int64(value_, &v))
+			return -1;
+	} else {
+		/* may die() */
+		v = git_config_int64(key_, value_ ? value_ : "", kvi);
+	}
+
+	strbuf_addf(buf, "%"PRId64, v);
+	return 0;
+}
+
 /*
  * Format the configuration key-value pair (`key_`, `value_`) and
  * append it into strbuf `buf`.  Returns a negative value on failure,
@@ -249,8 +268,9 @@ struct strbuf_list {
 static int format_config(const struct config_display_options *opts,
 			 struct strbuf *buf, const char *key_,
 			 const char *value_, const struct key_value_info *kvi,
-			 int gently UNUSED)
+			 int gently)
 {
+	int res = 0;
 	if (opts->show_scope)
 		show_config_scope(opts, kvi, buf);
 	if (opts->show_origin)
@@ -262,8 +282,7 @@ static int format_config(const struct config_display_options *opts,
 			strbuf_addch(buf, opts->key_delim);
 
 		if (opts->type == TYPE_INT)
-			strbuf_addf(buf, "%"PRId64,
-				    git_config_int64(key_, value_ ? value_ : "", kvi));
+			res = format_config_int64(buf, key_, value_, kvi, gently);
 		else if (opts->type == TYPE_BOOL)
 			strbuf_addstr(buf, git_config_bool(key_, value_) ?
 				      "true" : "false");
@@ -309,7 +328,7 @@ static int format_config(const struct config_display_options *opts,
 		}
 	}
 	strbuf_addch(buf, opts->term);
-	return 0;
+	return res;
 }
 
 static int show_all_config(const char *key_, const char *value_,
-- 
gitgitgadget

[PATCH v2 05/13] config: format bools gently

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

Move the logic for formatting bool config values into a helper method
and use gentle parsing when needed.

This makes 'git config list --type=bool' not fail when coming across a
non-boolean value. Such unparseable values are filtered out quietly.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 builtin/config.c  | 21 +++++++++++++++++++--
 t/t1300-config.sh |  4 +++-
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index d259a91d53..2c169fc126 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -256,6 +256,24 @@ static int format_config_int64(struct strbuf *buf,
 	return 0;
 }
 
+static int format_config_bool(struct strbuf *buf,
+			      const char *key_,
+			      const char *value_,
+			      int gently)
+{
+	int v = 0;
+	if (gently) {
+		if ((v = git_parse_maybe_bool(value_)) < 0)
+			return -1;
+	} else {
+		/* may die() */
+		v = git_config_bool(key_, value_);
+	}
+
+	strbuf_addstr(buf, v ? "true" : "false");
+	return 0;
+}
+
 /*
  * Format the configuration key-value pair (`key_`, `value_`) and
  * append it into strbuf `buf`.  Returns a negative value on failure,
@@ -284,8 +302,7 @@ static int format_config(const struct config_display_options *opts,
 		if (opts->type == TYPE_INT)
 			res = format_config_int64(buf, key_, value_, kvi, gently);
 		else if (opts->type == TYPE_BOOL)
-			strbuf_addstr(buf, git_config_bool(key_, value_) ?
-				      "true" : "false");
+			res = format_config_bool(buf, key_, value_, gently);
 		else if (opts->type == TYPE_BOOL_OR_INT) {
 			int is_bool, v;
 			v = git_config_bool_or_int(key_, value_, kvi,
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 362e580604..59a82b9aef 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -2514,7 +2514,9 @@ test_expect_success 'list --type=bool shows only canonicalizable bool values' '
 	section.big=true
 	EOF
 
-	test_must_fail git config ${mode_prefix}list --type=bool
+	git config ${mode_prefix}list --type=bool >actual 2>err &&
+	test_cmp expect actual &&
+	test_must_be_empty err
 '
 
 test_expect_success 'list --type=path shows only canonicalizable path values' '
-- 
gitgitgadget

[PATCH v2 06/13] config: format bools or ints gently

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

Move the logic for formatting bool-or-int config values into a helper
method and use gentle parsing when needed.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 builtin/config.c | 40 +++++++++++++++++++++++++++++++---------
 1 file changed, 31 insertions(+), 9 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 2c169fc126..2c93e1725b 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -274,6 +274,34 @@ static int format_config_bool(struct strbuf *buf,
 	return 0;
 }
 
+static int format_config_bool_or_int(struct strbuf *buf,
+				     const char *key_,
+				     const char *value_,
+				     const struct key_value_info *kvi,
+				     int gently)
+{
+	int v, is_bool = 0;
+
+	if (gently) {
+		v = git_parse_maybe_bool_text(value_);
+
+		if (v >= 0)
+			is_bool = 1;
+		else if (git_parse_int(value_, &v))
+			return -1;
+	} else {
+		v = git_config_bool_or_int(key_, value_, kvi,
+					   &is_bool);
+	}
+
+	if (is_bool)
+		strbuf_addstr(buf, v ? "true" : "false");
+	else
+		strbuf_addf(buf, "%d", v);
+
+	return 0;
+}
+
 /*
  * Format the configuration key-value pair (`key_`, `value_`) and
  * append it into strbuf `buf`.  Returns a negative value on failure,
@@ -303,15 +331,9 @@ static int format_config(const struct config_display_options *opts,
 			res = format_config_int64(buf, key_, value_, kvi, gently);
 		else if (opts->type == TYPE_BOOL)
 			res = format_config_bool(buf, key_, value_, gently);
-		else if (opts->type == TYPE_BOOL_OR_INT) {
-			int is_bool, v;
-			v = git_config_bool_or_int(key_, value_, kvi,
-						   &is_bool);
-			if (is_bool)
-				strbuf_addstr(buf, v ? "true" : "false");
-			else
-				strbuf_addf(buf, "%d", v);
-		} else if (opts->type == TYPE_BOOL_OR_STR) {
+		else if (opts->type == TYPE_BOOL_OR_INT)
+			res = format_config_bool_or_int(buf, key_, value_, kvi, gently);
+		else if (opts->type == TYPE_BOOL_OR_STR) {
 			int v = git_parse_maybe_bool(value_);
 			if (v < 0)
 				strbuf_addstr(buf, value_);
-- 
gitgitgadget

[PATCH v2 07/13] config: format bools or strings in helper

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

Move the logic for formatting bool-or-string config values into a
helper. This parsing has always been gentle, so this is not unlocking
new behavior. This extraction is only to match the formatting of the
other cases that do need a behavior change.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 builtin/config.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 2c93e1725b..0c539ff98e 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -302,6 +302,18 @@ static int format_config_bool_or_int(struct strbuf *buf,
 	return 0;
 }
 
+/* This mode is always gentle. */
+static int format_config_bool_or_str(struct strbuf *buf,
+				     const char *value_)
+{
+	int v = git_parse_maybe_bool(value_);
+	if (v < 0)
+		strbuf_addstr(buf, value_);
+	else
+		strbuf_addstr(buf, v ? "true" : "false");
+	return 0;
+}
+
 /*
  * Format the configuration key-value pair (`key_`, `value_`) and
  * append it into strbuf `buf`.  Returns a negative value on failure,
@@ -333,13 +345,9 @@ static int format_config(const struct config_display_options *opts,
 			res = format_config_bool(buf, key_, value_, gently);
 		else if (opts->type == TYPE_BOOL_OR_INT)
 			res = format_config_bool_or_int(buf, key_, value_, kvi, gently);
-		else if (opts->type == TYPE_BOOL_OR_STR) {
-			int v = git_parse_maybe_bool(value_);
-			if (v < 0)
-				strbuf_addstr(buf, value_);
-			else
-				strbuf_addstr(buf, v ? "true" : "false");
-		} else if (opts->type == TYPE_PATH) {
+		else if (opts->type == TYPE_BOOL_OR_STR)
+			res = format_config_bool_or_str(buf, value_);
+		else if (opts->type == TYPE_PATH) {
 			char *v;
 			if (git_config_pathname(&v, key_, value_) < 0)
 				return -1;
-- 
gitgitgadget

[PATCH v2 08/13] parse: add git_parse_maybe_pathname()

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

The git_config_pathname() method parses a config value as a path, but
always die()s on an error. Move this logic into a gentler parsing
algorithm that will return an error value instead of ending the process.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 config.c | 14 +-------------
 parse.c  | 24 ++++++++++++++++++++++++
 parse.h  |  2 ++
 3 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/config.c b/config.c
index 7f6d53b473..83257b7a97 100644
--- a/config.c
+++ b/config.c
@@ -1278,24 +1278,12 @@ int git_config_string(char **dest, const char *var, const char *value)
 
 int git_config_pathname(char **dest, const char *var, const char *value)
 {
-	bool is_optional;
-	char *path;
-
 	if (!value)
 		return config_error_nonbool(var);
 
-	is_optional = skip_prefix(value, ":(optional)", &value);
-	path = interpolate_path(value, 0);
-	if (!path)
+	if (git_parse_maybe_pathname(value, dest) < 0)
 		die(_("failed to expand user dir in: '%s'"), value);
 
-	if (is_optional && is_missing_file(path)) {
-		free(path);
-		*dest = NULL;
-		return 0;
-	}
-
-	*dest = path;
 	return 0;
 }
 
diff --git a/parse.c b/parse.c
index 48313571aa..3f37f0b93a 100644
--- a/parse.c
+++ b/parse.c
@@ -1,6 +1,7 @@
 #include "git-compat-util.h"
 #include "gettext.h"
 #include "parse.h"
+#include "path.h"
 
 static uintmax_t get_unit_factor(const char *end)
 {
@@ -209,3 +210,26 @@ unsigned long git_env_ulong(const char *k, unsigned long val)
 		die(_("failed to parse %s"), k);
 	return val;
 }
+
+int git_parse_maybe_pathname(const char *value, char **dest)
+{
+	bool is_optional;
+	char *path;
+
+	if (!value)
+		return -1;
+
+	is_optional = skip_prefix(value, ":(optional)", &value);
+	path = interpolate_path(value, 0);
+	if (!path)
+		return -1;
+
+	if (is_optional && is_missing_file(path)) {
+		free(path);
+		*dest = NULL;
+		return 0;
+	}
+
+	*dest = path;
+	return 0;
+}
diff --git a/parse.h b/parse.h
index ea32de9a91..4f97c3727a 100644
--- a/parse.h
+++ b/parse.h
@@ -19,4 +19,6 @@ int git_parse_maybe_bool_text(const char *value);
 int git_env_bool(const char *, int);
 unsigned long git_env_ulong(const char *, unsigned long);
 
+int git_parse_maybe_pathname(const char *value, char **dest);
+
 #endif /* PARSE_H */
-- 
gitgitgadget

[PATCH v2 09/13] config: format paths gently

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

Move the logic for formatting path config values into a helper method
and use gentle parsing when needed.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 builtin/config.c | 35 +++++++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 0c539ff98e..4664651dd2 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -314,6 +314,28 @@ static int format_config_bool_or_str(struct strbuf *buf,
 	return 0;
 }
 
+static int format_config_path(struct strbuf *buf,
+			      const char *key_,
+			      const char *value_,
+			      int gently)
+{
+	char *v;
+	if (gently) {
+		if (git_parse_maybe_pathname(value_, &v) < 0)
+			return -1;
+	} else if (git_config_pathname(&v, key_, value_) < 0) {
+		return -1;
+	}
+
+	if (v)
+		strbuf_addstr(buf, v);
+	else
+		return 1; /* :(optional)no-such-file */
+
+	free(v);
+	return 0;
+}
+
 /*
  * Format the configuration key-value pair (`key_`, `value_`) and
  * append it into strbuf `buf`.  Returns a negative value on failure,
@@ -347,16 +369,9 @@ static int format_config(const struct config_display_options *opts,
 			res = format_config_bool_or_int(buf, key_, value_, kvi, gently);
 		else if (opts->type == TYPE_BOOL_OR_STR)
 			res = format_config_bool_or_str(buf, value_);
-		else if (opts->type == TYPE_PATH) {
-			char *v;
-			if (git_config_pathname(&v, key_, value_) < 0)
-				return -1;
-			if (v)
-				strbuf_addstr(buf, v);
-			else
-				return 1; /* :(optional)no-such-file */
-			free((char *)v);
-		} else if (opts->type == TYPE_EXPIRY_DATE) {
+		else if (opts->type == TYPE_PATH)
+			res = format_config_path(buf, key_, value_, gently);
+		else if (opts->type == TYPE_EXPIRY_DATE) {
 			timestamp_t t;
 			if (git_config_expiry_date(&t, key_, value_) < 0)
 				return -1;
-- 
gitgitgadget

[PATCH v2 10/13] config: format expiry dates gently

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

Move the logic for formatting expiry date config values into a helper
method and use gentle parsing when needed.

There is an unfortunate asymmetry in these two parsing methods, but we
need to treat a positive response from parse_expiry_date() as an error
or we will get incorrect values.

This updates the behavior of 'git config list --type=expiry-date' to be
quiet when attempting parsing on non-date values.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 builtin/config.c  | 27 +++++++++++++++++++++------
 t/t1300-config.sh |  9 +--------
 2 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 4664651dd2..71b685d943 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -3,6 +3,7 @@
 #include "abspath.h"
 #include "config.h"
 #include "color.h"
+#include "date.h"
 #include "editor.h"
 #include "environment.h"
 #include "gettext.h"
@@ -336,6 +337,23 @@ static int format_config_path(struct strbuf *buf,
 	return 0;
 }
 
+static int format_config_expiry_date(struct strbuf *buf,
+				     const char *key_,
+				     const char *value_,
+				     int gently)
+{
+	timestamp_t t;
+	if (gently) {
+		if (parse_expiry_date(value_, &t))
+			return -1;
+	} else if (git_config_expiry_date(&t, key_, value_) < 0) {
+		return -1;
+	}
+
+	strbuf_addf(buf, "%"PRItime, t);
+	return 0;
+}
+
 /*
  * Format the configuration key-value pair (`key_`, `value_`) and
  * append it into strbuf `buf`.  Returns a negative value on failure,
@@ -371,12 +389,9 @@ static int format_config(const struct config_display_options *opts,
 			res = format_config_bool_or_str(buf, value_);
 		else if (opts->type == TYPE_PATH)
 			res = format_config_path(buf, key_, value_, gently);
-		else if (opts->type == TYPE_EXPIRY_DATE) {
-			timestamp_t t;
-			if (git_config_expiry_date(&t, key_, value_) < 0)
-				return -1;
-			strbuf_addf(buf, "%"PRItime, t);
-		} else if (opts->type == TYPE_COLOR) {
+		else if (opts->type == TYPE_EXPIRY_DATE)
+			res = format_config_expiry_date(buf, key_, value_, gently);
+		else if (opts->type == TYPE_COLOR) {
 			char v[COLOR_MAXLEN];
 			if (git_config_color(v, key_, value_) < 0)
 				return -1;
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 59a82b9aef..c134d85d8a 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -2536,13 +2536,6 @@ test_expect_success 'list --type=path shows only canonicalizable path values' '
 '
 
 test_expect_success 'list --type=expiry-date shows only canonicalizable dates' '
-	cat >expecterr <<-EOF &&
-	error: '\''True'\'' for '\''section.foo'\'' is not a valid timestamp
-	error: '\''~/dir'\'' for '\''section.path'\'' is not a valid timestamp
-	error: '\''red'\'' for '\''section.red'\'' is not a valid timestamp
-	error: '\''Blue'\'' for '\''section.blue'\'' is not a valid timestamp
-	EOF
-
 	git config ${mode_prefix}list --type=expiry-date >actual 2>err &&
 
 	# section.number and section.big parse as relative dates that could
@@ -2550,7 +2543,7 @@ test_expect_success 'list --type=expiry-date shows only canonicalizable dates' '
 	test_grep section.big actual &&
 	test_grep section.number actual &&
 	test_grep "section.date=$(git config --type=expiry-date section.$key)" actual &&
-	test_cmp expecterr err
+	test_must_be_empty err
 '
 
 test_expect_success 'list --type=color shows only canonicalizable color values' '
-- 
gitgitgadget

[PATCH v2 11/13] color: add color_parse_gently()

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

When parsing colors, a failed parse leads to an error message due to the
result returning error(). To allow for quiet parsing, create
color_parse_gently().

To accomplish this, convert the implementation of color_parse_mem() into
a static color_parse_mem_1() helper that adds a 'gently' parameter. The
color_parse_gently() method can then use this. Since it is a near
equivalent to color_parse(), move that method down in the file so they
can be nearby while also appearing after color_parse_mem_1().

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 color.c | 25 ++++++++++++++++++-------
 color.h |  1 +
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/color.c b/color.c
index 07ac8c9d40..ec8872d2dd 100644
--- a/color.c
+++ b/color.c
@@ -223,11 +223,6 @@ static int parse_attr(const char *name, size_t len)
 	return -1;
 }
 
-int color_parse(const char *value, char *dst)
-{
-	return color_parse_mem(value, strlen(value), dst);
-}
-
 /*
  * Write the ANSI color codes for "c" to "out"; the string should
  * already have the ANSI escape code in it. "out" should have enough
@@ -264,7 +259,8 @@ static int color_empty(const struct color *c)
 	return c->type <= COLOR_NORMAL;
 }
 
-int color_parse_mem(const char *value, int value_len, char *dst)
+static int color_parse_mem_1(const char *value, int value_len,
+			     char *dst, int gently)
 {
 	const char *ptr = value;
 	int len = value_len;
@@ -365,10 +361,25 @@ int color_parse_mem(const char *value, int value_len, char *dst)
 	OUT(0);
 	return 0;
 bad:
-	return error(_("invalid color value: %.*s"), value_len, value);
+	return gently ? -1 : error(_("invalid color value: %.*s"), value_len, value);
 #undef OUT
 }
 
+int color_parse_mem(const char *value, int value_len, char *dst)
+{
+	return color_parse_mem_1(value, value_len, dst, 0);
+}
+
+int color_parse(const char *value, char *dst)
+{
+	return color_parse_mem(value, strlen(value), dst);
+}
+
+int color_parse_gently(const char *value, char *dst)
+{
+	return color_parse_mem_1(value, strlen(value), dst, 1);
+}
+
 enum git_colorbool git_config_colorbool(const char *var, const char *value)
 {
 	if (value) {
diff --git a/color.h b/color.h
index 43e6c9ad09..30c783405d 100644
--- a/color.h
+++ b/color.h
@@ -118,6 +118,7 @@ bool want_color_fd(int fd, enum git_colorbool var);
  * terminal.
  */
 int color_parse(const char *value, char *dst);
+int color_parse_gently(const char *value, char *dst);
 int color_parse_mem(const char *value, int len, char *dst);
 
 /*
-- 
gitgitgadget

[PATCH v2 12/13] config: format colors gently

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

Move the logic for formatting color config value into a helper method
and use gentle parsing when needed.

This removes error messages when parsing a list of config values that do
not match color formats.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 builtin/config.c  | 27 +++++++++++++++++++++------
 t/t1300-config.sh |  9 +--------
 2 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 71b685d943..e8c02e5f21 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -354,6 +354,24 @@ static int format_config_expiry_date(struct strbuf *buf,
 	return 0;
 }
 
+static int format_config_color(struct strbuf *buf,
+			       const char *key_,
+			       const char *value_,
+			       int gently)
+{
+	char v[COLOR_MAXLEN];
+
+	if (gently) {
+		if (color_parse_gently(value_, v) < 0)
+			return -1;
+	} else if (git_config_color(v, key_, value_) < 0) {
+		return -1;
+	}
+
+	strbuf_addstr(buf, v);
+	return 0;
+}
+
 /*
  * Format the configuration key-value pair (`key_`, `value_`) and
  * append it into strbuf `buf`.  Returns a negative value on failure,
@@ -391,12 +409,9 @@ static int format_config(const struct config_display_options *opts,
 			res = format_config_path(buf, key_, value_, gently);
 		else if (opts->type == TYPE_EXPIRY_DATE)
 			res = format_config_expiry_date(buf, key_, value_, gently);
-		else if (opts->type == TYPE_COLOR) {
-			char v[COLOR_MAXLEN];
-			if (git_config_color(v, key_, value_) < 0)
-				return -1;
-			strbuf_addstr(buf, v);
-		} else if (value_) {
+		else if (opts->type == TYPE_COLOR)
+			res = format_config_color(buf, key_, value_, gently);
+		else if (value_) {
 			strbuf_addstr(buf, value_);
 		} else {
 			/* Just show the key name; back out delimiter */
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index c134d85d8a..79b2ee203c 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -2553,17 +2553,10 @@ test_expect_success 'list --type=color shows only canonicalizable color values'
 	section.blue=<BLUE>
 	EOF
 
-	cat >expecterr <<-EOF &&
-	error: invalid color value: True
-	error: invalid color value: 1M
-	error: invalid color value: ~/dir
-	error: invalid color value: Fri Jun 4 15:46:55 2010
-	EOF
-
 	git config ${mode_prefix}list --type=color >actual.raw 2>err &&
 	test_decode_color <actual.raw >actual &&
 	test_cmp expect actual &&
-	test_cmp expecterr err
+	test_must_be_empty err
 '
 
 test_expect_success '--type rejects unknown specifiers' '
-- 
gitgitgadget

[PATCH v2 13/13] config: restructure format_config()

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

The recent changes have replaced the bodies of most if/else-if cases
with simple helper method calls. This makes it easy to adapt the
structure into a clearer switch statement, leaving a simple if/else in
the default case.

Make things a little simpler to read by reducing the nesting depth via a
new goto statement when we want to skip values.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 builtin/config.c | 59 ++++++++++++++++++++++++++++++++----------------
 1 file changed, 40 insertions(+), 19 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index e8c02e5f21..1de3ce0eaa 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -393,25 +393,44 @@ static int format_config(const struct config_display_options *opts,
 		show_config_origin(opts, kvi, buf);
 	if (opts->show_keys)
 		strbuf_addstr(buf, key_);
-	if (!opts->omit_values) {
-		if (opts->show_keys)
-			strbuf_addch(buf, opts->key_delim);
-
-		if (opts->type == TYPE_INT)
-			res = format_config_int64(buf, key_, value_, kvi, gently);
-		else if (opts->type == TYPE_BOOL)
-			res = format_config_bool(buf, key_, value_, gently);
-		else if (opts->type == TYPE_BOOL_OR_INT)
-			res = format_config_bool_or_int(buf, key_, value_, kvi, gently);
-		else if (opts->type == TYPE_BOOL_OR_STR)
-			res = format_config_bool_or_str(buf, value_);
-		else if (opts->type == TYPE_PATH)
-			res = format_config_path(buf, key_, value_, gently);
-		else if (opts->type == TYPE_EXPIRY_DATE)
-			res = format_config_expiry_date(buf, key_, value_, gently);
-		else if (opts->type == TYPE_COLOR)
-			res = format_config_color(buf, key_, value_, gently);
-		else if (value_) {
+
+	if (opts->omit_values)
+		goto terminator;
+
+	if (opts->show_keys)
+		strbuf_addch(buf, opts->key_delim);
+
+	switch (opts->type) {
+	case TYPE_INT:
+		res = format_config_int64(buf, key_, value_, kvi, gently);
+		break;
+
+	case TYPE_BOOL:
+		res = format_config_bool(buf, key_, value_, gently);
+		break;
+
+	case TYPE_BOOL_OR_INT:
+		res = format_config_bool_or_int(buf, key_, value_, kvi, gently);
+		break;
+
+	case TYPE_BOOL_OR_STR:
+		res = format_config_bool_or_str(buf, value_);
+		break;
+
+	case TYPE_PATH:
+		res = format_config_path(buf, key_, value_, gently);
+		break;
+
+	case TYPE_EXPIRY_DATE:
+		res = format_config_expiry_date(buf, key_, value_, gently);
+		break;
+
+	case TYPE_COLOR:
+		res = format_config_color(buf, key_, value_, gently);
+		break;
+
+	default:
+		if (value_) {
 			strbuf_addstr(buf, value_);
 		} else {
 			/* Just show the key name; back out delimiter */
@@ -419,6 +438,8 @@ static int format_config(const struct config_display_options *opts,
 				strbuf_setlen(buf, buf->len - 1);
 		}
 	}
+
+terminator:
 	strbuf_addch(buf, opts->term);
 	return res;
 }
-- 
gitgitgadget

Re: [PATCH v2 04/13] config: format int64s gently

[Junio C Hamano]

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> +static int format_config_int64(struct strbuf *buf,
> +			       const char *key_,
> +			       const char *value_,
> +			       const struct key_value_info *kvi,
> +			       int gently)
> +{
> +	int64_t v = 0;
> +	if (gently) {
> +		if (git_parse_int64(value_, &v))
> +			return -1;
> +	} else {
> +		/* may die() */
> +		v = git_config_int64(key_, value_ ? value_ : "", kvi);
> +	}
> +
> +	strbuf_addf(buf, "%"PRId64, v);
> +	return 0;
> +}

This establishes the pattern the next handful of patches follow.  We
already have in parse.c helpers that we can use for the gentler
parsing, and otherwise we'd use git_config_*() that the caller of
these new helpers were using originally.

I'd have preferred to have the blank line moved to the gap between
the decl and the first statement, i.e.,

> +{
> +	int64_t v = 0;
> +
> +	if (gently) {
> +		if (git_parse_int64(value_, &v))
> +			return -1;
> +	} else {
> +		/* may die() */
> +		v = git_config_int64(key_, value_ ? value_ : "", kvi);
> +	}
> +	strbuf_addf(buf, "%"PRId64, v);
> +	return 0;
> +}

These "format X gently" steps look very good.

Re: [PATCH v2 03/13] config: make 'git config list --type=<X>' work

[Patrick Steinhardt]

On Fri, Feb 13, 2026 at 11:55:08PM +0000, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <stolee@gmail.com>
> 
> Previously, the --type=<X> argument to 'git config list' was ignored and
> did nothing. Now, we add the use of format_config() to the
> show_all_config() function so each key-value pair is attempted to be
> parsed. This is our first use of the 'gently' parameter with a nonzero
> value.
> 
> When listing multiple values, our initial settings for the output format
> is different. Add a new init helper to specify the fact that keys should
> be shown and also add the default delimiters as they were unset in some
> cases.
> 
> If there is an error in parsing, then the row is not output.

It might make sense to document the rationale behind this decision in
the commit message.

> diff --git a/builtin/config.c b/builtin/config.c
> index b4c4228311..4c4c791883 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -318,21 +318,12 @@ static int show_all_config(const char *key_, const char *value_,
>  {
>  	const struct config_display_options *opts = cb;
>  	const struct key_value_info *kvi = ctx->kvi;
> +	struct strbuf formatted = STRBUF_INIT;
>  
> -	if (opts->show_origin || opts->show_scope) {
> -		struct strbuf buf = STRBUF_INIT;
> -		if (opts->show_scope)
> -			show_config_scope(opts, kvi, &buf);
> -		if (opts->show_origin)
> -			show_config_origin(opts, kvi, &buf);
> -		/* Use fwrite as "buf" can contain \0's if "end_null" is set. */
> -		fwrite(buf.buf, 1, buf.len, stdout);
> -		strbuf_release(&buf);
> -	}
> -	if (!opts->omit_values && value_)
> -		printf("%s%c%s%c", key_, opts->delim, value_, opts->term);
> -	else
> -		printf("%s%c", key_, opts->term);
> +	if (format_config(opts, &formatted, key_, value_, kvi, 1) >= 0)
> +		fwrite(formatted.buf, 1, formatted.len, stdout);

We could probably use puts(3p) instead, but as we know the length of the
data ahead of time it might be more efficient to use fwrite(3p) indeed.
Ultimately I guess it doesn't matter much.

Patrick

Re: [PATCH v2 02/13] config: add 'gently' parameter to format_config()

[Patrick Steinhardt]

On Fri, Feb 13, 2026 at 11:55:07PM +0000, Derrick Stolee via GitGitGadget wrote:
> diff --git a/builtin/config.c b/builtin/config.c
> index 237f7a934d..b4c4228311 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -242,10 +242,14 @@ struct strbuf_list {
>   * append it into strbuf `buf`.  Returns a negative value on failure,
>   * 0 on success, 1 on a missing optional value (i.e., telling the
>   * caller to pretend that <key_,value_> did not exist).
> + *
> + * Note: 'gently' is currently ignored, but will be implemented in
> + * a future change.
>   */
>  static int format_config(const struct config_display_options *opts,
>  			 struct strbuf *buf, const char *key_,
> -			 const char *value_, const struct key_value_info *kvi)
> +			 const char *value_, const struct key_value_info *kvi,
> +			 int gently UNUSED)

I'd propose to either make this a bool, or turn it into an enum flag so
that it becomes easier to see at the callsite what the magic "true" or
"1" means:

    enum format_config_flags {
        /*
         * Do not die in case the value cannot be parsed properly, but
         * return an error instead.
         */
        FORMAT_CONFIG_GENTLY = (1 << 0),
    };

    format_config(opts, buf, key, value, kv, FORMAT_CONFIG_GENTLY);

I personally prefer this option over using a bool, even though it's a
bit more verbose.

Patrick

Re: [PATCH v2 04/13] config: format int64s gently

[Patrick Steinhardt]

On Fri, Feb 13, 2026 at 11:55:09PM +0000, Derrick Stolee via GitGitGadget wrote:
> diff --git a/builtin/config.c b/builtin/config.c
> index 4c4c791883..d259a91d53 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -237,6 +237,25 @@ struct strbuf_list {
>  	int alloc;
>  };
>  
> +static int format_config_int64(struct strbuf *buf,
> +			       const char *key_,
> +			       const char *value_,

Why do we have the trailing underscores here?

> @@ -249,8 +268,9 @@ struct strbuf_list {
>  static int format_config(const struct config_display_options *opts,
>  			 struct strbuf *buf, const char *key_,
>  			 const char *value_, const struct key_value_info *kvi,
> -			 int gently UNUSED)
> +			 int gently)
>  {
> +	int res = 0;
>  	if (opts->show_scope)
>  		show_config_scope(opts, kvi, buf);
>  	if (opts->show_origin)
> @@ -262,8 +282,7 @@ static int format_config(const struct config_display_options *opts,
>  			strbuf_addch(buf, opts->key_delim);
>  
>  		if (opts->type == TYPE_INT)
> -			strbuf_addf(buf, "%"PRId64,
> -				    git_config_int64(key_, value_ ? value_ : "", kvi));
> +			res = format_config_int64(buf, key_, value_, kvi, gently);
>  		else if (opts->type == TYPE_BOOL)
>  			strbuf_addstr(buf, git_config_bool(key_, value_) ?
>  				      "true" : "false");
> @@ -309,7 +328,7 @@ static int format_config(const struct config_display_options *opts,
>  		}
>  	}
>  	strbuf_addch(buf, opts->term);
> -	return 0;
> +	return res;
>  }

Okay. We bubble up the return value now, but we know that the return
value will only be different in case `gently != 0`. Otherwise, any error
would cause us to die.

Patrick

Re: [PATCH v2 06/13] config: format bools or ints gently

[Patrick Steinhardt]

On Fri, Feb 13, 2026 at 11:55:11PM +0000, Derrick Stolee via GitGitGadget wrote:
> diff --git a/builtin/config.c b/builtin/config.c
> index 2c169fc126..2c93e1725b 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -274,6 +274,34 @@ static int format_config_bool(struct strbuf *buf,
>  	return 0;
>  }
>  
> +static int format_config_bool_or_int(struct strbuf *buf,
> +				     const char *key_,
> +				     const char *value_,
> +				     const struct key_value_info *kvi,
> +				     int gently)
> +{
> +	int v, is_bool = 0;
> +
> +	if (gently) {
> +		v = git_parse_maybe_bool_text(value_);

This function also returns `1` in case `!value`. Is this intended? I
guess so due to our implicit bool thingy, and `git_config_bool_or_int()`
seems to behave the same.

Patrick

Re: [PATCH v2 09/13] config: format paths gently

[Patrick Steinhardt]

On Fri, Feb 13, 2026 at 11:55:14PM +0000, Derrick Stolee via GitGitGadget wrote:
> diff --git a/builtin/config.c b/builtin/config.c
> index 0c539ff98e..4664651dd2 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -314,6 +314,28 @@ static int format_config_bool_or_str(struct strbuf *buf,
>  	return 0;
>  }
>  
> +static int format_config_path(struct strbuf *buf,
> +			      const char *key_,
> +			      const char *value_,
> +			      int gently)
> +{
> +	char *v;
> +	if (gently) {
> +		if (git_parse_maybe_pathname(value_, &v) < 0)
> +			return -1;
> +	} else if (git_config_pathname(&v, key_, value_) < 0) {
> +		return -1;
> +	}
> +
> +	if (v)
> +		strbuf_addstr(buf, v);
> +	else
> +		return 1; /* :(optional)no-such-file */

Okay, this is the first callsite where we return a vaule `> 0`, if I see
correctly. But in `show_all_config()` we check for `res >= 0`, and if so
we would print the configuration regardless.

But `buf` will now be an empty string. So wouldn't this cause us to
print such an empty string, too? I'm not quite sure whether this
behaviour is intentional or not, or whether I'm missing something here.

In any case, I think this should be documented in the commit message.

Patrick

Re: [PATCH v2 11/13] color: add color_parse_gently()

[Patrick Steinhardt]

On Fri, Feb 13, 2026 at 11:55:16PM +0000, Derrick Stolee via GitGitGadget wrote:
> diff --git a/color.c b/color.c
> index 07ac8c9d40..ec8872d2dd 100644
> --- a/color.c
> +++ b/color.c
> @@ -264,7 +259,8 @@ static int color_empty(const struct color *c)
>  	return c->type <= COLOR_NORMAL;
>  }
>  
> -int color_parse_mem(const char *value, int value_len, char *dst)
> +static int color_parse_mem_1(const char *value, int value_len,
> +			     char *dst, int gently)
>  {
>  	const char *ptr = value;
>  	int len = value_len;
> @@ -365,10 +361,25 @@ int color_parse_mem(const char *value, int value_len, char *dst)
>  	OUT(0);
>  	return 0;
>  bad:
> -	return error(_("invalid color value: %.*s"), value_len, value);
> +	return gently ? -1 : error(_("invalid color value: %.*s"), value_len, value);
>  #undef OUT
>  }

As far as I can see this isn't really about whether or not the function
should be gentle. It's rather whether or not the function should print
an error message when it sees an error.

So should we rename the parameter to `quiet`?

>  
> +int color_parse_mem(const char *value, int value_len, char *dst)
> +{
> +	return color_parse_mem_1(value, value_len, dst, 0);
> +}
> +
> +int color_parse(const char *value, char *dst)
> +{
> +	return color_parse_mem(value, strlen(value), dst);
> +}
> +
> +int color_parse_gently(const char *value, char *dst)
> +{
> +	return color_parse_mem_1(value, strlen(value), dst, 1);
> +}

And if so, this should probably be called `color_parse_quiet()`.

Patrick

Re: [PATCH v2 13/13] config: restructure format_config()

[Patrick Steinhardt]

On Fri, Feb 13, 2026 at 11:55:18PM +0000, Derrick Stolee via GitGitGadget wrote:
> diff --git a/builtin/config.c b/builtin/config.c
> index e8c02e5f21..1de3ce0eaa 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -393,25 +393,44 @@ static int format_config(const struct config_display_options *opts,
>  		show_config_origin(opts, kvi, buf);
>  	if (opts->show_keys)
>  		strbuf_addstr(buf, key_);
> -	if (!opts->omit_values) {
> -		if (opts->show_keys)
> -			strbuf_addch(buf, opts->key_delim);
> -
> -		if (opts->type == TYPE_INT)
> -			res = format_config_int64(buf, key_, value_, kvi, gently);
> -		else if (opts->type == TYPE_BOOL)
> -			res = format_config_bool(buf, key_, value_, gently);
> -		else if (opts->type == TYPE_BOOL_OR_INT)
> -			res = format_config_bool_or_int(buf, key_, value_, kvi, gently);
> -		else if (opts->type == TYPE_BOOL_OR_STR)
> -			res = format_config_bool_or_str(buf, value_);
> -		else if (opts->type == TYPE_PATH)
> -			res = format_config_path(buf, key_, value_, gently);
> -		else if (opts->type == TYPE_EXPIRY_DATE)
> -			res = format_config_expiry_date(buf, key_, value_, gently);
> -		else if (opts->type == TYPE_COLOR)
> -			res = format_config_color(buf, key_, value_, gently);
> -		else if (value_) {
> +
> +	if (opts->omit_values)
> +		goto terminator;
> +
> +	if (opts->show_keys)
> +		strbuf_addch(buf, opts->key_delim);
> +
> +	switch (opts->type) {

I very much prefer this layout. Switches are more verbose, but if you
ask me they are easier to parse.

> +	case TYPE_INT:
> +		res = format_config_int64(buf, key_, value_, kvi, gently);
> +		break;
> +

That being said, I'm not a huge fan of the empty newlines here. But feel
free to ignore.

> +	case TYPE_BOOL:
> +		res = format_config_bool(buf, key_, value_, gently);
> +		break;
> +
> +	case TYPE_BOOL_OR_INT:
> +		res = format_config_bool_or_int(buf, key_, value_, kvi, gently);
> +		break;
> +
> +	case TYPE_BOOL_OR_STR:
> +		res = format_config_bool_or_str(buf, value_);
> +		break;
> +
> +	case TYPE_PATH:
> +		res = format_config_path(buf, key_, value_, gently);
> +		break;
> +
> +	case TYPE_EXPIRY_DATE:
> +		res = format_config_expiry_date(buf, key_, value_, gently);
> +		break;
> +
> +	case TYPE_COLOR:
> +		res = format_config_color(buf, key_, value_, gently);
> +		break;
> +
> +	default:

Should we maybe handle all valid types explicitly and have the `default`
case `BUG()` instead?

Patrick

Re: [PATCH v2 03/13] config: make 'git config list --type=<X>' work

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

>> -	if (!opts->omit_values && value_)
>> -		printf("%s%c%s%c", key_, opts->delim, value_, opts->term);
>> -	else
>> -		printf("%s%c", key_, opts->term);
>> +	if (format_config(opts, &formatted, key_, value_, kvi, 1) >= 0)
>> +		fwrite(formatted.buf, 1, formatted.len, stdout);
>
> We could probably use puts(3p) instead, but as we know the length of the
> data ahead of time it might be more efficient to use fwrite(3p) indeed.
> Ultimately I guess it doesn't matter much.
>
> Patrick

If we are not always doing LF-delimited output, puts(3) would not
help us very much, I suspect.

Re: [PATCH v2 03/13] config: make 'git config list --type=<X>' work

[Patrick Steinhardt]

On Tue, Feb 17, 2026 at 08:11:40AM -0800, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> >> -	if (!opts->omit_values && value_)
> >> -		printf("%s%c%s%c", key_, opts->delim, value_, opts->term);
> >> -	else
> >> -		printf("%s%c", key_, opts->term);
> >> +	if (format_config(opts, &formatted, key_, value_, kvi, 1) >= 0)
> >> +		fwrite(formatted.buf, 1, formatted.len, stdout);
> >
> > We could probably use puts(3p) instead, but as we know the length of the
> > data ahead of time it might be more efficient to use fwrite(3p) indeed.
> > Ultimately I guess it doesn't matter much.
> >
> > Patrick
> 
> If we are not always doing LF-delimited output, puts(3) would not
> help us very much, I suspect.

D'oh, right.

Patrick

Re: [PATCH v2 11/13] color: add color_parse_gently()

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> On Fri, Feb 13, 2026 at 11:55:16PM +0000, Derrick Stolee via GitGitGadget wrote:
>> diff --git a/color.c b/color.c
>> index 07ac8c9d40..ec8872d2dd 100644
>> --- a/color.c
>> +++ b/color.c
>> @@ -264,7 +259,8 @@ static int color_empty(const struct color *c)
>>  	return c->type <= COLOR_NORMAL;
>>  }
>>  
>> -int color_parse_mem(const char *value, int value_len, char *dst)
>> +static int color_parse_mem_1(const char *value, int value_len,
>> +			     char *dst, int gently)
>>  {
>>  	const char *ptr = value;
>>  	int len = value_len;
>> @@ -365,10 +361,25 @@ int color_parse_mem(const char *value, int value_len, char *dst)
>>  	OUT(0);
>>  	return 0;
>>  bad:
>> -	return error(_("invalid color value: %.*s"), value_len, value);
>> +	return gently ? -1 : error(_("invalid color value: %.*s"), value_len, value);
>>  #undef OUT
>>  }
>
> As far as I can see this isn't really about whether or not the function
> should be gentle. It's rather whether or not the function should print
> an error message when it sees an error.

Do you mean that this error() call is not die(), the flag does not
fit the usual "gently" criteria?  In other words, should we make
this call die() if we call it "gently"?

>
> So should we rename the parameter to `quiet`?
>
>>  
>> +int color_parse_mem(const char *value, int value_len, char *dst)
>> +{
>> +	return color_parse_mem_1(value, value_len, dst, 0);
>> +}
>> +
>> +int color_parse(const char *value, char *dst)
>> +{
>> +	return color_parse_mem(value, strlen(value), dst);
>> +}
>> +
>> +int color_parse_gently(const char *value, char *dst)
>> +{
>> +	return color_parse_mem_1(value, strlen(value), dst, 1);
>> +}
>
> And if so, this should probably be called `color_parse_quiet()`.
>
> Patrick

Re: [PATCH v2 11/13] color: add color_parse_gently()

[Derrick Stolee]

On 2/17/26 11:20 AM, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
>> On Fri, Feb 13, 2026 at 11:55:16PM +0000, Derrick Stolee via GitGitGadget wrote:
>>> diff --git a/color.c b/color.c
>>> index 07ac8c9d40..ec8872d2dd 100644
>>> --- a/color.c
>>> +++ b/color.c
>>> @@ -264,7 +259,8 @@ static int color_empty(const struct color *c)
>>>   	return c->type <= COLOR_NORMAL;
>>>   }
>>>   
>>> -int color_parse_mem(const char *value, int value_len, char *dst)
>>> +static int color_parse_mem_1(const char *value, int value_len,
>>> +			     char *dst, int gently)
>>>   {
>>>   	const char *ptr = value;
>>>   	int len = value_len;
>>> @@ -365,10 +361,25 @@ int color_parse_mem(const char *value, int value_len, char *dst)
>>>   	OUT(0);
>>>   	return 0;
>>>   bad:
>>> -	return error(_("invalid color value: %.*s"), value_len, value);
>>> +	return gently ? -1 : error(_("invalid color value: %.*s"), value_len, value);
>>>   #undef OUT
>>>   }
>>
>> As far as I can see this isn't really about whether or not the function
>> should be gentle. It's rather whether or not the function should print
>> an error message when it sees an error.
> 
> Do you mean that this error() call is not die(), the flag does not
> fit the usual "gently" criteria?  In other words, should we make
> this call die() if we call it "gently"?

This is an interesting case where the existing color parsing logic is
not following the typical pattern that uses die() on a failed parse.

If we want to change the behavior to die() later, then that could be
considered, though I don't want to consider the ramifications right now.

I think the easiest "local" fix is to use the 'quiet' way, though it adds
some asymmetry in the config code in how it uses the 'gently' parameter.
Let me give this a try in the next version so we can see how it feels.

Thanks,
-Stolee

Re: [PATCH v2 06/13] config: format bools or ints gently

[Derrick Stolee]

On 2/17/26 4:05 AM, Patrick Steinhardt wrote:
> On Fri, Feb 13, 2026 at 11:55:11PM +0000, Derrick Stolee via GitGitGadget wrote:
>> diff --git a/builtin/config.c b/builtin/config.c
>> index 2c169fc126..2c93e1725b 100644
>> --- a/builtin/config.c
>> +++ b/builtin/config.c
>> @@ -274,6 +274,34 @@ static int format_config_bool(struct strbuf *buf,
>>   	return 0;
>>   }
>>   
>> +static int format_config_bool_or_int(struct strbuf *buf,
>> +				     const char *key_,
>> +				     const char *value_,
>> +				     const struct key_value_info *kvi,
>> +				     int gently)
>> +{
>> +	int v, is_bool = 0;
>> +
>> +	if (gently) {
>> +		v = git_parse_maybe_bool_text(value_);
> 
> This function also returns `1` in case `!value`. Is this intended? I
> guess so due to our implicit bool thingy, and `git_config_bool_or_int()`
> seems to behave the same.

Do you mean in the case of a NULL value?

Based on the rules for iterating through config values, deep down in
get_value() the value parameter sent to the function is never NULL.
It may be an empty string, but never NULL.

Thanks,
-Stolee

Re: [PATCH v2 04/13] config: format int64s gently

[Derrick Stolee]

On 2/17/26 4:05 AM, Patrick Steinhardt wrote:
> On Fri, Feb 13, 2026 at 11:55:09PM +0000, Derrick Stolee via GitGitGadget wrote:
>> diff --git a/builtin/config.c b/builtin/config.c
>> index 4c4c791883..d259a91d53 100644
>> --- a/builtin/config.c
>> +++ b/builtin/config.c
>> @@ -237,6 +237,25 @@ struct strbuf_list {
>>   	int alloc;
>>   };
>>   
>> +static int format_config_int64(struct strbuf *buf,
>> +			       const char *key_,
>> +			       const char *value_,
> 
> Why do we have the trailing underscores here?

This is all to match the existing names from format_config(). This may help to
recognize moved lines by keeping the variable names the same. Definitely not
my preference to use this name format, but I thought it fitting to avoid a
rename of all variables.

Thanks,
-Stolee

Re: [PATCH v2 11/13] color: add color_parse_gently()

[Junio C Hamano]

Derrick Stolee <stolee@gmail.com> writes:

>> Do you mean that this error() call is not die(), the flag does not
>> fit the usual "gently" criteria?  In other words, should we make
>> this call die() if we call it "gently"?
>
> This is an interesting case where the existing color parsing logic is
> not following the typical pattern that uses die() on a failed parse.

I see.  I personally would view that an existing bug worth fixing,
but I ...

> If we want to change the behavior to die() later, then that could be
> considered, though I don't want to consider the ramifications right now.

... agree with you that it should be fixed outside the scope of this
topic.

> I think the easiest "local" fix is to use the 'quiet' way, though it adds
> some asymmetry in the config code in how it uses the 'gently' parameter.

Or, just add comments to the function that takes gently but does not
die() to warn those who would add new callers.  They can pass
gently=1 if they want to handle the errors themselves and keep it
that way.  If they want the function to die, well they have to wait
until the function is fixed to behave like everybody else.

[PATCH v3 00/13] Make 'git config list --type=' parse and filter types

[Derrick Stolee via GitGitGadget]

I started down this road based on feedback on my 'git config-batch' RFC [1].

[1]
https://lore.kernel.org/git/pull.2033.git.1770214803.gitgitgadget@gmail.com/

I had described my intention to use 'git config-batch' as a single process
to load multiple config values one-by-one. Brian mentioned that 'git config
list -z' would probably suffice, so I started experimenting in that
direction [2].

[2]
https://github.com/git-ecosystem/git-credential-manager/compare/main...derrickstolee:config-list

However, I ran into a problem: the most critical performance bottleneck is
related to path-formatted config values that are queried with 'git config
get --type=path -z'. It wasn't hard to update things to lazily load the full
list of config values by type [3], but I then noticed a big problem!

[3]
https://github.com/git-ecosystem/git-credential-manager/commit/d403c8e24ce6f37da920cce23842dd5a6cf6481d

Problem: 'git config list' doesn't respect --type=<X>!

This boils down to the fact that the iterator function show_all_config()
doesn't call format_config(), which includes the type-parsing code.

This wasn't super trivial to update:

 1. format_config() uses git_config_parse_*() methods, which die() on a bad
    parse.
 2. The path parsing code didn't have a gentle version.
 3. The two paths ('git config list' and 'git config --list') needed to
    standardize their display options to work with format_config().
 4. Finally, we need to filter out key-value pairs that don't match the
    given type.


Updates in v2
=============

Based on the positive feedback in round one, this is no longer an RFC.

 * format_config() now uses a 'gently' parameter instead of 'die_on_parse'
   (flipped).
 * format_config() is more carefully updated with helper methods and a
   global refactor.
 * New gentle parsing code is introduced right before the format_config()
   helper is created to use it.
 * I squashed the change that updates the display_opts initial state into
   the patch that uses format_config() for the 'list' command. The initial
   state change on its own leads to test failures, so I am making a slightly
   bigger patch to keep things passing tests at every change.
 * More tests for 'git config list --type=<X>' are added.
 * I rearranged things so the 'git config list --type' integration follows
   the format_config() update immediately. The tests at that time show what
   such a trivial implementation would do, including failing on bool parsing
   and having several error messages for color and expiry-date parsing. The
   tests modify as these issues are fixed with gentle parsers.
 * I have a prototype implementation of GCM using this option in [4] and it
   gets the performance improvements I was hoping for. It requires polish
   and a compatibility check that uses the Git version to guarantee that
   this --type behavior change is recognized.

[4] https://github.com/git-ecosystem/git-credential-manager/pull/2268


Updates in V3
=============

 * Expanded the commit message of patch 3 to include reasoning for the
   filter.
 * Expanded the commit message of patch 3 to specify that tests are added
   demonstrating problematic behavior before they are fixed in later
   patches.
 * New test cases are added for ':(optional)' macros and int parsing.
 * The uses of git_parse_int64() and git_parse_int() were incorrect. These
   are fixed after being revealed by new tests.
 * The git_parse_maybe_pathname() was removed after realizing it did not
   contribute to a behavior change. Path parsing was already gentle, it just
   needed assistance with ':(optional)' macros for nonexistent paths.
 * A distinction is made between parsing 'gently' (do not die()) and
   'quietly' (do not error()). Neither cause the command to fail or emit to
   stderr, but the distinction is made by what the original parsing behavior
   did.
 * The conversion of the format_config() logic into a switch() is made more
   complete by adding a BUG() statement for the default case.
 * The options for the config type are now converted into an enum to make
   such switch() statements easier to validate as being complete.

Thanks for any and all feedback, -Stolee

Derrick Stolee (13):
  config: move show_all_config()
  config: add 'gently' parameter to format_config()
  config: make 'git config list --type=<X>' work
  config: format int64s gently
  config: format bools gently
  config: format bools or ints gently
  config: format bools or strings in helper
  config: format paths gently
  config: format expiry dates quietly
  color: add color_parse_quietly()
  config: format colors quietly
  config: restructure format_config()
  config: use an enum for type

 Documentation/git-config.adoc |   3 +
 builtin/config.c              | 306 +++++++++++++++++++++++++---------
 color.c                       |  25 ++-
 color.h                       |   1 +
 t/t1300-config.sh             |  84 +++++++++-
 5 files changed, 331 insertions(+), 88 deletions(-)


base-commit: 67ad42147a7acc2af6074753ebd03d904476118f
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2044%2Fderrickstolee%2Fconfig-list-type-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2044/derrickstolee/config-list-type-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/2044

Range-diff vs v2:

  1:  bca83d8ca8 =  1:  bca83d8ca8 config: move show_all_config()
  2:  93c94a1b25 =  2:  93c94a1b25 config: add 'gently' parameter to format_config()
  3:  6d2a48a3b7 !  3:  2b2a325b55 config: make 'git config list --type=<X>' work
     @@ Commit message
          be shown and also add the default delimiters as they were unset in some
          cases.
      
     -    If there is an error in parsing, then the row is not output.
     +    Our intention is that if there is an error in parsing, then the row is
     +    not output. This is necessary to avoid the caller needing to build their
     +    own validator to understand the difference between valid, canonicalized
     +    types and other raw string values. The raw values will always be
     +    available to the user if they do not specify the --type=<X> option.
     +
     +    The current behavior is more complicated, including error messages on
     +    bad parsing or potentially complete failure of the command.  We add
     +    tests at this point that demonstrate the current behavior so we can
     +    witness the fix in future changes that parse these values quietly and
     +    gently.
      
          This is a change in behavior! We are starting to respect an option that
          was previously ignored, leading to potential user confusion. This is
     @@ t/t1300-config.sh: done
      +red = red
      +blue = Blue
      +date = Fri Jun 4 15:46:55 2010
     ++missing=:(optional)no-such-path
     ++exists=:(optional)expect
       EOF
       
       test_expect_success 'identical modern --type specifiers are allowed' '
     @@ t/t1300-config.sh: test_expect_success 'unset type specifiers may be reset to co
       	test_cmp_config 1048576 --type=bool --no-type --type=int section.big
       '
       
     ++test_expect_success 'list --type=int shows only canonicalizable int values' '
     ++	cat >expect <<-EOF &&
     ++	section.number=10
     ++	section.big=1048576
     ++	EOF
     ++
     ++	test_must_fail git config ${mode_prefix}list --type=int
     ++'
     ++
      +test_expect_success 'list --type=bool shows only canonicalizable bool values' '
      +	cat >expect <<-EOF &&
      +	section.foo=true
     @@ t/t1300-config.sh: test_expect_success 'unset type specifiers may be reset to co
      +	test_must_fail git config ${mode_prefix}list --type=bool
      +'
      +
     ++test_expect_success 'list --type=bool-or-int shows only canonicalizable values' '
     ++	cat >expect <<-EOF &&
     ++	section.foo=true
     ++	section.number=10
     ++	section.big=1048576
     ++	EOF
     ++
     ++	test_must_fail git config ${mode_prefix}list --type=bool-or-int
     ++'
     ++
      +test_expect_success 'list --type=path shows only canonicalizable path values' '
     ++	# TODO: handling of missing path is incorrect here.
      +	cat >expect <<-EOF &&
      +	section.foo=True
      +	section.number=10
     @@ t/t1300-config.sh: test_expect_success 'unset type specifiers may be reset to co
      +	section.red=red
      +	section.blue=Blue
      +	section.date=Fri Jun 4 15:46:55 2010
     ++	section.missing=section.exists=expect
      +	EOF
      +
      +	git config ${mode_prefix}list --type=path >actual 2>err &&
     @@ t/t1300-config.sh: test_expect_success 'unset type specifiers may be reset to co
      +	error: '\''~/dir'\'' for '\''section.path'\'' is not a valid timestamp
      +	error: '\''red'\'' for '\''section.red'\'' is not a valid timestamp
      +	error: '\''Blue'\'' for '\''section.blue'\'' is not a valid timestamp
     ++	error: '\'':(optional)no-such-path'\'' for '\''section.missing'\'' is not a valid timestamp
     ++	error: '\'':(optional)expect'\'' for '\''section.exists'\'' is not a valid timestamp
      +	EOF
      +
      +	git config ${mode_prefix}list --type=expiry-date >actual 2>err &&
     @@ t/t1300-config.sh: test_expect_success 'unset type specifiers may be reset to co
      +	error: invalid color value: 1M
      +	error: invalid color value: ~/dir
      +	error: invalid color value: Fri Jun 4 15:46:55 2010
     ++	error: invalid color value: :(optional)no-such-path
     ++	error: invalid color value: :(optional)expect
      +	EOF
      +
      +	git config ${mode_prefix}list --type=color >actual.raw 2>err &&
  4:  2bca4d2316 !  4:  4835ee5180 config: format int64s gently
     @@ builtin/config.c: struct strbuf_list {
      +{
      +	int64_t v = 0;
      +	if (gently) {
     -+		if (git_parse_int64(value_, &v))
     ++		if (!git_parse_int64(value_, &v))
      +			return -1;
      +	} else {
      +		/* may die() */
     @@ builtin/config.c: static int format_config(const struct config_display_options *
       }
       
       static int show_all_config(const char *key_, const char *value_,
     +
     + ## t/t1300-config.sh ##
     +@@ t/t1300-config.sh: test_expect_success 'list --type=int shows only canonicalizable int values' '
     + 	section.big=1048576
     + 	EOF
     + 
     +-	test_must_fail git config ${mode_prefix}list --type=int
     ++	git config ${mode_prefix}list --type=int >actual 2>err &&
     ++	test_cmp expect actual &&
     ++	test_must_be_empty err
     + '
     + 
     + test_expect_success 'list --type=bool shows only canonicalizable bool values' '
  5:  f8e0b8304f !  5:  24eb757a40 config: format bools gently
     @@ t/t1300-config.sh: test_expect_success 'list --type=bool shows only canonicaliza
      +	test_must_be_empty err
       '
       
     - test_expect_success 'list --type=path shows only canonicalizable path values' '
     + test_expect_success 'list --type=bool-or-int shows only canonicalizable values' '
  6:  0a428d2ffe !  6:  02849ca621 config: format bools or ints gently
     @@ builtin/config.c: static int format_config_bool(struct strbuf *buf,
      +
      +		if (v >= 0)
      +			is_bool = 1;
     -+		else if (git_parse_int(value_, &v))
     ++		else if (!git_parse_int(value_, &v))
      +			return -1;
      +	} else {
      +		v = git_config_bool_or_int(key_, value_, kvi,
     @@ builtin/config.c: static int format_config(const struct config_display_options *
       			int v = git_parse_maybe_bool(value_);
       			if (v < 0)
       				strbuf_addstr(buf, value_);
     +
     + ## t/t1300-config.sh ##
     +@@ t/t1300-config.sh: test_expect_success 'list --type=bool-or-int shows only canonicalizable values'
     + 	section.big=1048576
     + 	EOF
     + 
     +-	test_must_fail git config ${mode_prefix}list --type=bool-or-int
     ++	git config ${mode_prefix}list --type=bool-or-int >actual 2>err &&
     ++	test_cmp expect actual &&
     ++	test_must_be_empty err
     + '
     + 
     + test_expect_success 'list --type=path shows only canonicalizable path values' '
  7:  3fec3abbd6 =  7:  9f06db29b9 config: format bools or strings in helper
  8:  fafafc5465 <  -:  ---------- parse: add git_parse_maybe_pathname()
  9:  d1cfa0c5e1 !  8:  d198c238e9 config: format paths gently
     @@ Commit message
          Move the logic for formatting path config values into a helper method
          and use gentle parsing when needed.
      
     +    We need to be careful about how to handle the ':(optional)' macro, which
     +    as tested in t1311-config-optional.sh must allow for ignoring a missing
     +    path when other multiple values exist, but cause 'git config get' to
     +    fail if it is the only possible value and thus no result is output.
     +
     +    In the case of our list, we need to omit those values silently. This
     +    necessitates the use of the 'gently' parameter here.
     +
          Signed-off-by: Derrick Stolee <stolee@gmail.com>
      
       ## builtin/config.c ##
     @@ builtin/config.c: static int format_config_bool_or_str(struct strbuf *buf,
      +			      int gently)
      +{
      +	char *v;
     -+	if (gently) {
     -+		if (git_parse_maybe_pathname(value_, &v) < 0)
     -+			return -1;
     -+	} else if (git_config_pathname(&v, key_, value_) < 0) {
     ++
     ++	if (git_config_pathname(&v, key_, value_) < 0)
      +		return -1;
     -+	}
      +
      +	if (v)
      +		strbuf_addstr(buf, v);
      +	else
     -+		return 1; /* :(optional)no-such-file */
     ++		return gently ? -1 : 1; /* :(optional)no-such-file */
      +
      +	free(v);
      +	return 0;
     @@ builtin/config.c: static int format_config(const struct config_display_options *
       			timestamp_t t;
       			if (git_config_expiry_date(&t, key_, value_) < 0)
       				return -1;
     +
     + ## t/t1300-config.sh ##
     +@@ t/t1300-config.sh: test_expect_success 'list --type=bool-or-int shows only canonicalizable values'
     + '
     + 
     + test_expect_success 'list --type=path shows only canonicalizable path values' '
     +-	# TODO: handling of missing path is incorrect here.
     + 	cat >expect <<-EOF &&
     + 	section.foo=True
     + 	section.number=10
     +@@ t/t1300-config.sh: test_expect_success 'list --type=path shows only canonicalizable path values' '
     + 	section.red=red
     + 	section.blue=Blue
     + 	section.date=Fri Jun 4 15:46:55 2010
     +-	section.missing=section.exists=expect
     ++	section.exists=expect
     + 	EOF
     + 
     + 	git config ${mode_prefix}list --type=path >actual 2>err &&
 10:  9221ca2352 !  9:  7568a0bc34 config: format expiry dates gently
     @@ Metadata
      Author: Derrick Stolee <stolee@gmail.com>
      
       ## Commit message ##
     -    config: format expiry dates gently
     +    config: format expiry dates quietly
      
          Move the logic for formatting expiry date config values into a helper
     -    method and use gentle parsing when needed.
     +    method and use quiet parsing when needed.
     +
     +    Note that git_config_expiry_date() will show an error on a bad parse and
     +    not die() like most other git_config...() parsers. Thus, we use
     +    'quietly' here instead of 'gently'.
      
          There is an unfortunate asymmetry in these two parsing methods, but we
          need to treat a positive response from parse_expiry_date() as an error
     @@ builtin/config.c: static int format_config_path(struct strbuf *buf,
      +static int format_config_expiry_date(struct strbuf *buf,
      +				     const char *key_,
      +				     const char *value_,
     -+				     int gently)
     ++				     int quietly)
      +{
      +	timestamp_t t;
     -+	if (gently) {
     ++	if (quietly) {
      +		if (parse_expiry_date(value_, &t))
      +			return -1;
      +	} else if (git_config_expiry_date(&t, key_, value_) < 0) {
     @@ t/t1300-config.sh: test_expect_success 'list --type=path shows only canonicaliza
      -	error: '\''~/dir'\'' for '\''section.path'\'' is not a valid timestamp
      -	error: '\''red'\'' for '\''section.red'\'' is not a valid timestamp
      -	error: '\''Blue'\'' for '\''section.blue'\'' is not a valid timestamp
     +-	error: '\'':(optional)no-such-path'\'' for '\''section.missing'\'' is not a valid timestamp
     +-	error: '\'':(optional)expect'\'' for '\''section.exists'\'' is not a valid timestamp
      -	EOF
      -
       	git config ${mode_prefix}list --type=expiry-date >actual 2>err &&
 11:  ddf6131ac9 ! 10:  d98966f53a color: add color_parse_gently()
     @@ Metadata
      Author: Derrick Stolee <stolee@gmail.com>
      
       ## Commit message ##
     -    color: add color_parse_gently()
     +    color: add color_parse_quietly()
      
          When parsing colors, a failed parse leads to an error message due to the
          result returning error(). To allow for quiet parsing, create
     -    color_parse_gently().
     +    color_parse_quietly(). This is in contrast to an ..._gently() version
     +    because the original does not die(), so both options are technically
     +    'gentle'.
      
          To accomplish this, convert the implementation of color_parse_mem() into
     -    a static color_parse_mem_1() helper that adds a 'gently' parameter. The
     -    color_parse_gently() method can then use this. Since it is a near
     +    a static color_parse_mem_1() helper that adds a 'quiet' parameter. The
     +    color_parse_quietly() method can then use this. Since it is a near
          equivalent to color_parse(), move that method down in the file so they
          can be nearby while also appearing after color_parse_mem_1().
      
     @@ color.c: static int color_empty(const struct color *c)
       
      -int color_parse_mem(const char *value, int value_len, char *dst)
      +static int color_parse_mem_1(const char *value, int value_len,
     -+			     char *dst, int gently)
     ++			     char *dst, int quiet)
       {
       	const char *ptr = value;
       	int len = value_len;
     @@ color.c: int color_parse_mem(const char *value, int value_len, char *dst)
       	return 0;
       bad:
      -	return error(_("invalid color value: %.*s"), value_len, value);
     -+	return gently ? -1 : error(_("invalid color value: %.*s"), value_len, value);
     ++	return quiet ? -1 : error(_("invalid color value: %.*s"), value_len, value);
       #undef OUT
       }
       
     @@ color.c: int color_parse_mem(const char *value, int value_len, char *dst)
      +	return color_parse_mem(value, strlen(value), dst);
      +}
      +
     -+int color_parse_gently(const char *value, char *dst)
     ++int color_parse_quietly(const char *value, char *dst)
      +{
      +	return color_parse_mem_1(value, strlen(value), dst, 1);
      +}
     @@ color.h: bool want_color_fd(int fd, enum git_colorbool var);
        * terminal.
        */
       int color_parse(const char *value, char *dst);
     -+int color_parse_gently(const char *value, char *dst);
     ++int color_parse_quietly(const char *value, char *dst);
       int color_parse_mem(const char *value, int len, char *dst);
       
       /*
 12:  d14937e6d1 ! 11:  078065cff0 config: format colors gently
     @@ Metadata
      Author: Derrick Stolee <stolee@gmail.com>
      
       ## Commit message ##
     -    config: format colors gently
     +    config: format colors quietly
      
          Move the logic for formatting color config value into a helper method
     -    and use gentle parsing when needed.
     +    and use quiet parsing when needed.
      
          This removes error messages when parsing a list of config values that do
          not match color formats.
     @@ builtin/config.c: static int format_config_expiry_date(struct strbuf *buf,
      +	char v[COLOR_MAXLEN];
      +
      +	if (gently) {
     -+		if (color_parse_gently(value_, v) < 0)
     ++		if (color_parse_quietly(value_, v) < 0)
      +			return -1;
      +	} else if (git_config_color(v, key_, value_) < 0) {
      +		return -1;
     @@ t/t1300-config.sh: test_expect_success 'list --type=color shows only canonicaliz
      -	error: invalid color value: 1M
      -	error: invalid color value: ~/dir
      -	error: invalid color value: Fri Jun 4 15:46:55 2010
     +-	error: invalid color value: :(optional)no-such-path
     +-	error: invalid color value: :(optional)expect
      -	EOF
      -
       	git config ${mode_prefix}list --type=color >actual.raw 2>err &&
 13:  48fc882785 ! 12:  76fc7670fc config: restructure format_config()
     @@ Commit message
          Signed-off-by: Derrick Stolee <stolee@gmail.com>
      
       ## builtin/config.c ##
     +@@ builtin/config.c: struct config_display_options {
     + 	.key_delim = ' ', \
     + }
     + 
     ++#define TYPE_NONE		0
     + #define TYPE_BOOL		1
     + #define TYPE_INT		2
     + #define TYPE_BOOL_OR_INT	3
      @@ builtin/config.c: static int format_config(const struct config_display_options *opts,
       		show_config_origin(opts, kvi, buf);
       	if (opts->show_keys)
     @@ builtin/config.c: static int format_config(const struct config_display_options *
      +		res = format_config_color(buf, key_, value_, gently);
      +		break;
      +
     -+	default:
     ++	case TYPE_NONE:
      +		if (value_) {
       			strbuf_addstr(buf, value_);
       		} else {
       			/* Just show the key name; back out delimiter */
     -@@ builtin/config.c: static int format_config(const struct config_display_options *opts,
     + 			if (opts->show_keys)
       				strbuf_setlen(buf, buf->len - 1);
       		}
     ++		break;
     ++
     ++	default:
     ++		BUG("undefined type %d", opts->type);
       	}
      +
      +terminator:
  -:  ---------- > 13:  3c32c037bc config: use an enum for type

-- 
gitgitgadget

[PATCH v3 01/13] config: move show_all_config()

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

In anticipation of using format_config() in this method, move
show_all_config() lower in the file without changes.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 builtin/config.c | 48 ++++++++++++++++++++++++------------------------
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 288ebdfdaa..237f7a934d 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -231,30 +231,6 @@ static void show_config_scope(const struct config_display_options *opts,
 	strbuf_addch(buf, term);
 }
 
-static int show_all_config(const char *key_, const char *value_,
-			   const struct config_context *ctx,
-			   void *cb)
-{
-	const struct config_display_options *opts = cb;
-	const struct key_value_info *kvi = ctx->kvi;
-
-	if (opts->show_origin || opts->show_scope) {
-		struct strbuf buf = STRBUF_INIT;
-		if (opts->show_scope)
-			show_config_scope(opts, kvi, &buf);
-		if (opts->show_origin)
-			show_config_origin(opts, kvi, &buf);
-		/* Use fwrite as "buf" can contain \0's if "end_null" is set. */
-		fwrite(buf.buf, 1, buf.len, stdout);
-		strbuf_release(&buf);
-	}
-	if (!opts->omit_values && value_)
-		printf("%s%c%s%c", key_, opts->delim, value_, opts->term);
-	else
-		printf("%s%c", key_, opts->term);
-	return 0;
-}
-
 struct strbuf_list {
 	struct strbuf *items;
 	int nr;
@@ -332,6 +308,30 @@ static int format_config(const struct config_display_options *opts,
 	return 0;
 }
 
+static int show_all_config(const char *key_, const char *value_,
+			   const struct config_context *ctx,
+			   void *cb)
+{
+	const struct config_display_options *opts = cb;
+	const struct key_value_info *kvi = ctx->kvi;
+
+	if (opts->show_origin || opts->show_scope) {
+		struct strbuf buf = STRBUF_INIT;
+		if (opts->show_scope)
+			show_config_scope(opts, kvi, &buf);
+		if (opts->show_origin)
+			show_config_origin(opts, kvi, &buf);
+		/* Use fwrite as "buf" can contain \0's if "end_null" is set. */
+		fwrite(buf.buf, 1, buf.len, stdout);
+		strbuf_release(&buf);
+	}
+	if (!opts->omit_values && value_)
+		printf("%s%c%s%c", key_, opts->delim, value_, opts->term);
+	else
+		printf("%s%c", key_, opts->term);
+	return 0;
+}
+
 #define GET_VALUE_ALL        (1 << 0)
 #define GET_VALUE_KEY_REGEXP (1 << 1)
 
-- 
gitgitgadget

[PATCH v3 02/13] config: add 'gently' parameter to format_config()

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

This parameter is set to 0 for all current callers and is UNUSED.
However, we will start using this option in future changes and in a
critical change that requires gentle parsing (not using die()) to try
parsing all values in a list.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 builtin/config.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 237f7a934d..b4c4228311 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -242,10 +242,14 @@ struct strbuf_list {
  * append it into strbuf `buf`.  Returns a negative value on failure,
  * 0 on success, 1 on a missing optional value (i.e., telling the
  * caller to pretend that <key_,value_> did not exist).
+ *
+ * Note: 'gently' is currently ignored, but will be implemented in
+ * a future change.
  */
 static int format_config(const struct config_display_options *opts,
 			 struct strbuf *buf, const char *key_,
-			 const char *value_, const struct key_value_info *kvi)
+			 const char *value_, const struct key_value_info *kvi,
+			 int gently UNUSED)
 {
 	if (opts->show_scope)
 		show_config_scope(opts, kvi, buf);
@@ -372,7 +376,7 @@ static int collect_config(const char *key_, const char *value_,
 	strbuf_init(&values->items[values->nr], 0);
 
 	status = format_config(data->display_opts, &values->items[values->nr++],
-			       key_, value_, kvi);
+			       key_, value_, kvi, 0);
 	if (status < 0)
 		return status;
 	if (status) {
@@ -463,7 +467,7 @@ static int get_value(const struct config_location_options *opts,
 		strbuf_init(item, 0);
 
 		status = format_config(display_opts, item, key_,
-				       display_opts->default_value, &kvi);
+				       display_opts->default_value, &kvi, 0);
 		if (status < 0)
 			die(_("failed to format default config value: %s"),
 			    display_opts->default_value);
@@ -743,7 +747,7 @@ static int get_urlmatch(const struct config_location_options *opts,
 
 		status = format_config(&display_opts, &buf, item->string,
 				       matched->value_is_null ? NULL : matched->value.buf,
-				       &matched->kvi);
+				       &matched->kvi, 0);
 		if (!status)
 			fwrite(buf.buf, 1, buf.len, stdout);
 		strbuf_release(&buf);
-- 
gitgitgadget

[PATCH v3 03/13] config: make 'git config list --type=<X>' work

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

Previously, the --type=<X> argument to 'git config list' was ignored and
did nothing. Now, we add the use of format_config() to the
show_all_config() function so each key-value pair is attempted to be
parsed. This is our first use of the 'gently' parameter with a nonzero
value.

When listing multiple values, our initial settings for the output format
is different. Add a new init helper to specify the fact that keys should
be shown and also add the default delimiters as they were unset in some
cases.

Our intention is that if there is an error in parsing, then the row is
not output. This is necessary to avoid the caller needing to build their
own validator to understand the difference between valid, canonicalized
types and other raw string values. The raw values will always be
available to the user if they do not specify the --type=<X> option.

The current behavior is more complicated, including error messages on
bad parsing or potentially complete failure of the command.  We add
tests at this point that demonstrate the current behavior so we can
witness the fix in future changes that parse these values quietly and
gently.

This is a change in behavior! We are starting to respect an option that
was previously ignored, leading to potential user confusion. This is
probably still a good option, since the --type argument did not change
behavior at all previously, so users can get the behavior they expect by
removing the --type argument or adding the --no-type argument.

t1300-config.sh is updated with the current behavior of this formatting
logic to justify the upcoming refactoring of format_config() that will
incrementally fix some of these cases to be more user-friendly.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 Documentation/git-config.adoc |  3 ++
 builtin/config.c              | 35 +++++++------
 t/t1300-config.sh             | 97 ++++++++++++++++++++++++++++++++++-
 3 files changed, 119 insertions(+), 16 deletions(-)

diff --git a/Documentation/git-config.adoc b/Documentation/git-config.adoc
index ac3b536a15..5300dd4c51 100644
--- a/Documentation/git-config.adoc
+++ b/Documentation/git-config.adoc
@@ -240,6 +240,9 @@ Valid `<type>`'s include:
   that the given value is canonicalize-able as an ANSI color, but it is written
   as-is.
 +
+If the command is in `list` mode, then the `--type <type>` argument will apply
+to each listed config value. If the value does not successfully parse in that
+format, then it will be omitted from the list.
 
 --bool::
 --int::
diff --git a/builtin/config.c b/builtin/config.c
index b4c4228311..4c4c791883 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -318,21 +318,12 @@ static int show_all_config(const char *key_, const char *value_,
 {
 	const struct config_display_options *opts = cb;
 	const struct key_value_info *kvi = ctx->kvi;
+	struct strbuf formatted = STRBUF_INIT;
 
-	if (opts->show_origin || opts->show_scope) {
-		struct strbuf buf = STRBUF_INIT;
-		if (opts->show_scope)
-			show_config_scope(opts, kvi, &buf);
-		if (opts->show_origin)
-			show_config_origin(opts, kvi, &buf);
-		/* Use fwrite as "buf" can contain \0's if "end_null" is set. */
-		fwrite(buf.buf, 1, buf.len, stdout);
-		strbuf_release(&buf);
-	}
-	if (!opts->omit_values && value_)
-		printf("%s%c%s%c", key_, opts->delim, value_, opts->term);
-	else
-		printf("%s%c", key_, opts->term);
+	if (format_config(opts, &formatted, key_, value_, kvi, 1) >= 0)
+		fwrite(formatted.buf, 1, formatted.len, stdout);
+
+	strbuf_release(&formatted);
 	return 0;
 }
 
@@ -872,6 +863,19 @@ static void display_options_init(struct config_display_options *opts)
 	}
 }
 
+static void display_options_init_list(struct config_display_options *opts)
+{
+	opts->show_keys = 1;
+
+	if (opts->end_nul) {
+		display_options_init(opts);
+	} else {
+		opts->term = '\n';
+		opts->delim = ' ';
+		opts->key_delim = '=';
+	}
+}
+
 static int cmd_config_list(int argc, const char **argv, const char *prefix,
 			   struct repository *repo UNUSED)
 {
@@ -890,7 +894,7 @@ static int cmd_config_list(int argc, const char **argv, const char *prefix,
 	check_argc(argc, 0, 0);
 
 	location_options_init(&location_opts, prefix);
-	display_options_init(&display_opts);
+	display_options_init_list(&display_opts);
 
 	setup_auto_pager("config", 1);
 
@@ -1321,6 +1325,7 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
 
 	if (actions == ACTION_LIST) {
 		check_argc(argc, 0, 0);
+		display_options_init_list(&display_opts);
 		if (config_with_options(show_all_config, &display_opts,
 					&location_opts.source, the_repository,
 					&location_opts.options) < 0) {
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 9850fcd5b5..dc744c0bae 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -2459,9 +2459,15 @@ done
 
 cat >.git/config <<-\EOF &&
 [section]
-foo = true
+foo = True
 number = 10
 big = 1M
+path = ~/dir
+red = red
+blue = Blue
+date = Fri Jun 4 15:46:55 2010
+missing=:(optional)no-such-path
+exists=:(optional)expect
 EOF
 
 test_expect_success 'identical modern --type specifiers are allowed' '
@@ -2503,6 +2509,95 @@ test_expect_success 'unset type specifiers may be reset to conflicting ones' '
 	test_cmp_config 1048576 --type=bool --no-type --type=int section.big
 '
 
+test_expect_success 'list --type=int shows only canonicalizable int values' '
+	cat >expect <<-EOF &&
+	section.number=10
+	section.big=1048576
+	EOF
+
+	test_must_fail git config ${mode_prefix}list --type=int
+'
+
+test_expect_success 'list --type=bool shows only canonicalizable bool values' '
+	cat >expect <<-EOF &&
+	section.foo=true
+	section.number=true
+	section.big=true
+	EOF
+
+	test_must_fail git config ${mode_prefix}list --type=bool
+'
+
+test_expect_success 'list --type=bool-or-int shows only canonicalizable values' '
+	cat >expect <<-EOF &&
+	section.foo=true
+	section.number=10
+	section.big=1048576
+	EOF
+
+	test_must_fail git config ${mode_prefix}list --type=bool-or-int
+'
+
+test_expect_success 'list --type=path shows only canonicalizable path values' '
+	# TODO: handling of missing path is incorrect here.
+	cat >expect <<-EOF &&
+	section.foo=True
+	section.number=10
+	section.big=1M
+	section.path=$HOME/dir
+	section.red=red
+	section.blue=Blue
+	section.date=Fri Jun 4 15:46:55 2010
+	section.missing=section.exists=expect
+	EOF
+
+	git config ${mode_prefix}list --type=path >actual 2>err &&
+	test_cmp expect actual &&
+	test_must_be_empty err
+'
+
+test_expect_success 'list --type=expiry-date shows only canonicalizable dates' '
+	cat >expecterr <<-EOF &&
+	error: '\''True'\'' for '\''section.foo'\'' is not a valid timestamp
+	error: '\''~/dir'\'' for '\''section.path'\'' is not a valid timestamp
+	error: '\''red'\'' for '\''section.red'\'' is not a valid timestamp
+	error: '\''Blue'\'' for '\''section.blue'\'' is not a valid timestamp
+	error: '\'':(optional)no-such-path'\'' for '\''section.missing'\'' is not a valid timestamp
+	error: '\'':(optional)expect'\'' for '\''section.exists'\'' is not a valid timestamp
+	EOF
+
+	git config ${mode_prefix}list --type=expiry-date >actual 2>err &&
+
+	# section.number and section.big parse as relative dates that could
+	# have clock skew in their results.
+	test_grep section.big actual &&
+	test_grep section.number actual &&
+	test_grep "section.date=$(git config --type=expiry-date section.$key)" actual &&
+	test_cmp expecterr err
+'
+
+test_expect_success 'list --type=color shows only canonicalizable color values' '
+	cat >expect <<-EOF &&
+	section.number=<>
+	section.red=<RED>
+	section.blue=<BLUE>
+	EOF
+
+	cat >expecterr <<-EOF &&
+	error: invalid color value: True
+	error: invalid color value: 1M
+	error: invalid color value: ~/dir
+	error: invalid color value: Fri Jun 4 15:46:55 2010
+	error: invalid color value: :(optional)no-such-path
+	error: invalid color value: :(optional)expect
+	EOF
+
+	git config ${mode_prefix}list --type=color >actual.raw 2>err &&
+	test_decode_color <actual.raw >actual &&
+	test_cmp expect actual &&
+	test_cmp expecterr err
+'
+
 test_expect_success '--type rejects unknown specifiers' '
 	test_must_fail git config --type=nonsense section.foo 2>error &&
 	test_grep "unrecognized --type argument" error
-- 
gitgitgadget

[PATCH v3 04/13] config: format int64s gently

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

Move the logic for formatting int64 config values into a helper method
and use gentle parsing when needed.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 builtin/config.c  | 27 +++++++++++++++++++++++----
 t/t1300-config.sh |  4 +++-
 2 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 4c4c791883..448b148563 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -237,6 +237,25 @@ struct strbuf_list {
 	int alloc;
 };
 
+static int format_config_int64(struct strbuf *buf,
+			       const char *key_,
+			       const char *value_,
+			       const struct key_value_info *kvi,
+			       int gently)
+{
+	int64_t v = 0;
+	if (gently) {
+		if (!git_parse_int64(value_, &v))
+			return -1;
+	} else {
+		/* may die() */
+		v = git_config_int64(key_, value_ ? value_ : "", kvi);
+	}
+
+	strbuf_addf(buf, "%"PRId64, v);
+	return 0;
+}
+
 /*
  * Format the configuration key-value pair (`key_`, `value_`) and
  * append it into strbuf `buf`.  Returns a negative value on failure,
@@ -249,8 +268,9 @@ struct strbuf_list {
 static int format_config(const struct config_display_options *opts,
 			 struct strbuf *buf, const char *key_,
 			 const char *value_, const struct key_value_info *kvi,
-			 int gently UNUSED)
+			 int gently)
 {
+	int res = 0;
 	if (opts->show_scope)
 		show_config_scope(opts, kvi, buf);
 	if (opts->show_origin)
@@ -262,8 +282,7 @@ static int format_config(const struct config_display_options *opts,
 			strbuf_addch(buf, opts->key_delim);
 
 		if (opts->type == TYPE_INT)
-			strbuf_addf(buf, "%"PRId64,
-				    git_config_int64(key_, value_ ? value_ : "", kvi));
+			res = format_config_int64(buf, key_, value_, kvi, gently);
 		else if (opts->type == TYPE_BOOL)
 			strbuf_addstr(buf, git_config_bool(key_, value_) ?
 				      "true" : "false");
@@ -309,7 +328,7 @@ static int format_config(const struct config_display_options *opts,
 		}
 	}
 	strbuf_addch(buf, opts->term);
-	return 0;
+	return res;
 }
 
 static int show_all_config(const char *key_, const char *value_,
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index dc744c0bae..05a812fd6d 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -2515,7 +2515,9 @@ test_expect_success 'list --type=int shows only canonicalizable int values' '
 	section.big=1048576
 	EOF
 
-	test_must_fail git config ${mode_prefix}list --type=int
+	git config ${mode_prefix}list --type=int >actual 2>err &&
+	test_cmp expect actual &&
+	test_must_be_empty err
 '
 
 test_expect_success 'list --type=bool shows only canonicalizable bool values' '
-- 
gitgitgadget

[PATCH v3 05/13] config: format bools gently

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

Move the logic for formatting bool config values into a helper method
and use gentle parsing when needed.

This makes 'git config list --type=bool' not fail when coming across a
non-boolean value. Such unparseable values are filtered out quietly.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 builtin/config.c  | 21 +++++++++++++++++++--
 t/t1300-config.sh |  4 +++-
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 448b148563..d8b38c51d3 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -256,6 +256,24 @@ static int format_config_int64(struct strbuf *buf,
 	return 0;
 }
 
+static int format_config_bool(struct strbuf *buf,
+			      const char *key_,
+			      const char *value_,
+			      int gently)
+{
+	int v = 0;
+	if (gently) {
+		if ((v = git_parse_maybe_bool(value_)) < 0)
+			return -1;
+	} else {
+		/* may die() */
+		v = git_config_bool(key_, value_);
+	}
+
+	strbuf_addstr(buf, v ? "true" : "false");
+	return 0;
+}
+
 /*
  * Format the configuration key-value pair (`key_`, `value_`) and
  * append it into strbuf `buf`.  Returns a negative value on failure,
@@ -284,8 +302,7 @@ static int format_config(const struct config_display_options *opts,
 		if (opts->type == TYPE_INT)
 			res = format_config_int64(buf, key_, value_, kvi, gently);
 		else if (opts->type == TYPE_BOOL)
-			strbuf_addstr(buf, git_config_bool(key_, value_) ?
-				      "true" : "false");
+			res = format_config_bool(buf, key_, value_, gently);
 		else if (opts->type == TYPE_BOOL_OR_INT) {
 			int is_bool, v;
 			v = git_config_bool_or_int(key_, value_, kvi,
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 05a812fd6d..568cfaa3c5 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -2527,7 +2527,9 @@ test_expect_success 'list --type=bool shows only canonicalizable bool values' '
 	section.big=true
 	EOF
 
-	test_must_fail git config ${mode_prefix}list --type=bool
+	git config ${mode_prefix}list --type=bool >actual 2>err &&
+	test_cmp expect actual &&
+	test_must_be_empty err
 '
 
 test_expect_success 'list --type=bool-or-int shows only canonicalizable values' '
-- 
gitgitgadget

[PATCH v3 06/13] config: format bools or ints gently

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

Move the logic for formatting bool-or-int config values into a helper
method and use gentle parsing when needed.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 builtin/config.c  | 40 +++++++++++++++++++++++++++++++---------
 t/t1300-config.sh |  4 +++-
 2 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index d8b38c51d3..491a880e56 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -274,6 +274,34 @@ static int format_config_bool(struct strbuf *buf,
 	return 0;
 }
 
+static int format_config_bool_or_int(struct strbuf *buf,
+				     const char *key_,
+				     const char *value_,
+				     const struct key_value_info *kvi,
+				     int gently)
+{
+	int v, is_bool = 0;
+
+	if (gently) {
+		v = git_parse_maybe_bool_text(value_);
+
+		if (v >= 0)
+			is_bool = 1;
+		else if (!git_parse_int(value_, &v))
+			return -1;
+	} else {
+		v = git_config_bool_or_int(key_, value_, kvi,
+					   &is_bool);
+	}
+
+	if (is_bool)
+		strbuf_addstr(buf, v ? "true" : "false");
+	else
+		strbuf_addf(buf, "%d", v);
+
+	return 0;
+}
+
 /*
  * Format the configuration key-value pair (`key_`, `value_`) and
  * append it into strbuf `buf`.  Returns a negative value on failure,
@@ -303,15 +331,9 @@ static int format_config(const struct config_display_options *opts,
 			res = format_config_int64(buf, key_, value_, kvi, gently);
 		else if (opts->type == TYPE_BOOL)
 			res = format_config_bool(buf, key_, value_, gently);
-		else if (opts->type == TYPE_BOOL_OR_INT) {
-			int is_bool, v;
-			v = git_config_bool_or_int(key_, value_, kvi,
-						   &is_bool);
-			if (is_bool)
-				strbuf_addstr(buf, v ? "true" : "false");
-			else
-				strbuf_addf(buf, "%d", v);
-		} else if (opts->type == TYPE_BOOL_OR_STR) {
+		else if (opts->type == TYPE_BOOL_OR_INT)
+			res = format_config_bool_or_int(buf, key_, value_, kvi, gently);
+		else if (opts->type == TYPE_BOOL_OR_STR) {
 			int v = git_parse_maybe_bool(value_);
 			if (v < 0)
 				strbuf_addstr(buf, value_);
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 568cfaa3c5..1fc8e788ee 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -2539,7 +2539,9 @@ test_expect_success 'list --type=bool-or-int shows only canonicalizable values'
 	section.big=1048576
 	EOF
 
-	test_must_fail git config ${mode_prefix}list --type=bool-or-int
+	git config ${mode_prefix}list --type=bool-or-int >actual 2>err &&
+	test_cmp expect actual &&
+	test_must_be_empty err
 '
 
 test_expect_success 'list --type=path shows only canonicalizable path values' '
-- 
gitgitgadget

[PATCH v3 07/13] config: format bools or strings in helper

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

Move the logic for formatting bool-or-string config values into a
helper. This parsing has always been gentle, so this is not unlocking
new behavior. This extraction is only to match the formatting of the
other cases that do need a behavior change.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 builtin/config.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 491a880e56..79c139c5b0 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -302,6 +302,18 @@ static int format_config_bool_or_int(struct strbuf *buf,
 	return 0;
 }
 
+/* This mode is always gentle. */
+static int format_config_bool_or_str(struct strbuf *buf,
+				     const char *value_)
+{
+	int v = git_parse_maybe_bool(value_);
+	if (v < 0)
+		strbuf_addstr(buf, value_);
+	else
+		strbuf_addstr(buf, v ? "true" : "false");
+	return 0;
+}
+
 /*
  * Format the configuration key-value pair (`key_`, `value_`) and
  * append it into strbuf `buf`.  Returns a negative value on failure,
@@ -333,13 +345,9 @@ static int format_config(const struct config_display_options *opts,
 			res = format_config_bool(buf, key_, value_, gently);
 		else if (opts->type == TYPE_BOOL_OR_INT)
 			res = format_config_bool_or_int(buf, key_, value_, kvi, gently);
-		else if (opts->type == TYPE_BOOL_OR_STR) {
-			int v = git_parse_maybe_bool(value_);
-			if (v < 0)
-				strbuf_addstr(buf, value_);
-			else
-				strbuf_addstr(buf, v ? "true" : "false");
-		} else if (opts->type == TYPE_PATH) {
+		else if (opts->type == TYPE_BOOL_OR_STR)
+			res = format_config_bool_or_str(buf, value_);
+		else if (opts->type == TYPE_PATH) {
 			char *v;
 			if (git_config_pathname(&v, key_, value_) < 0)
 				return -1;
-- 
gitgitgadget

[PATCH v3 08/13] config: format paths gently

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

Move the logic for formatting path config values into a helper method
and use gentle parsing when needed.

We need to be careful about how to handle the ':(optional)' macro, which
as tested in t1311-config-optional.sh must allow for ignoring a missing
path when other multiple values exist, but cause 'git config get' to
fail if it is the only possible value and thus no result is output.

In the case of our list, we need to omit those values silently. This
necessitates the use of the 'gently' parameter here.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 builtin/config.c  | 32 ++++++++++++++++++++++----------
 t/t1300-config.sh |  3 +--
 2 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 79c139c5b0..2828b6dcf1 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -314,6 +314,25 @@ static int format_config_bool_or_str(struct strbuf *buf,
 	return 0;
 }
 
+static int format_config_path(struct strbuf *buf,
+			      const char *key_,
+			      const char *value_,
+			      int gently)
+{
+	char *v;
+
+	if (git_config_pathname(&v, key_, value_) < 0)
+		return -1;
+
+	if (v)
+		strbuf_addstr(buf, v);
+	else
+		return gently ? -1 : 1; /* :(optional)no-such-file */
+
+	free(v);
+	return 0;
+}
+
 /*
  * Format the configuration key-value pair (`key_`, `value_`) and
  * append it into strbuf `buf`.  Returns a negative value on failure,
@@ -347,16 +366,9 @@ static int format_config(const struct config_display_options *opts,
 			res = format_config_bool_or_int(buf, key_, value_, kvi, gently);
 		else if (opts->type == TYPE_BOOL_OR_STR)
 			res = format_config_bool_or_str(buf, value_);
-		else if (opts->type == TYPE_PATH) {
-			char *v;
-			if (git_config_pathname(&v, key_, value_) < 0)
-				return -1;
-			if (v)
-				strbuf_addstr(buf, v);
-			else
-				return 1; /* :(optional)no-such-file */
-			free((char *)v);
-		} else if (opts->type == TYPE_EXPIRY_DATE) {
+		else if (opts->type == TYPE_PATH)
+			res = format_config_path(buf, key_, value_, gently);
+		else if (opts->type == TYPE_EXPIRY_DATE) {
 			timestamp_t t;
 			if (git_config_expiry_date(&t, key_, value_) < 0)
 				return -1;
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 1fc8e788ee..48d9c554d8 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -2545,7 +2545,6 @@ test_expect_success 'list --type=bool-or-int shows only canonicalizable values'
 '
 
 test_expect_success 'list --type=path shows only canonicalizable path values' '
-	# TODO: handling of missing path is incorrect here.
 	cat >expect <<-EOF &&
 	section.foo=True
 	section.number=10
@@ -2554,7 +2553,7 @@ test_expect_success 'list --type=path shows only canonicalizable path values' '
 	section.red=red
 	section.blue=Blue
 	section.date=Fri Jun 4 15:46:55 2010
-	section.missing=section.exists=expect
+	section.exists=expect
 	EOF
 
 	git config ${mode_prefix}list --type=path >actual 2>err &&
-- 
gitgitgadget

[PATCH v3 09/13] config: format expiry dates quietly

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

Move the logic for formatting expiry date config values into a helper
method and use quiet parsing when needed.

Note that git_config_expiry_date() will show an error on a bad parse and
not die() like most other git_config...() parsers. Thus, we use
'quietly' here instead of 'gently'.

There is an unfortunate asymmetry in these two parsing methods, but we
need to treat a positive response from parse_expiry_date() as an error
or we will get incorrect values.

This updates the behavior of 'git config list --type=expiry-date' to be
quiet when attempting parsing on non-date values.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 builtin/config.c  | 27 +++++++++++++++++++++------
 t/t1300-config.sh | 11 +----------
 2 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 2828b6dcf1..ee77ddc87c 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -3,6 +3,7 @@
 #include "abspath.h"
 #include "config.h"
 #include "color.h"
+#include "date.h"
 #include "editor.h"
 #include "environment.h"
 #include "gettext.h"
@@ -333,6 +334,23 @@ static int format_config_path(struct strbuf *buf,
 	return 0;
 }
 
+static int format_config_expiry_date(struct strbuf *buf,
+				     const char *key_,
+				     const char *value_,
+				     int quietly)
+{
+	timestamp_t t;
+	if (quietly) {
+		if (parse_expiry_date(value_, &t))
+			return -1;
+	} else if (git_config_expiry_date(&t, key_, value_) < 0) {
+		return -1;
+	}
+
+	strbuf_addf(buf, "%"PRItime, t);
+	return 0;
+}
+
 /*
  * Format the configuration key-value pair (`key_`, `value_`) and
  * append it into strbuf `buf`.  Returns a negative value on failure,
@@ -368,12 +386,9 @@ static int format_config(const struct config_display_options *opts,
 			res = format_config_bool_or_str(buf, value_);
 		else if (opts->type == TYPE_PATH)
 			res = format_config_path(buf, key_, value_, gently);
-		else if (opts->type == TYPE_EXPIRY_DATE) {
-			timestamp_t t;
-			if (git_config_expiry_date(&t, key_, value_) < 0)
-				return -1;
-			strbuf_addf(buf, "%"PRItime, t);
-		} else if (opts->type == TYPE_COLOR) {
+		else if (opts->type == TYPE_EXPIRY_DATE)
+			res = format_config_expiry_date(buf, key_, value_, gently);
+		else if (opts->type == TYPE_COLOR) {
 			char v[COLOR_MAXLEN];
 			if (git_config_color(v, key_, value_) < 0)
 				return -1;
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 48d9c554d8..72bdd6ab03 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -2562,15 +2562,6 @@ test_expect_success 'list --type=path shows only canonicalizable path values' '
 '
 
 test_expect_success 'list --type=expiry-date shows only canonicalizable dates' '
-	cat >expecterr <<-EOF &&
-	error: '\''True'\'' for '\''section.foo'\'' is not a valid timestamp
-	error: '\''~/dir'\'' for '\''section.path'\'' is not a valid timestamp
-	error: '\''red'\'' for '\''section.red'\'' is not a valid timestamp
-	error: '\''Blue'\'' for '\''section.blue'\'' is not a valid timestamp
-	error: '\'':(optional)no-such-path'\'' for '\''section.missing'\'' is not a valid timestamp
-	error: '\'':(optional)expect'\'' for '\''section.exists'\'' is not a valid timestamp
-	EOF
-
 	git config ${mode_prefix}list --type=expiry-date >actual 2>err &&
 
 	# section.number and section.big parse as relative dates that could
@@ -2578,7 +2569,7 @@ test_expect_success 'list --type=expiry-date shows only canonicalizable dates' '
 	test_grep section.big actual &&
 	test_grep section.number actual &&
 	test_grep "section.date=$(git config --type=expiry-date section.$key)" actual &&
-	test_cmp expecterr err
+	test_must_be_empty err
 '
 
 test_expect_success 'list --type=color shows only canonicalizable color values' '
-- 
gitgitgadget

[PATCH v3 10/13] color: add color_parse_quietly()

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

When parsing colors, a failed parse leads to an error message due to the
result returning error(). To allow for quiet parsing, create
color_parse_quietly(). This is in contrast to an ..._gently() version
because the original does not die(), so both options are technically
'gentle'.

To accomplish this, convert the implementation of color_parse_mem() into
a static color_parse_mem_1() helper that adds a 'quiet' parameter. The
color_parse_quietly() method can then use this. Since it is a near
equivalent to color_parse(), move that method down in the file so they
can be nearby while also appearing after color_parse_mem_1().

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 color.c | 25 ++++++++++++++++++-------
 color.h |  1 +
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/color.c b/color.c
index 07ac8c9d40..00b53f97ac 100644
--- a/color.c
+++ b/color.c
@@ -223,11 +223,6 @@ static int parse_attr(const char *name, size_t len)
 	return -1;
 }
 
-int color_parse(const char *value, char *dst)
-{
-	return color_parse_mem(value, strlen(value), dst);
-}
-
 /*
  * Write the ANSI color codes for "c" to "out"; the string should
  * already have the ANSI escape code in it. "out" should have enough
@@ -264,7 +259,8 @@ static int color_empty(const struct color *c)
 	return c->type <= COLOR_NORMAL;
 }
 
-int color_parse_mem(const char *value, int value_len, char *dst)
+static int color_parse_mem_1(const char *value, int value_len,
+			     char *dst, int quiet)
 {
 	const char *ptr = value;
 	int len = value_len;
@@ -365,10 +361,25 @@ int color_parse_mem(const char *value, int value_len, char *dst)
 	OUT(0);
 	return 0;
 bad:
-	return error(_("invalid color value: %.*s"), value_len, value);
+	return quiet ? -1 : error(_("invalid color value: %.*s"), value_len, value);
 #undef OUT
 }
 
+int color_parse_mem(const char *value, int value_len, char *dst)
+{
+	return color_parse_mem_1(value, value_len, dst, 0);
+}
+
+int color_parse(const char *value, char *dst)
+{
+	return color_parse_mem(value, strlen(value), dst);
+}
+
+int color_parse_quietly(const char *value, char *dst)
+{
+	return color_parse_mem_1(value, strlen(value), dst, 1);
+}
+
 enum git_colorbool git_config_colorbool(const char *var, const char *value)
 {
 	if (value) {
diff --git a/color.h b/color.h
index 43e6c9ad09..0d72540300 100644
--- a/color.h
+++ b/color.h
@@ -118,6 +118,7 @@ bool want_color_fd(int fd, enum git_colorbool var);
  * terminal.
  */
 int color_parse(const char *value, char *dst);
+int color_parse_quietly(const char *value, char *dst);
 int color_parse_mem(const char *value, int len, char *dst);
 
 /*
-- 
gitgitgadget

[PATCH v3 11/13] config: format colors quietly

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

Move the logic for formatting color config value into a helper method
and use quiet parsing when needed.

This removes error messages when parsing a list of config values that do
not match color formats.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 builtin/config.c  | 27 +++++++++++++++++++++------
 t/t1300-config.sh | 11 +----------
 2 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index ee77ddc87c..45304076dc 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -351,6 +351,24 @@ static int format_config_expiry_date(struct strbuf *buf,
 	return 0;
 }
 
+static int format_config_color(struct strbuf *buf,
+			       const char *key_,
+			       const char *value_,
+			       int gently)
+{
+	char v[COLOR_MAXLEN];
+
+	if (gently) {
+		if (color_parse_quietly(value_, v) < 0)
+			return -1;
+	} else if (git_config_color(v, key_, value_) < 0) {
+		return -1;
+	}
+
+	strbuf_addstr(buf, v);
+	return 0;
+}
+
 /*
  * Format the configuration key-value pair (`key_`, `value_`) and
  * append it into strbuf `buf`.  Returns a negative value on failure,
@@ -388,12 +406,9 @@ static int format_config(const struct config_display_options *opts,
 			res = format_config_path(buf, key_, value_, gently);
 		else if (opts->type == TYPE_EXPIRY_DATE)
 			res = format_config_expiry_date(buf, key_, value_, gently);
-		else if (opts->type == TYPE_COLOR) {
-			char v[COLOR_MAXLEN];
-			if (git_config_color(v, key_, value_) < 0)
-				return -1;
-			strbuf_addstr(buf, v);
-		} else if (value_) {
+		else if (opts->type == TYPE_COLOR)
+			res = format_config_color(buf, key_, value_, gently);
+		else if (value_) {
 			strbuf_addstr(buf, value_);
 		} else {
 			/* Just show the key name; back out delimiter */
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 72bdd6ab03..128971ee12 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -2579,19 +2579,10 @@ test_expect_success 'list --type=color shows only canonicalizable color values'
 	section.blue=<BLUE>
 	EOF
 
-	cat >expecterr <<-EOF &&
-	error: invalid color value: True
-	error: invalid color value: 1M
-	error: invalid color value: ~/dir
-	error: invalid color value: Fri Jun 4 15:46:55 2010
-	error: invalid color value: :(optional)no-such-path
-	error: invalid color value: :(optional)expect
-	EOF
-
 	git config ${mode_prefix}list --type=color >actual.raw 2>err &&
 	test_decode_color <actual.raw >actual &&
 	test_cmp expect actual &&
-	test_cmp expecterr err
+	test_must_be_empty err
 '
 
 test_expect_success '--type rejects unknown specifiers' '
-- 
gitgitgadget

[PATCH v3 12/13] config: restructure format_config()

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

The recent changes have replaced the bodies of most if/else-if cases
with simple helper method calls. This makes it easy to adapt the
structure into a clearer switch statement, leaving a simple if/else in
the default case.

Make things a little simpler to read by reducing the nesting depth via a
new goto statement when we want to skip values.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 builtin/config.c | 64 ++++++++++++++++++++++++++++++++++--------------
 1 file changed, 45 insertions(+), 19 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 45304076dc..2e8bc6590c 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -124,6 +124,7 @@ struct config_display_options {
 	.key_delim = ' ', \
 }
 
+#define TYPE_NONE		0
 #define TYPE_BOOL		1
 #define TYPE_INT		2
 #define TYPE_BOOL_OR_INT	3
@@ -390,32 +391,57 @@ static int format_config(const struct config_display_options *opts,
 		show_config_origin(opts, kvi, buf);
 	if (opts->show_keys)
 		strbuf_addstr(buf, key_);
-	if (!opts->omit_values) {
-		if (opts->show_keys)
-			strbuf_addch(buf, opts->key_delim);
-
-		if (opts->type == TYPE_INT)
-			res = format_config_int64(buf, key_, value_, kvi, gently);
-		else if (opts->type == TYPE_BOOL)
-			res = format_config_bool(buf, key_, value_, gently);
-		else if (opts->type == TYPE_BOOL_OR_INT)
-			res = format_config_bool_or_int(buf, key_, value_, kvi, gently);
-		else if (opts->type == TYPE_BOOL_OR_STR)
-			res = format_config_bool_or_str(buf, value_);
-		else if (opts->type == TYPE_PATH)
-			res = format_config_path(buf, key_, value_, gently);
-		else if (opts->type == TYPE_EXPIRY_DATE)
-			res = format_config_expiry_date(buf, key_, value_, gently);
-		else if (opts->type == TYPE_COLOR)
-			res = format_config_color(buf, key_, value_, gently);
-		else if (value_) {
+
+	if (opts->omit_values)
+		goto terminator;
+
+	if (opts->show_keys)
+		strbuf_addch(buf, opts->key_delim);
+
+	switch (opts->type) {
+	case TYPE_INT:
+		res = format_config_int64(buf, key_, value_, kvi, gently);
+		break;
+
+	case TYPE_BOOL:
+		res = format_config_bool(buf, key_, value_, gently);
+		break;
+
+	case TYPE_BOOL_OR_INT:
+		res = format_config_bool_or_int(buf, key_, value_, kvi, gently);
+		break;
+
+	case TYPE_BOOL_OR_STR:
+		res = format_config_bool_or_str(buf, value_);
+		break;
+
+	case TYPE_PATH:
+		res = format_config_path(buf, key_, value_, gently);
+		break;
+
+	case TYPE_EXPIRY_DATE:
+		res = format_config_expiry_date(buf, key_, value_, gently);
+		break;
+
+	case TYPE_COLOR:
+		res = format_config_color(buf, key_, value_, gently);
+		break;
+
+	case TYPE_NONE:
+		if (value_) {
 			strbuf_addstr(buf, value_);
 		} else {
 			/* Just show the key name; back out delimiter */
 			if (opts->show_keys)
 				strbuf_setlen(buf, buf->len - 1);
 		}
+		break;
+
+	default:
+		BUG("undefined type %d", opts->type);
 	}
+
+terminator:
 	strbuf_addch(buf, opts->term);
 	return res;
 }
-- 
gitgitgadget

[PATCH v3 13/13] config: use an enum for type

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <stolee@gmail.com>

The --type=<X> option for 'git config' has previously been defined using
macros, but using a typed enum is better for tracking the possible
values.

Move the definition up to make sure it is defined before a macro uses
some of its terms.

Update the initializer for config_display_options to explicitly set
'type' to TYPE_NONE even though this is implied by a zero value.

This assists in knowing that the switch statement added in the previous
change has a complete set of cases for a properly-valued enum.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 builtin/config.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 2e8bc6590c..7c4857be62 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -86,6 +86,17 @@ struct config_location_options {
 	.respect_includes_opt = -1, \
 }
 
+enum config_type {
+	TYPE_NONE = 0,
+	TYPE_BOOL,
+	TYPE_INT,
+	TYPE_BOOL_OR_INT,
+	TYPE_PATH,
+	TYPE_EXPIRY_DATE,
+	TYPE_COLOR,
+	TYPE_BOOL_OR_STR,
+};
+
 #define CONFIG_TYPE_OPTIONS(type) \
 	OPT_GROUP(N_("Type")), \
 	OPT_CALLBACK('t', "type", &type, N_("type"), N_("value is given this type"), option_parse_type), \
@@ -111,7 +122,7 @@ struct config_display_options {
 	int show_origin;
 	int show_scope;
 	int show_keys;
-	int type;
+	enum config_type type;
 	char *default_value;
 	/* Populated via `display_options_init()`. */
 	int term;
@@ -122,17 +133,9 @@ struct config_display_options {
 	.term = '\n', \
 	.delim = '=', \
 	.key_delim = ' ', \
+	.type = TYPE_NONE, \
 }
 
-#define TYPE_NONE		0
-#define TYPE_BOOL		1
-#define TYPE_INT		2
-#define TYPE_BOOL_OR_INT	3
-#define TYPE_PATH		4
-#define TYPE_EXPIRY_DATE	5
-#define TYPE_COLOR		6
-#define TYPE_BOOL_OR_STR	7
-
 #define OPT_CALLBACK_VALUE(s, l, v, h, i) { \
 	.type = OPTION_CALLBACK, \
 	.short_name = (s), \
-- 
gitgitgadget