Re: [PATCH 2/3] environment: clarify repository config getter documentation

[Tian Yuchen <cat@malon.dev>]

On 8/6/26 05:38, Junio C Hamano wrote:
> Tian Yuchen <cat@malon.dev> writes:
> 
>> Update the comment above repository config getters to describe their
>> common behavior.
>>
>> The getters handle repositories that are not fully initialized by
>> returning the corresponding default values.
>>
>> Mentored-by: Christian Couder <christian.couder@gmail.com>
>> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
>> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
>> Signed-off-by: Tian Yuchen <cat@malon.dev>
>> ---
>>   environment.h | 11 +++--------
>>   1 file changed, 3 insertions(+), 8 deletions(-)
>>
>> diff --git a/environment.h b/environment.h
>> index e7ec5b0437..30678257b5 100644
>> --- a/environment.h
>> +++ b/environment.h
>> @@ -175,18 +175,13 @@ int git_default_core_config(const char *var, const char *value,
>>   			    const struct config_context *ctx, void *cb);
>>   
>>   /*
>> - * Getters for the `protect_hfs` and `protect_ntfs` fields of `struct repo_config_values`.
>> - * They check `repo->initialized` to prevent calling `repo_config_values()`
>> - * before the repository setup is fully complete or in non-git environments.
>> + * Getters for configuration variables in `struct repo_config_values`.
>> + * These functions handle uninitialized repositories or non-git
>> + * environments by returning appropriate default values.
>>    */
>>   int repo_protect_hfs(struct repository *repo);
>>   int repo_protect_ntfs(struct repository *repo);
>>   
>> -/*
> 
> Two puzzlements.
> 
>   * Is the above comment block meant to apply to repo_ignore_case()
>     in addition to repo_protect_ntfs() and repo_protect_hfs()?  If
>     so, the blank line before repo_ignore_case() is a bit misleading.
> 

Not really, they are meant to apply to all getters below. I will remove 
the blank lines.

>   * The phrase "uninitialized repositories or non-Git environments"
>     strongly hints that I can pass NULL to indicate that we are
>     running in a non-Git environment.  However, the change in
>     [PATCH 1/3] we just saw means I would get a segfault if I did so,
>     does it not?
> 

This is a mistake. I meant "these getters can handle repositories, even 
when they are not fully initailzed" but not "these getters can handle 
whatever we pass in". So I will change it in the next reroll.

>> - * Getter for the `ignore_case` field of `struct repo_config_values`.
>> - * It checks `repo->initialized` to prevent calling repo_config_values()`
>> - * before the repository setup is fully complete or in non-git environments.
>> - */
>>   int repo_ignore_case(struct repository *repo);
>>   
>>   int repo_trust_executable_bit(struct repository *repo);

Thanks! yuchen