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

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

On Sat, Jun 28, 2025 at 7:08 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> On 27/06/2025 15:52, Junio C Hamano wrote:
> > Phillip Wood <phillip.wood123@gmail.com> writes:
> >
> >>> +   size_t cutoff;
> >>> +
> >>> +   /* Ignore comment chars in trailing comments (e.g., Conflicts:) */
> >>> +   cutoff = sb->len - ignored_log_message_bytes(sb->buf, sb->len);
> >>
> >> This finds the "Conflicts:" line. I was surprised to see that the
> >> string it looks for is hard coded and not translated, however the
> >> sequencer (also surprisingly) does not translate that message either
> >> so it should work.
> >
> > There is a funny chicken-and-egg problem, though.  It limits the
> > search for "Conflicts" by using wt_status_locate_end() based on the
> > current value of comment_line_str.  When core.commentstring is set
> > to "auto", the code that reads the configuration does not touch the
> > comment_line_str variable, which is initialized to '#'.  So
> >
> >       [core]
> >           commentstring = '%'
> >           commentstring = auto
> >
> > would have '%' in comment_line_str upon entering this codepath, let
> > wt_status_locate_end() use '%' as the comment string to find the end
> > of the log message, and then looks for "Conflicts:" in the result.
> >
> > Which may or may not be what you want.
>
> Oh, good point - I'd not looked at the config parsing. So we'd create
> conflict comments that look like
>
>      % Conflicts:
>      %    some-file.c
>
> but so long as the commit message did not contain a '#' character [1]
> "git commit" would select '#' as the automatic comment char and our
> conflicts lines would not be treated as a comment.
>
> Should we be resetting comment_line_str to '#' when core.commentString
> is set to "auto"? That wont help if the commit message contains a '#'
> but at least it would be consistently broken.
>
> We could move adjust_comment_line_char() into libgit.a, use that to
> select the comment character used in append_conflicts_hint() and set
> core.commentString to that character when we run "git commit". I think
> doing that would mean that appending conflict comments would always work
> with core.commentString=auto but it is a more complex solution as we
> would need to remember the comment character and then pass it to git
> commit once the user had fixed the conflicts.
>

So, my GSoC project is refactoring in order to reduce the global state
in Git. I was trying to remove the global variables related to comment
characters. What I tried is to create one single function which
returns the comment string, and we could then pass a strbuf in case of
core.commentString=auto. You can check my attempts on my fork here [2]
(check repo_get_comment_line_str() in config.c), and also mentioned
this in my blog [3]. I thought I had it figured out, but turns out I
failed one test where core.commentString=auto. It was that moment I
realised that I would need to remember the comment character or the
strbuf in functions. Just wanted to share this in case anything
strikes you when looking at the approach.

> One final note - although the commit message mentions a change to "git
> rebase" I think this problem already affected cherry-pick, revert and
> merge before that change. In practice I suspect it is only cherry-pick
> where one is likely to see this problem because the template messages
> for the other two commands are unlikely to contain a '#'.
>
> Thanks
>
> Phillip
>
> [1] For some reason adjust_comment_line_char() will not select '#' as
> the comment char if it occurs anywhere in the message but the other
> candidates are selected so long as they are not the first character on
> any line.

Thanks:)

[2] : https://github.com/ayu-ch/git/commits/comment-line-str-4
[3] : https://ayu-ch.github.io/2025/06/09/gsoc-week-1.html