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

From: Phillip Wood <[email-masked]>
Date: Sat, 28 Jun 2025 14:38:01 +0100

On 27/06/2025 15:52, Junio C Hamano wrote:
> Phillip Wood <[email-masked]> 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.

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.