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

[Junio C Hamano <gitster@pobox.com>]

"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?