git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Tanay Abhra <tanayabh@gmail.com>
Date
Jul 24, 2014, 19:29 UTC
Message-ID
<53D15EB4.1050303@gmail.com>
In-Reply-To
<xmqqppgu8sxz.fsf@gitster.dls.corp.google.com>
On 7/25/2014 12:50 AM, Junio C Hamano wrote:
Show 33 quoted lines
> 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.

Show 11 quoted lines
>>> 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.

Previous: Junio C HamanoNext: Junio C Hamano
Message 31 of 51 in “Rewrite `git_config()` using config-set API”
  1. 0/7 Rewrite `git_config()` using config-set APITanay Abhra, Jul 23, 2014
  2. 1/7 config.c: fix accuracy of line number in errorsTanay Abhra, Jul 23, 2014
  3. Junio C HamanoJul 23, 2014
  4. Tanay AbhraJul 24, 2014
  5. Junio C HamanoJul 24, 2014
  6. Tanay AbhraJul 24, 2014
  7. Matthieu MoyJul 24, 2014
  8. 2/7 rewrite git_config() to use the config-set APITanay Abhra, Jul 23, 2014
  9. Matthieu MoyJul 23, 2014
  10. Matthieu MoyJul 23, 2014
  11. Junio C HamanoJul 23, 2014
  12. Matthieu MoyJul 24, 2014
  13. 3/7 add a test for semantic errors in config filesTanay Abhra, Jul 23, 2014
  14. Matthieu MoyJul 23, 2014
  15. Junio C HamanoJul 23, 2014
  16. Tanay AbhraJul 24, 2014
  17. Matthieu MoyJul 24, 2014
  18. Junio C HamanoJul 24, 2014
  19. 4/7 add line number and file name info to `config_set`Tanay Abhra, Jul 23, 2014
  20. Junio C HamanoJul 23, 2014
  21. 5/7 enforce `xfuncname` precedence over `funcname`Tanay Abhra, Jul 23, 2014
  22. Matthieu MoyJul 23, 2014
  23. Eric SunshineJul 23, 2014
  24. Junio C HamanoJul 23, 2014
  25. Matthieu MoyJul 24, 2014
  26. Junio C HamanoJul 24, 2014
  27. Tanay AbhraJul 24, 2014
  28. Matthieu MoyJul 24, 2014
  29. John KeepingJul 24, 2014
  30. Junio C HamanoJul 24, 2014
  31. Tanay AbhraJul 24, 2014
  32. Junio C HamanoJul 24, 2014
  33. Ramsay JonesJul 24, 2014
  34. Tanay AbhraJul 25, 2014
  35. Matthieu MoyJul 25, 2014
  36. Junio C HamanoJul 25, 2014
  37. Matthieu MoyJul 25, 2014
  38. Junio C HamanoJul 25, 2014
  39. 6/7 config: add `git_die_config()` to the config-set APITanay Abhra, Jul 23, 2014
  40. 7/7 Add tests for `git_config_get_string()`Tanay Abhra, Jul 23, 2014
  41. Matthieu MoyJul 23, 2014
  42. Matthieu MoyJul 23, 2014
  43. Junio C HamanoJul 23, 2014
  44. Tanay AbhraJul 24, 2014
  45. Matthieu MoyJul 24, 2014
  46. Tanay AbhraJul 24, 2014
  47. Matthieu MoyJul 24, 2014
  48. Junio C HamanoJul 24, 2014
  49. 1/2 add `config_set` API for caching config-like filesTanay Abhra, Jul 24, 2014
  50. Junio C HamanoJul 24, 2014
  51. 2/2 test-config: add tests for the config_set APITanay Abhra, Jul 24, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.