From: Phillip Wood <phillip.wood123@gmail.com>
Date: Fri, 04 Jul 2025 08:23:02 GMT
Subject: Re: [GSOC PATCH v3] commit: avoid scanning trailing comments when 'core.commentChar' is "auto"
Message-ID: <062e7abd-97b1-4806-9753-338906642265@gmail.com>
In-Reply-To: <CAE7as+abNzqbGSCWsuYe8D_c5dBUuRdDEbHL0pVW5j3kTMER4Q@mail.gmail.com>

Hi Ayush

On 03/07/2025 00:46, Ayush Chandekar wrote:
> On Wed, Jul 2, 2025 at 1:02 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>> diff --git a/config.c b/config.c
>> index eb60c293ab3..bb75bdc65d3 100644
>> --- a/config.c
>> +++ b/config.c
>> @@ -1537,9 +1537,11 @@ static int git_default_core_config(const char
>> *var, const char *value,
>>                !strcmp(var, "core.commentstring")) {
>>                    if (!value)
>>                            return config_error_nonbool(var);
>> -                else if (!strcasecmp(value, "auto"))
>> +                else if (!strcasecmp(value, "auto")) {
>>                            auto_comment_line_char = 1;
>> -                else if (value[0]) {
>> +                        FREE_AND_NULL(comment_line_str_to_free);
>> +                        comment_line_str = "#";
>> +                } else if (value[0]) {
>>                            if (strchr(value, '\n'))
>>                                    return error(_("%s cannot contain
>> newline"), var);
>>                            comment_line_str = value;
>>
> 
> Thanks, I understood it.
> 
> What if we simply return the function `adjust_comment_line_char()` if
> we get a non-zero value from `ignored_log_message_bytes()`, i.e we
> won't scan the commit message in case conflict message exists, and we
> let the old code exist as it is?
> 
> +       if(ignored_log_message_bytes(sb->buf, sb->len))
> +               return;

So we'd ignore core.commentChar=auto if we detected conflict comments? 
That might be surprising to the user - it would mean that we'd always 
avoid adding the conflict comments to the commit message but we'd lose 
any lines that begin with the comment string. I think I'm leaning 
slightly towards the original solution but it is not clear to me that 
one option is much better that the other.

Thanks

Phillip