merge & sequencer: turn "Conflicts:" hint into a comment

Just like other hints such as "Changes to be committed" we show in the editor to remind the committer what paths were involved in the resulting commit to help improving their log message, this section is merely a reminder. Traditionally, it was not made into comments primarily because it has to be generated outside the wt-status infrastructure, and also because it was meant as a bit stronger reminder than the others (i.e. explaining how you resolved conflicts is much more important than mentioning what you did to every paths involved in the commit). But that still does not make this hint a part of the log message proper, and not showing it as a comment is inviting mistakes. Note that we still notice "Conflicts:" followed by list of indented pathnames as an old-style cruft and insert a new Signed-off-by: before it. This is so that "commit --amend -s" adds the new S-o-b at the right place when used on an older commit. Signed-off-by: Junio C Hamano <gitster@pobox.com>

Junio C Hamano committed Oct 28, 2014 at 13:04 UTC 261f315bebfa9af2d09f20211960100ff06f87cb
3 files changed +68 -28
builtin/commit.c
+31 -16
@@ -596,32 +596,47 @@ static char *cut_ident_timestamp_part(char *string)
596 /*
597 * Inspect sb and determine the true "end" of the log message, in
598 * order to find where to put a new Signed-off-by: line. Ignored are
599 - * trailing "Conflict:" block.
599 + * trailing comment lines and blank lines, and also the traditional
600 + * "Conflicts:" block that is not commented out, so that we can use
601 + * "git commit -s --amend" on an existing commit that forgot to remove
602 + * it.
603 *
604 * Returns the number of bytes from the tail to ignore, to be fed as
605 * the second parameter to append_signoff().
606 */
607 static int ignore_non_trailer(struct strbuf *sb)
608 {
606 - int ignore_footer = 0;
607 - int i, eol, previous = 0;
608 - const char *nl;
609 + int boc = 0;
610 + int bol = 0;
611 + int in_old_conflicts_block = 0;
612
610 - for (i = 0; i < sb->len; i++) {
611 - nl = memchr(sb->buf + i, '\n', sb->len - i);
612 - if (nl)
613 - eol = nl - sb->buf;
613 + while (bol < sb->len) {
614 + char *next_line;
615 +
616 + if (!(next_line = memchr(sb->buf + bol, '\n', sb->len - bol)))
617 + next_line = sb->buf + sb->len;
618 else
615 - eol = sb->len;
616 - if (!prefixcmp(sb->buf + previous, "\nConflicts:\n")) {
617 - ignore_footer = sb->len - previous;
618 - break;
619 + next_line++;
620 +
621 + if (sb->buf[bol] == comment_line_char || sb->buf[bol] == '\n') {
622 + /* is this the first of the run of comments? */
623 + if (!boc)
624 + boc = bol;
625 + /* otherwise, it is just continuing */
626 + } else if (!prefixcmp(sb->buf + bol, "Conflicts:\n")) {
627 + in_old_conflicts_block = 1;
628 + if (!boc)
629 + boc = bol;
630 + } else if (in_old_conflicts_block && sb->buf[bol] == '\t') {
631 + ; /* a pathname in the conflicts block */
632 + } else if (boc) {
633 + /* the previous was not trailing comment */
634 + boc = 0;
635 + in_old_conflicts_block = 0;
636 }
620 - while (i < eol)
621 - i++;
622 - previous = eol;
637 + bol = next_line - sb->buf;
638 }
624 - return ignore_footer;
639 + return boc ? sb->len - boc : 0;
640 }
641
642 static int prepare_to_commit(const char *index_file, const char *prefix,
sequencer.c
+3 -4
@@ -291,13 +291,12 @@ void append_conflicts_hint(struct strbuf *msgbuf)
291 {
292 int i;
293
294 - strbuf_addstr(msgbuf, "\nConflicts:\n");
294 + strbuf_addch(msgbuf, '\n');
295 + strbuf_commented_addf(msgbuf, "Conflicts:\n");
296 for (i = 0; i < active_nr;) {
297 const struct cache_entry *ce = active_cache[i++];
298 if (ce_stage(ce)) {
298 - strbuf_addch(msgbuf, '\t');
299 - strbuf_addstr(msgbuf, ce->name);
300 - strbuf_addch(msgbuf, '\n');
299 + strbuf_commented_addf(msgbuf, "\t%s\n", ce->name);
300 while (i < active_nr && !strcmp(ce->name,
301 active_cache[i]->name))
302 i++;
t/t3507-cherry-pick-conflict.sh
+34 -8
@@ -351,19 +351,45 @@ test_expect_success 'commit after failed cherry-pick does not add duplicated -s'
351 test_expect_success 'commit after failed cherry-pick adds -s at the right place' '
352 pristine_detach initial &&
353 test_must_fail git cherry-pick picked &&
354 +
355 git commit -a -s &&
355 - pwd &&
356 - cat <<EOF > expected &&
357 -picked
356
359 -Signed-off-by: C O Mitter <committer@example.com>
357 + # Do S-o-b and Conflicts appear in the right order?
358 + cat <<-\EOF >expect &&
359 + Signed-off-by: C O Mitter <committer@example.com>
360 + # Conflicts:
361 + EOF
362 + grep -e "^# Conflicts:" -e '^Signed-off-by' <.git/COMMIT_EDITMSG >actual &&
363 + test_cmp expect actual &&
364 +
365 + cat <<-\EOF >expected &&
366 + picked
367
361 -Conflicts:
362 - foo
363 -EOF
368 + Signed-off-by: C O Mitter <committer@example.com>
369 + EOF
370
365 - git show -s --pretty=format:%B > actual &&
371 + git show -s --pretty=format:%B >actual &&
372 test_cmp expected actual
373 '
374
375 +test_expect_success 'commit --amend -s places the sign-off at the right place' '
376 + pristine_detach initial &&
377 + test_must_fail git cherry-pick picked &&
378 +
379 + # emulate old-style conflicts block
380 + mv .git/MERGE_MSG .git/MERGE_MSG+ &&
381 + sed -e "/^# Conflicts:/,\$s/^# *//" <.git/MERGE_MSG+ >.git/MERGE_MSG &&
382 +
383 + git commit -a &&
384 + git commit --amend -s &&
385 +
386 + # Do S-o-b and Conflicts appear in the right order?
387 + cat <<-\EOF >expect &&
388 + Signed-off-by: C O Mitter <committer@example.com>
389 + Conflicts:
390 + EOF
391 + grep -e "^Conflicts:" -e '^Signed-off-by' <.git/COMMIT_EDITMSG >actual &&
392 + test_cmp expect actual
393 +'
394 +
395 test_done