Re: [GSOC PATCH v3] commit: avoid scanning trailing comments when 'core.commentChar' is "auto"

[Ayush Chandekar <ayu.chandekar@gmail.com>]

Hey, Phillip

On Fri, Jul 4, 2025 at 1:53 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> 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
>

Now that we're planning to get rid of the 'auto' keyword from
commentChar [1], do you think it would be better if we just ignored
the keyword when we detect conflict comments? Also, how is it that a
user will end up having lines starting with the character being the
same as the conflict comment's character?

Thanks!
Ayush

[1]: https://lore.kernel.org/git/cover.1751983009.git.phillip.wood@dunelm.org.uk/