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

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Jun 28, 2025, 13:38 UTC
Message-ID: <f39a3285-574a-45c6-9646-04eb175f4770@gmail.com>
In-Reply-To: <xmqqms9t8cfd.fsf@gitster.g>

On 27/06/2025 15:52, Junio C Hamano wrote:

Show 27 quoted lines

> 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.

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.