Re: [PATCH v4 3/3] config: add '--show-origin' option to print the origin of a config value

On 16/02/16 09:51, Lars Schneider wrote:

On 15 Feb 2016, at 23:39, Ramsay Jones [off-list ref] wrote:

quoted

On 15/02/16 21:40, Jeff King wrote:

quoted

On Mon, Feb 15, 2016 at 09:36:23PM +0000, Ramsay Jones wrote:

quoted

quoted

+test_expect_success '--show-origin stdin' '
+	cat >expect <<-\EOF &&
+		stdin:	user.custom=true

So, as with the previous patch, I think this should be:
		file:<stdin>	user.custom=true

That's ambiguous with a file named "<stdin>", which was the point of
having the two separate prefixes in the first place.

I think in practice we _could_ get by with an ambiguous output (it's not
like "<stdin>" is a common filename), but that was discussed earlier in
the thread, and Lars decided to go for something unambiguous.

sure, I just don't think it would cause a problem in practice.
How about using '-' for <stdin>? Hmm, you can actually create
such a file in the filesystem! Oh well, I guess its not a big deal.

quoted

That doesn't necessarily have to bleed over into the error messages,
though (which could continue to use "<stdin>" if we want to put in a
little extra code to covering the cases separately.

Yep.

Sorry for not replying earlier - today has been hectic, so far.

OK, I am happy to add the extra code. 

Unless I've missed something (quite possible), this patch does not
need to change. (you have (both) convinced me that your current
solution is the best).

The only change that I would suggest is the one-liner I already
suggested to the previous patch (plus the one-liner in the test,
of course. err ... so the two-liner!). Having said that, I didn't
try it out - I was just typing into my email client, so ...

                                      However, out of curiosity, can
you explain in what cases you actually use configs from stdin? I wasn't
aware of this feature before working on this patch and I still wonder
when I would use it.

Personally, I can't imagine ever using it. (I don't have a great
imagination. ;-)

Since I couldn't recall when this feature was added, I looked for
the commit that added it and found it was merged in commit 08f36302.
In particular, commit 3caec73b ("config: teach "git config --file -"
to read from the standard input", 19-02-2014) does not seem to
include any motivation for the change. The corresponding release
notes for v2.2.0 do not seem to add anything either.

So, I'm not much help here. :(

[Ah, looking at the date on the merge explains why I didn't
notice this.]

                 If it is only a seldom used feature then I am not
sure if adding the extra code to restore the existing error message
is worth the effort?

ATB,
Ramsay Jones