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

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

From
Junio C Hamano <gitster@pobox.com>
Date
Jul 24, 2014, 19:20 UTC
Message-ID
<xmqqppgu8sxz.fsf@gitster.dls.corp.google.com>
In-Reply-To
<vpqegxa386m.fsf@anie.imag.fr>
Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
Show 19 quoted lines
> 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.

>> 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.

Previous: John KeepingNext: Tanay Abhra
Message 30 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.