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

[Ramsay Jones <ramsay@ramsayjones.plus.com>]

On 16/02/16 09:51, Lars Schneider wrote:
> 
> On 15 Feb 2016, at 23:39, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote:
> 
>>
>>
>> On 15/02/16 21:40, Jeff King wrote:
>>> On Mon, Feb 15, 2016 at 09:36:23PM +0000, Ramsay Jones wrote:
>>>
>>>>> +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.
>>
>>>
>>> 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