Re: [PATCH v2 3/4] config: factor out global config file retrieval

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 1582 bytes --]

On Fri, Jan 19, 2024 at 08:40:51AM +0100, Kristoffer Haugsbakk wrote:
> On Fri, Jan 19, 2024, at 07:18, Patrick Steinhardt wrote:
> > But second, I think that the new function you introduce here has the
> > same issue as the old function that you refactored in the preceding
> > patch: `git_config_global()` isn't very descriptive, and it is also
> > inconsistent the new `git_config_global_paths()`. I'd propose to name
> > the new function something like `git_config_global_preferred_path()` or
> > `git_config_global_path()`.
> 
> The choice of `git_config_global` is mostly motivated by it working the
> same way as `git_config_system`:
> 
> ```
> given_config_source.file = git_system_config();
> […]
> given_config_source.file = git_global_config();
> ```
> 
> (The extra logic imposed by XDG for “global” is implied by `man git
> config`. I don’t know what the guidelines are for spelling that out or not
> in the internal functions.)
> 
> Your suggestion makes sense. But should `git_system_config` be renamed as
> well?

Yeah, you're right that `git_system_config()` is bad in the same way. In
fact I think it's worse here because we have both `git_config_system()`
and `git_system_config()`, which has certainly confused me multiple
times in the past. So I'd be happy to see it renamed, as well, either
now or in a follow-up patch series.

But as I said, I don't think it's a prereq for this patch series to land
and others may have differing opinions. So please, go ahead as you deem
fit (or wait for other opinions).

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]