v2config: add '--sources' option to print the source of a config value

> From: Lars Schneider <larsxschneider@gmail.com>
> 
> If config values are queried using 'git config' (e.g. via '--list' flag
> or the '--get*' flags) then it is sometimes hard to find the
> configuration file where the values were defined.
> 
> Teach 'git config' the '--sources' option to print the source
> configuration file for every printed value.
> 
> Based-on-patch-by: Jeff King <peff@peff.net>
> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
> ---
> 
> diff to v1:
> 
> * add documention
> * produce a consistent (tab-delimited) format that can be parsed
> * adhere declaration-after-statement style
> * prefix every source line with the source type (file, stdin, blob, cmd)
> * add relative path test case
> * add blob ref test case
> * add "git config --local" test case (Note: I think I found a bug there that I
>   plan to fix/investigate in a seperate patch. Is it ok to leave the TODO?)
> * add a test case to check funny character escapes (file name with tabs)
> 
> Sebastian suggested "--show-origin" as a better option name over "--sources".
> I still believe "--sources" might be slightly better as I fear that users could
> somehow related "origin" to "remote" kind of configs. However, I am happy to
> change that if a majority prefers "--show-origin".

>> Sebastian suggested "--show-origin" as a better option name over "--sources".
>> I still believe "--sources" might be slightly better as I fear that users could
>> somehow related "origin" to "remote" kind of configs. However, I am happy to
>> change that if a majority prefers "--show-origin".
>
> <bikeshed>
> As I have said before, I'm not very good at naming things, but ...
>
> Of the two, I *slightly* prefer --show-origin, since I don't think
> there will be any confusion. However, I think --source may be OK too
> (for some reason it sounds better than the plural). Another idea
> may be --show-source. ;-)
>
> </bikeshed>

> On Wed, Feb 10, 2016 at 1:47 PM, Ramsay Jones
> <ramsay@ramsayjones.plus.com> wrote:
> 
>>> Sebastian suggested "--show-origin" as a better option name over "--sources".
>>> I still believe "--sources" might be slightly better as I fear that users could
>>> somehow related "origin" to "remote" kind of configs. However, I am happy to
>>> change that if a majority prefers "--show-origin".
>>
>> <bikeshed>
>> As I have said before, I'm not very good at naming things, but ...
>>
>> Of the two, I *slightly* prefer --show-origin, since I don't think
>> there will be any confusion. However, I think --source may be OK too
>> (for some reason it sounds better than the plural). Another idea
>> may be --show-source. ;-)
>>
>> </bikeshed>
> 
> I agree that using --source sounds better than --sources, as the
> latter sounds even more like "source code".
> 
> Here's another idea: How about --declaration or --show-declaration?
> 

> diff to v1:
> 
> * add documention
> * produce a consistent (tab-delimited) format that can be parsed
> * adhere declaration-after-statement style
> * prefix every source line with the source type (file, stdin, blob, cmd)
> * add relative path test case
> * add blob ref test case
> * add "git config --local" test case (Note: I think I found a bug there that I
>   plan to fix/investigate in a seperate patch. Is it ok to leave the TODO?)
> * add a test case to check funny character escapes (file name with tabs)
> 
> Sebastian suggested "--show-origin" as a better option name over "--sources".
> I still believe "--sources" might be slightly better as I fear that users could
> somehow related "origin" to "remote" kind of configs. However, I am happy to
> change that if a majority prefers "--show-origin".

> @@ -194,6 +194,11 @@ See also <<FILES>>.
>  	Output only the names of config variables for `--list` or
>  	`--get-regexp`.
> 
> +--sources::
> +	Augment the output of all queried config options with the
> +	source type (file, stdin, blob, cmd) and the actual source
> +	(config file path, ref, or blob id if applicable).

> +/* output to either fp or buf; only one should be non-NULL */
> +static void show_config_source(struct strbuf *buf, FILE *fp)
> +{
> +	char term = '\t';
> +	char *prefix;
> +	const char *fn = current_config_filename();

> +	if (end_null)
> +		term = '\0';
> +
> +	if (fn) {
> +		if (given_config_source.use_stdin)
> +			prefix = "stdin";
> +		else if (given_config_source.blob)
> +			prefix = "blob";
> +		else
> +			prefix = "file";
> +	} else {
> +		fn = "";
> +		prefix = "cmd";
> +	}

> +	if (fp)
> +		fprintf(fp, "%s", prefix);
> +	else {
> +		strbuf_addstr(buf, prefix);
> +	}
> +
> +	if (fp)
> +		fputc(term, fp);
> +	else
> +		strbuf_addch(buf, term);

> +
> +	is_query_action = actions & (
> +		ACTION_GET|ACTION_GET_ALL|ACTION_GET_REGEXP|ACTION_LIST
> +	);
> +
> +	if (show_sources && !is_query_action) {
> +		error("--sources is only applicable to --list or --get-* actions");
> +		usage_with_options(builtin_config_usage, builtin_config_options);
> +	}

> @@ -1078,7 +1078,7 @@ static int do_config_from_file(config_fn_t fn,
> 
>  static int git_config_from_stdin(config_fn_t fn, void *data)
>  {
> -	return do_config_from_file(fn, "<stdin>", NULL, stdin, data);
> +	return do_config_from_file(fn, "", NULL, stdin, data);
>  }

> +	# TODO:
> +	# The locally included config value is not printed below. I think
> +	# this is a bug.
> +	# "file	.git/../include/relative.include	user.relative=include"
> +	#
> +	cat >expect <<-EOF &&
> +		file	.git/config	include.path=../include/relative.include
> +		file	.git/config	user.local=true
> +		file	.git/config	user.override=local
> +		EOF
> +	git config --local --list --sources >output &&
> +	test_cmp expect output &&

> +	cat >expect <<-EOF &&
> +		file	$HOME/.gitconfig	user.global true
> +		file	.git/config	user.local true
> +	EOF
> +	git config --sources --get-regexp "user\.[g|l].*" >output &&
> +	test_cmp expect output &&
> +
> +	cat >expect <<-EOF &&
> +		file	.git/config	local
> +	EOF
> +	git config --sources user.override >output &&
> +	test_cmp expect output &&

>> +--sources::
>> +     Augment the output of all queried config options with the
>> +     source type (file, stdin, blob, cmd) and the actual source
>> +     (config file path, ref, or blob id if applicable).
>
> I think something like "cmdline" might be more descriptive than "cmd".

> So the format here is like:
>
>   file\t<filename>\t<value...>
>   blob\t<blob>\t<value...>
>   stdin\t\t<value...>
>   cmd\t\t<value...>
>
> where two of the prefixes have nothing in the second slot. I expected
> something more like:
>
>   file:<filename>\t</value...>
>   blob:<blob>\t<value...>
>   stdin\t<value...>
>   cmd\t<value...>
>
> with a single delimited slot for the source, which can then be broken
> down further if desired.  I can't think of any reason to prefer one over
> the other rather than personal preference, though. They can both be
> parsed unambiguously.

> > where two of the prefixes have nothing in the second slot. I expected
> > something more like:
> >
> >   file:<filename>\t</value...>
> >   blob:<blob>\t<value...>
> >   stdin\t<value...>
> >   cmd\t<value...>
> >
> > with a single delimited slot for the source, which can then be broken
> > down further if desired.  I can't think of any reason to prefer one over
> > the other rather than personal preference, though. They can both be
> > parsed unambiguously.
> 
> I also would have expected sopme like the latter, except that I'd also
> expect a colon after "stdin" and "cmd" (or "cmdline", as said above).
> I.e. the colon should be part of the prefix to mark it as such.

>> >   file:<filename>\t</value...>
>> >   blob:<blob>\t<value...>
>> >   stdin\t<value...>
>> >   cmd\t<value...>
>> >
>> > with a single delimited slot for the source, which can then be broken
>> > down further if desired.  I can't think of any reason to prefer one over
>> > the other rather than personal preference, though. They can both be
>> > parsed unambiguously.
>>
>> I also would have expected sopme like the latter, except that I'd also
>> expect a colon after "stdin" and "cmd" (or "cmdline", as said above).
>> I.e. the colon should be part of the prefix to mark it as such.
>
> Yeah, I waffled on that. Having a colon means you can definitely parse
> to the first ":" without looking at what the prefix is. But if you don't
> know what the prefix is, I don't know what good that does you. IOW, I'd

> >> I also would have expected sopme like the latter, except that I'd also
> >> expect a colon after "stdin" and "cmd" (or "cmdline", as said above).
> >> I.e. the colon should be part of the prefix to mark it as such.
> >
> > Yeah, I waffled on that. Having a colon means you can definitely parse
> > to the first ":" without looking at what the prefix is. But if you don't
> > know what the prefix is, I don't know what good that does you. IOW, I'd
> 
> IMO that's asking the wrong question. The question should not be "what
> good does it do if we add the colons also there", but "what
> justification do we have to introduce an inconsistency by not adding
> them".

> I agree the option wording mostly is personal preference. On the other
> hand, I find discussions like these often prematurely waved aside as
> bikeshedding, just because e.g. Perl can parse the one as good as the
> other. But it's not about Perl, it's about humans. IMO Git has
> historically made many mistakes by not caring enough about consistency
> in docs, command and command line option naming, so I don't see it as
> wasted time to discuss about things like this.

> If config values are queried using 'git config' (e.g. via '--list' flag
> or the '--get*' flags) then it is sometimes hard to find the
> configuration file where the values were defined.
> 
> Teach 'git config' the '--sources' option to print the source
> configuration file for every printed value.
> 
> Based-on-patch-by: Jeff King <peff@peff.net>
> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
> ---
> diff --git a/builtin/config.c b/builtin/config.c
> @@ -91,8 +94,58 @@ static void check_argc(int argc, int min, int max) {
> +/* output to either fp or buf; only one should be non-NULL */
> +static void show_config_source(struct strbuf *buf, FILE *fp)
> +{
> +	char term = '\t';
> +	char *prefix;
> +	const char *fn = current_config_filename();
> +
> +	if (end_null)
> +		term = '\0';
> +
> +	if (fn) {
> +		if (given_config_source.use_stdin)
> +			prefix = "stdin";
> +		else if (given_config_source.blob)
> +			prefix = "blob";
> +		else
> +			prefix = "file";
> +	} else {
> +		fn = "";
> +		prefix = "cmd";
> +	}
> +
> +	if (fp)
> +		fprintf(fp, "%s", prefix);
> +	else {
> +		strbuf_addstr(buf, prefix);
> +	}

> +
> +	if (fp)
> +		fputc(term, fp);
> +	else
> +		strbuf_addch(buf, term);

> +
> +	if (!end_null)
> +		quote_c_style(fn, buf, fp, 0);
> +	else {
> +		if (fp)
> +			fprintf(fp, "%s", fn);
> +		else
> +			strbuf_addstr(buf, fn);
> +	}
> +
> +	if (fp)
> +		fputc(term, fp);
> +	else
> +		strbuf_addch(buf, term);
> +}