Re: [PATCH v6 3/6] config: add repo_config_set_worktree_gently()

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 9, 2022, 17:49 UTC
Message-ID: <xmqqzgn07wwf.fsf@gitster.g>
In-Reply-To: <ab6021a8-cbb9-3261-8e08-bb4a83287473@gmail.com>

Derrick Stolee <stolee@gmail.com> writes:

Show 19 quoted lines

> On 2/8/2022 5:18 PM, Junio C Hamano wrote:
>> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> ...
>>> @@ -3181,14 +3196,28 @@ void git_config_set_multivar_in_file(const char *config_filename,
>>>  int git_config_set_multivar_gently(const char *key, const char *value,
>>>  				   const char *value_pattern, unsigned flags)
>>>  {
>>> -	return git_config_set_multivar_in_file_gently(NULL, key, value, value_pattern,
>>> -						      flags);
>>> +	return repo_config_set_multivar_gently(the_repository, key, value,
>>> +					       value_pattern, flags);
>>> +}
>> 
>> Is this an unrelated "morally no-op" change?
>
> This one is to match the pattern of how "git_*" methods should
> depend on their "repo_*" counterparts (with "the_repository" inserted
> properly). So, it's part of the standard process for creating these
> "repo_*" variants.

If only one of repo_config_set_multivar_gently() and git_config_set_multivar_gently() existed and we were completing the pair, then I would understand the explanation, but the title says that it is adding repo_config_set_worktree_gently(), which is not, and that is where the "unrelated" comes from.

It needs to be a separate preparatory step to add repo_config_set_multivar_gently() before we add repo_config_set_worktree_gently(), perhaps?

A bit higher level question is if the public part of "config-set" API functions should gain an "easy" (in the sense of curl_easy_* set of API functions) API to allow the callers to say "I do not care to find out if per-worktree configuration is in use, or this particular variable is meant to be per-worktree, just set it to this value".

On this question, I am of two minds. As certain variables (like core.sparseCheckout) should always be per-worktree just like certain refs (like HEAD) should always be per-worktree, I can understand the viewpoint that the callers _ought_ to know and explicitly say that they want to get/set in the per-worktree configuration file, but at the same time, I would think the callers should not have to care. So, I dunno.

Thanks.