Matthieu Moy <Matthieu.Moy@xxxxxxxxxxxxxxx> writes: > Tanay Abhra <tanayabh@xxxxxxxxx> 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. >> 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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html