Re: git-name-rev off-by-one bug

From: Linus Torvalds <torvalds@osdl.org>
Date: 2005-11-30 16:06:10
On Tue, 29 Nov 2005, Linus Torvalds wrote:
> 
> Something like this (untested, of course).
> 
> It _should_ write out
> 
> 	* Unmerged path <filename>
> 
> followed by a regular diff, exactly like you'd want.

The more I thinking about this, the more I think this is a wonderful 
approach, but it would be even better to add a flag to let it choose 
between diffing against stage2 by default or diffing against stage3 (and 
hey, maybe even diffing against the original).

In fact, here's a patch that does that, and also makes the "resolve" merge 
create these kinds of merges. As usual, my python knowledge is useless, 
since the only thing I know about python is that thou shalt not count to 
four. As a result, the standard recursive merge doesn't do this yet ;(

The magic incantation is to just do

	git diff

and you'll get a diff against the first branch. If you want a diff against 
the second branch, just use the "-2" option, and if you want a diff 
against the common base (which is actually surprisingly useful, I noticed, 
when I tried this with a conflict), use "-0".

I've also changed "git diff" to _not_ drop the "-M" and "-p" options just 
because you give some other diff option. That was always a mistake. If you 
really want the raw git diff format, use the raw "git-diff-xyz" programs 
directly.

Whaddaya think? I really like it. Here's an example, where I merged two 
branches that had the file "hello" in it, and the first branch had:

	Hi there
	This is the master branch

and the second one had

	Hi there
	This is the 'other' branch

and the base version had just the "Hi there", of course.

The default (or "-1" arg) behaviour is:

	[torvalds@g5 test-merge]$ git diff
	* Unmerged path hello
	diff --git a/hello b/hello
	index 7cebcf8..3fa4697 100644
	--- a/hello
	+++ b/hello
	@@ -1,2 +1,6 @@
	 Hi there
	+<<<<<<< hello
	 This is the master branch
	+=======
	+This is the 'other' branch
	+>>>>>>> .merge_file_fJWiNf

which is obvious enough. You see exactly the conflict, and you see the 
part of the first branch that is unchanged.

Diffing against the original gives you

	[torvalds@g5 test-merge]$ git diff -0
	* Unmerged path hello
	diff --git a/hello b/hello
	index 6530b63..3fa4697 100644
	--- a/hello
	+++ b/hello
	@@ -1 +1,6 @@
	 Hi there
	+<<<<<<< hello
	+This is the master branch
	+=======
	+This is the 'other' branch
	+>>>>>>> .merge_file_fJWiNf

which I actually found really readable. I realize that this is a really 
stupid example, but for a lot of trivial merges, this won't be _that_ far 
off, and it basically shows what happened in both branches, and ignores 
what neither side changed.

The "-2" in this case is just the same as "-1" except obviously the "+" 
characters are situated differently. Still useful (especially if the 
changes were more complex).

Me likee. Hope you guys do too.

(And this is quite independently of the advantage that you can't commit an 
unmerged state by mistake, which is perhaps an even bigger one).

		Linus

---
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 6b496ed..afc7334 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -18,6 +18,12 @@
 	object name of pre- and post-image blob on the "index"
 	line when generating a patch format output.	
 
+-0 -1 -2::
+	When an unmerged entry is seen, diff against the base version,
+	the "first branch" or the "second branch" respectively.
+
+	The default is to diff against the first branch.
+
 -B::
 	Break complete rewrite changes into pairs of delete and create.
 
diff --git a/diff-files.c b/diff-files.c
index 38599b5..d744636 100644
--- a/diff-files.c
+++ b/diff-files.c
@@ -13,6 +13,7 @@ COMMON_DIFF_OPTIONS_HELP;
 
 static struct diff_options diff_options;
 static int silent = 0;
+static int diff_unmerged_stage = 2;
 
 static void show_unmerge(const char *path)
 {
@@ -46,7 +47,13 @@ int main(int argc, const char **argv)
 			argc--;
 			break;
 		}
-		if (!strcmp(argv[1], "-q"))
+		if (!strcmp(argv[1], "-0"))
+			diff_unmerged_stage = 1;
+		else if (!strcmp(argv[1], "-1"))
+			diff_unmerged_stage = 2;
+		else if (!strcmp(argv[1], "-2"))
+			diff_unmerged_stage = 3;
+		else if (!strcmp(argv[1], "-q"))
 			silent = 1;
 		else if (!strcmp(argv[1], "-r"))
 			; /* no-op */
@@ -95,11 +102,23 @@ int main(int argc, const char **argv)
 
 		if (ce_stage(ce)) {
 			show_unmerge(ce->name);
-			while (i < entries &&
-			       !strcmp(ce->name, active_cache[i]->name))
+			while (i < entries) {
+				struct cache_entry *nce = active_cache[i];
+
+				if (strcmp(ce->name, nce->name))
+					break;
+				/* Prefer to diff against the proper unmerged stage */
+				if (ce_stage(nce) == diff_unmerged_stage)
+					ce = nce;
 				i++;
-			i--; /* compensate for loop control increments */
-			continue;
+			}
+			/*
+			 * Compensate for loop update
+			 */
+			i--;
+			/*
+			 * Show the diff for the 'ce' we chose
+			 */
 		}
 
 		if (lstat(ce->name, &st) < 0) {
diff --git a/git-diff.sh b/git-diff.sh
index b3ec84b..efe8f75 100755
--- a/git-diff.sh
+++ b/git-diff.sh
@@ -3,12 +3,13 @@
 # Copyright (c) 2005 Linus Torvalds
 # Copyright (c) 2005 Junio C Hamano
 
+# Some way to turn these off?
+default_flags="-M -p"
+
 rev=$(git-rev-parse --revs-only --no-flags --sq "$@") || exit
-flags=$(git-rev-parse --no-revs --flags --sq "$@")
+flags=$(git-rev-parse --no-revs --flags --sq $default_flags "$@")
 files=$(git-rev-parse --no-revs --no-flags --sq "$@")
 
-: ${flags:="'-M' '-p'"}
-
 # I often say 'git diff --cached -p' and get scolded by git-diff-files, but
 # obviously I mean 'git diff --cached -p HEAD' in that case.
 case "$rev" in
diff --git a/git-merge-one-file.sh b/git-merge-one-file.sh
index c3eca8b..739a072 100755
--- a/git-merge-one-file.sh
+++ b/git-merge-one-file.sh
@@ -79,11 +79,7 @@ case "${1:-.}${2:-.}${3:-.}" in
 		;;
 	esac
 
-	# We reset the index to the first branch, making
-	# git-diff-file useful
-	git-update-index --add --cacheinfo "$6" "$2" "$4"
-		git-checkout-index -u -f -- "$4" &&
-		merge "$4" "$orig" "$src2"
+	merge "$4" "$orig" "$src2"
 	ret=$?
 	rm -f -- "$orig" "$src2"
 
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Received on Wed Nov 30 16:10:17 2005

This archive was generated by hypermail 2.1.8 : 2005-11-30 16:10:22 EST