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

From: Derrick Stolee <stolee@gmail.com>
Date: Feb 9, 2022, 02:27 UTC
Message-ID: <ab6021a8-cbb9-3261-8e08-bb4a83287473@gmail.com>
In-Reply-To: <xmqqv8xpato3.fsf@gitster.g>

On 2/8/2022 5:18 PM, Junio C Hamano wrote:

> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

...

Show 11 quoted lines

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

Show 14 quoted lines

>>  void git_config_set_multivar(const char *key, const char *value,
>>  			     const char *value_pattern, unsigned flags)
>>  {
>> -	git_config_set_multivar_in_file(NULL, key, value, value_pattern,
>> +	git_config_set_multivar_in_file(git_path("config"),
>> +					key, value, value_pattern,
>>  					flags);
>>  }
> 
> Is this an unrelated "morally no-op" change?
> 
> It might have value to make caller more explicit by reducing the use
> of "I give NULL, you use 'config' for me", but that doesn't sound
> related to the addition of set_per_worktree_config_gently() helper.

Here, you're right. This one should have followed the same pattern of having the "git_*" equivalent call the "repo_*" method, but instead I incorrectly inlined some of the code.

The proper body should be

void git_config_set_multivar(const char *key, const char *value,
			     const char *value_pattern, unsigned flags)
{
	repo_config_set_multivar_gently(the_repository, key, value,
					value_pattern, flags);
}

to follow convention.

Thanks, -Stolee