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

[Junio C Hamano <gitster@pobox.com>]

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.

> If you used an existing file (F1 or F2) like most of the rest of the
> tests in this file we could avoid creating this commit and save
> ourselves a couple of processes.

Excellent suggestion.

>> +	test_grep "^# Changes to be committed:$" actual
>
> I agree that it is a good idea to anchor the start of the message, but
> I'm not sure it is helpful to anchor the end of the message as we
> don't want the test to fail just because an unrelated change adds some
> whitespace to the end of this line. I'd be tempted to drop the ':' for
> the same reason.

Again, excellent.


> Thanks for fixing this
>
> Phillip

Thanks.