Re: [PATCH 5/7] enforce `xfuncname` precedence over `funcname`

[Tanay Abhra <tanayabh@gmail.com>]

On 7/25/2014 12:50 AM, Junio C Hamano wrote:
> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> 
>> Tanay Abhra <tanayabh@gmail.com> writes:
>>
>>> For core the only test failing was xfuncname vs funcname,
>>
>> Being a little pessimistic: there may be other cases where the hashtable
>> magically gives the right order for existing tests, but that would fail
>> for untested use-cases.
>>
>> But I can't think of any such case.
>>
>>> so the situation is not as bad as you think. One course of action
>>> would be leave git_config() as it is, so that third party apps
>>> may not be broken. Provide a function like git_config_cache(),
>>> then rename all the git_config() calls in core to git_config_cache(),
>>> fallback to git_config() where it is not applicable (for example,
>>> git config -l).
>>
>> I think Junio's point about "git config -l" is correct: we should just
>> keep git_config_raw there.
> 
> I have a slight preference of making git_config() do the right thing
> and take advantage of caching behaviour, to be honest.  How much
> extra effort is necessary in your code to carry a piece of
> information, in addition to what lets you say "here are the values
> associated with that key in the order we read from the files", in
> order to be able to say "by the way, here is the order of key-value
> pairs, if you want to show everything we read in the same order"?
> If it would be excessive, using _raw() could be an easy way to punt,
> but because we know it is easy to decide to punt, I'd like to see us
> see if a real solution is feasible first before punting.
>

I am thinking over it, lets see if there is a way before we take the
easy route.

>>> Also can you name any third party apps that use the git_config()
>>> system on which I can test the patches.
>>
>> There are probably tons of. I can think of git-multimail.
> 
> This question does not really matter.
> 
> Among the various points, I actually think the last one you omitted
> from your quote, closing door to one useful way to fix UI mistakes
> in the future, is the most serious one.
>

If we take the easy way out, fixing UI mistakes would be easier,
just replace git_config_cache() with git_config_raw() for such cases.