Re: [GSOC PATCH v3] commit: avoid scanning trailing comments when 'core.commentChar' is "auto"
- From
Ayush Chandekar <ayu.chandekar@gmail.com>
- Date
- Jul 8, 2025, 15:47 UTC
- Message-ID
- <CAE7as+Yp9GWRohqe4oHHmYa1MfuKbyg9qKRf_z6N50bCSZ8vzQ@mail.gmail.com>
- In-Reply-To
- <062e7abd-97b1-4806-9753-338906642265@gmail.com>
Hey, Phillip
On Fri, Jul 4, 2025 at 1:53 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
Show 48 quoted lines
>
> 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/