Re: [RFC/PATCH] mergetool: clarify local/remote terminology

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 29, 2008, 05:47 UTC
Message-ID: <7v3arcfh54.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <20080228084328.GA18350@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> I like it; I think it made it a lot more obvious which side was which
> during the rebase. I checked with cherry-pick, as well; it more or less
> makes sense, except that the cherry-picked commit is called "upstream."

While I like the fact that somebody is trying to tackle the consistency issue, I do not like the approach itself. Fudging the issue at the merge-tool UI level may make things appear more consistent when viewing the merge from within merge-tool, but it makes the views merge-tool gives and vi/less gives inconsistent.

It would be a lot more sensible to make sure that we always show the side that the end-user modified first and then the side the other party changed.

A regular merge, cherry-pick and revert all start from user's side and play the change from the other side, and conflicted hunks are shown in the right order. It is only "rebase" (with or without -m) that gets it wrong. Even though it internally operates as if our own changes are coming from the other side, we should show our change first and then theirs to make it consistent and easier to read.

So how about doing it like this, instead? I am not proud of the use of an environment variable to pass this control around, but the idea should be obvious.

Note that some tests are written to expect the current hunk order and will start failing, so if we were to go this route, we would need to adjust them as well.

--

 git-rebase.sh |    3 +++
 ll-merge.c    |    6 ++++++
 2 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index bdcea0e..422879f 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -48,6 +48,9 @@ prec=4
 verbose=
 git_am_opt=
 
+GITHEAD_SWAP_CONFLICT=in-rebase
+export GITHEAD_SWAP_CONFLICT
+
 continue_merge () {
 	test -n "$prev_head" || die "prev_head must be defined"
 	test -d "$dotest" || die "$dotest directory does not exist"
diff --git a/ll-merge.c b/ll-merge.c
index 5ae7433..1d0040f 100644
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -76,6 +76,12 @@ static int ll_xdl_merge(const struct ll_merge_driver *drv_unused,
 				       virtual_ancestor);
 	}
 
+	if (getenv("GITHEAD_SWAP_CONFLICT")) {
+		mmfile_t *s;
+		const char *n;
+		s = src1; src1 = src2; src2 = s;
+		n = name1; name1 = name2; name2 = n;
+	}
 	memset(&xpp, 0, sizeof(xpp));
 	return xdl_merge(orig,
 			 src1, name1,