Subject: Re: [PATCH] merge & sequencer: turn "Conflicts:" hint into a comment

From: Junio C Hamano <[email-masked]>
Date: Mon, 27 Oct 2014 10:32:15 -0700

Jeff King <[email-masked]> writes:

> Just to play devil's advocate for a moment, though, are we hurting
> people who find it useful to record that information in the commit
> message?

I thought about it, but ultimately, they are using it wrong if they
did depend on them.  As you said, the paths themselves are not that
interesting, unless they are accompanied by description in the log
message explaining what caused conflicts and how they were resolved
at the semantic level.  The hints are to remind them what conflicts
in which paths to describe.

I do not mind "merge.commentConflictsHint = no" as a backward
compatibility toggle, if somebody cares deeply about it, though.

> If that is the only casualty, I think it is probably a net-win. We may
> want better tooling around viewing the merge later, but that can wait
> until somebody steps up with a real use case (because even that conflict
> list may not be completely what they want; they may also want the list
> of files that were auto-merged successfully, for example).

Yup.

Also Christian's "trailer" series may want to learn the same trick
we did to builtin/commit.c in this series, if it does not already
know about possible trailing comment and blank lines.

>> diff --git a/sequencer.c b/sequencer.c
>> index 0f84bbe..1d97da3 100644
>> --- a/sequencer.c
>> +++ b/sequencer.c
>> @@ -291,13 +291,12 @@ void append_conflicts_hint(struct strbuf *msgbuf)
>>  {
>>  	int i;
>>  
>> -	strbuf_addstr(msgbuf, "\nConflicts:\n");
>> +	strbuf_addch(msgbuf, '\n');
>> +	strbuf_commented_addf(msgbuf, "Conflicts:\n");
>>  	for (i = 0; i < active_nr;) {
>>  		const struct cache_entry *ce = active_cache[i++];
>>  		if (ce_stage(ce)) {
>> -			strbuf_addch(msgbuf, '\t');
>> -			strbuf_addstr(msgbuf, ce->name);
>> -			strbuf_addch(msgbuf, '\n');
>> +			strbuf_commented_addf(msgbuf, "\t%s\n", ce->name);
>
> This ends up adding a space followed by a tab. Besides being redundant,
> it makes my editor highlight it as a whitespace error. I realize this is
> a pretty minor nit, though.

Interesting ;-)

I do not think it is too hard to teach strbuf_commented_addf() about
the leading HT, but that would be a separate topic; if squashing the
SP-HT to HT is worth doing for this codepath, doing it at that helper
would benefit all callers.