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

[Junio C Hamano <gitster@pobox.com>]

Derrick Stolee <stolee@gmail.com> writes:

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