Repository: mystor/git-revise

Issue:
Feature request: Say which patch failed when editing a conflict
A useful feature of `git rebase -i` is that it says which commit failed to apply when there is a conflict:

    Could not apply badbeef... Commit bla bla

To me, this is essential information, because the editing is after all not of the end result, but some intermediate commit – you need to know which commit you're editing. Even if you can guess it, you don't want to take chances here, because of how easily this can make a tangled mess of the commits.

Git rebase also repeats the commit title inside the editable git conflict (whereas revise just puts "incoming" here). That would also suffice, though I really don't like the userfriendlyness of standard git conflicts. Thus why I suggest printing a message in plain English.

Perhaps the git conflict is the better place for this info, but then I would say the real problem is the readability of standard git conflicts: They look too much like a conflict between equals, and presents which two commits it came from, when what matters is which one commit you're editing. They really should just say `You are editing commit "Commit bla bla"`. If this is fixable, I would say you don't need the message, because then, a preview would hint even better to the user whether to edit or abort.

Discussion:
anordal: I changed the wording a little in the conflict file, but I wasn't so sure about it. What do you think?

* `incoming` → `this`: Short for "You are editing this commit".
* `new parent`/`old parent` → `new basis`/`old basis`: Parent obviously means parent commit in this case, but basis can stand on its own. It is the basis for the change.
mystor: > I changed the wording a little in the conflict file, but wasn't too sure about it. What do you think?
> 
>     * `incoming` → `this`: Short for "You are editing this commit".

"incoming" definitely wasn't the best name for it, but I'm not sure about "this" either. How about "current"?

>     * `new parent`/`old parent` → `new basis`/`old basis`: To say it without jargon, this is the "basis for the change".

I think I prefer the old wording. I find "basis" quite jargony as well, and "parent" is a more common word, also used by git. In my experience it hasn't been a source of confusion for people using git-revise.

Patch:
diff --git a/gitrevise/merge.py b/gitrevise/merge.py
index 56caeb4..c0171d0 100644
--- a/gitrevise/merge.py
+++ b/gitrevise/merge.py
@@ -31,7 +31,7 @@ def rebase(commit: Commit, parent: Commit) -> Commit:
 
     tree = merge_trees(
         Path("/"),
-        ("new parent", "old parent", "incoming"),
+        (parent.summary(), commit.parent().summary(), commit.summary()),
         parent.tree(),
         commit.parent().tree(),
         commit.tree(),
@@ -185,9 +185,9 @@ def merge_blobs(
             "merge-file",
             "-q",
             "-p",
-            f"-L{path} ({labels[0]})",
-            f"-L{path} ({labels[1]})",
-            f"-L{path} ({labels[2]})",
+            f"-L{path} (new parent): {labels[0]}",
+            f"-L{path} (old parent): {labels[1]}",
+            f"-L{path} (current): {labels[2]}",
             str(tmpdir / "current"),
             str(tmpdir / "base"),
             str(tmpdir / "other"),
@@ -201,7 +201,8 @@ def merge_blobs(
 
         # At this point, we know that there are merge conflicts to resolve.
         # Prompt to try and trigger manual resolution.
-        print(f"Merge conflict for '{path}'")
+        print(f"Conflict in applying '{labels[2]}'")
+        print(f"  Path: '{path}'")
         if input("  Edit conflicted file? (Y/n) ").lower() == "n":
             raise MergeConflict("user aborted")
 


Test patch:
diff --git a/tests/test_fixup.py b/tests/test_fixup.py
index 8730f22..406d311 100644
--- a/tests/test_fixup.py
+++ b/tests/test_fixup.py
@@ -115,12 +115,12 @@ def test_fixup_nonhead_conflict(basic_repo):
         with ed.next_file() as f:
             assert f.equals_dedent(
                 f"""\
-                <<<<<<< {os.sep}file1 (new parent)
+                <<<<<<< {os.sep}file1 (new parent): commit1
                 Hello, World!
                 How are things?
                 =======
                 conflict
-                >>>>>>> {os.sep}file1 (incoming)
+                >>>>>>> {os.sep}file1 (current): <git index>
                 """
             )
             f.replace_dedent("conflict1\n")
@@ -128,13 +128,13 @@ def test_fixup_nonhead_conflict(basic_repo):
         with ed.next_file() as f:
             assert f.equals_dedent(
                 f"""\
-                <<<<<<< {os.sep}file1 (new parent)
+                <<<<<<< {os.sep}file1 (new parent): commit1
                 conflict1
                 =======
                 Hello, World!
                 Oops, gotta add a new line!
                 How are things?
-                >>>>>>> {os.sep}file1 (incoming)
+                >>>>>>> {os.sep}file1 (current): commit2
                 """
             )
             f.replace_dedent("conflict2\n")