git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2] sequencer: avoid garbled merge machinery messages due to commit labels

From
Elijah Newren via GitGitGadget <gitgitgadget@gmail.com>
Date
Aug 12, 2020, 14:40 UTC
Message-ID
<pull.828.v2.git.git.1597243205137.gitgitgadget@gmail.com>
In-Reply-To
<pull.828.git.git.1596654345718.gitgitgadget@gmail.com>
From: Elijah Newren <newren@gmail.com>
sequencer's get_message() exists to provide good labels on conflict
hunks; see commits
  d68565402a ("revert: clarify label on conflict hunks", 2010-03-20)
  bf975d379d ("cherry-pick, revert: add a label for ancestor", 2010-03-20)
  043a4492b3 ("sequencer: factor code out of revert builtin", 2012-01-11).
for background on this function.  These labels are of the form
  <commitID>... <commit summary>
or
  parent of <commitID>... <commit summary>
These labels are then passed as branch names to the merge machinery.
However, these labels, as formatted, often also serve to confuse.  For
example, if we have a rename involved in a content merge, then it
results in text such as the following:
    <<<<<<<< HEAD:foo.c
      int j;
    ========
      int counter;
    >>>>>>>> b01dface... Removed unnecessary stuff:bar.c
Or in various conflict messages, it can make it very difficult to read:
    CONFLICT (rename/delete): foo.c deleted in b01dface... Removed
    unnecessary stuff and renamed in HEAD.  Version HEAD of foo.c left
    in tree.
    CONFLICT (file location): dir1/foo.c added in b01dface... Removed
    unnecessary stuff inside a directory that was renamed in HEAD,
    suggesting it should perhaps be moved to dir2/foo.c.

Make a minor change to remove the ellipses and add parentheses around the commit summary; this makes all three examples much easier to read:

    <<<<<<<< HEAD:foo.c
      int j;
    ========
      int counter;
    >>>>>>>> b01dface (Removed unnecessary stuff):bar.c
    CONFLICT (rename/delete): foo.c deleted in b01dface (Removed
    unnecessary stuff) and renamed in HEAD.  Version HEAD of foo.c left
    in tree.
    CONFLICT (file location): dir1/foo.c added in b01dface (Removed
    unnecessary stuff) inside a directory that was renamed in HEAD,
    suggesting it should perhaps be moved to dir2/foo.c.
Signed-off-by: Elijah Newren <newren@gmail.com>
Reviewed-by: Taylor Blau <me@ttaylorr.com>
Acked-by: Junio C Hamano <gitster@pobox.com>
---
    sequencer: avoid garbled merge machinery messages due to commit labels
    
    Changes since v1:
    
     * Added Taylor's Reviewed-by, and Junio's Acked-by (I assume that's a
       fair translation of "Looks good", anyway; feel free to adjust when
       you apply otherwise).
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-828%2Fnewren%2Fsequencer-merge-messages-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-828/newren/sequencer-merge-messages-v2
Pull-Request: https://github.com/git/git/pull/828
Range-diff vs v1:
 1:  da49e2eb58 ! 1:  762eb962cc sequencer: avoid garbled merge machinery messages due to commit labels
     @@ Commit message
              suggesting it should perhaps be moved to dir2/foo.c.
      
          Signed-off-by: Elijah Newren <newren@gmail.com>
     +    Reviewed-by: Taylor Blau <me@ttaylorr.com>
     +    Acked-by: Junio C Hamano <gitster@pobox.com>
      
       ## sequencer.c ##
      @@ sequencer.c: static int get_message(struct commit *commit, struct commit_message *out)
 sequencer.c                     |  2 +-
 t/t3404-rebase-interactive.sh   |  2 +-
 t/t3507-cherry-pick-conflict.sh | 20 ++++++++++----------
 3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index fd7701c88a..e988c12ad2 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -355,7 +355,7 @@ static int get_message(struct commit *commit, struct commit_message *out)
 	subject_len = find_commit_subject(out->message, &subject);
 
 	out->subject = xmemdupz(subject, subject_len);
-	out->label = xstrfmt("%s... %s", abbrev, out->subject);
+	out->label = xstrfmt("%s (%s)", abbrev, out->subject);
 	out->parent_label = xstrfmt("parent of %s", out->label);
 
 	return 0;
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 4a7d21f898..1d0a656ebd 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -256,7 +256,7 @@ test_expect_success 'stop on conflicting pick' '
 	D
 	=======
 	G
-	>>>>>>> $commit... G
+	>>>>>>> $commit (G)
 	EOF
 	git tag new-branch1 &&
 	test_must_fail git rebase -i master &&
diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
index 752bc43487..152ea11dc9 100755
--- a/t/t3507-cherry-pick-conflict.sh
+++ b/t/t3507-cherry-pick-conflict.sh
@@ -283,12 +283,12 @@ test_expect_success 'failed cherry-pick describes conflict in work tree' '
 	a
 	=======
 	c
-	>>>>>>> objid picked
+	>>>>>>> objid (picked)
 	EOF
 
 	test_must_fail git cherry-pick picked &&
 
-	sed "s/[a-f0-9]*\.\.\./objid/" foo >actual &&
+	sed "s/[a-f0-9]* (/objid (/" foo >actual &&
 	test_cmp expected actual
 '
 
@@ -298,16 +298,16 @@ test_expect_success 'diff3 -m style' '
 	cat <<-EOF >expected &&
 	<<<<<<< HEAD
 	a
-	||||||| parent of objid picked
+	||||||| parent of objid (picked)
 	b
 	=======
 	c
-	>>>>>>> objid picked
+	>>>>>>> objid (picked)
 	EOF
 
 	test_must_fail git cherry-pick picked &&
 
-	sed "s/[a-f0-9]*\.\.\./objid/" foo >actual &&
+	sed "s/[a-f0-9]* (/objid (/" foo >actual &&
 	test_cmp expected actual
 '
 
@@ -319,7 +319,7 @@ test_expect_success 'revert also handles conflicts sanely' '
 	a
 	=======
 	b
-	>>>>>>> parent of objid picked
+	>>>>>>> parent of objid (picked)
 	EOF
 	{
 		git checkout picked -- foo &&
@@ -345,7 +345,7 @@ test_expect_success 'revert also handles conflicts sanely' '
 	test_must_fail git update-index --refresh -q &&
 	test_must_fail git diff-index --exit-code HEAD &&
 	test_cmp expected-stages actual-stages &&
-	sed "s/[a-f0-9]*\.\.\./objid/" foo >actual &&
+	sed "s/[a-f0-9]* (/objid (/" foo >actual &&
 	test_cmp expected actual
 '
 
@@ -429,16 +429,16 @@ test_expect_success 'revert conflict, diff3 -m style' '
 	cat <<-EOF >expected &&
 	<<<<<<< HEAD
 	a
-	||||||| objid picked
+	||||||| objid (picked)
 	c
 	=======
 	b
-	>>>>>>> parent of objid picked
+	>>>>>>> parent of objid (picked)
 	EOF
 
 	test_must_fail git revert picked &&
 
-	sed "s/[a-f0-9]*\.\.\./objid/" foo >actual &&
+	sed "s/[a-f0-9]* (/objid (/" foo >actual &&
 	test_cmp expected actual
 '
 

base-commit: dc04167d378fb29d30e1647ff6ff51dd182bc9a3
-- 
gitgitgadget
Previous: Junio C HamanoNext: Johannes Schindelin
Message 4 of 5 in “sequencer: avoid garbled merge machinery messages due to commit labels”
  1. sequencer: avoid garbled merge machinery messages due to commit labelsElijah Newren via GitGitGadget, Aug 5, 2020
  2. Taylor BlauAug 5, 2020
  3. Junio C HamanoAug 5, 2020
  4. sequencer: avoid garbled merge machinery messages due to commit labelsElijah Newren via GitGitGadget, Aug 12, 2020
  5. Johannes SchindelinAug 14, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.