[PATCH] diff: remove dead code that flips arguments order

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



In the olden days, "git cmd A..B" used to push ^A and then B into the list
of parsed revisions. The output from "git rev-list A..B" still does this,
and shows "B" first and then "^A" for backward compatibility.

The command line parser for "git diff A..B" was written originally to take
this swappage into account; when it internally sees an uninteresting
commit as its second input argument, it swapped the arguments to make it
the origin of the comparison, and the other one the destination.

These days, there is no need for this swappage, as the internal machinery
feeds ^A and then B to builtin_diff_tree().  Remove the dead code.

The funny thing is, I think this was a dead code from day one when it
was first written 6505602 (built-in diff., 2006-04-28); the author of
that patch must have known the implementation of the scripted version
that needed to deal with the swapped output from rev-parse too deeply
and didn't realize that the gotcha the dead code attempts to protect
him from didn't even apply to this codepath.

This change _will_ regress if somebody is insane enough to manually call
"git diff B ^A", but that is simply too insane for me to care.

There is exactly the same tree swapping code in diff-tree; that is not to
be changed not to break "git diff-tree B ^A" issued by Porcelain scripts,
but reword the comment to explain why we swap trees a bit better.

Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---

 builtin/diff-tree.c |    4 ++--
 builtin/diff.c      |   13 +------------
 2 files changed, 3 insertions(+), 14 deletions(-)

diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
index 0d2a3e9..c843546 100644
--- a/builtin/diff-tree.c
+++ b/builtin/diff-tree.c
@@ -133,8 +133,8 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
 	}
 
 	/*
-	 * NOTE! We expect "a ^b" to be equal to "a..b", so we
-	 * reverse the order of the objects if the second one
+	 * NOTE! We expect "b ^a" to have come from "rev-parse a..b",
+	 * so we reverse the order of the objects if the second one
 	 * is marked UNINTERESTING.
 	 */
 	nr_sha1 = opt->pending.nr;
diff --git a/builtin/diff.c b/builtin/diff.c
index 4c9deb2..86614d4 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -146,20 +146,9 @@ static int builtin_diff_tree(struct rev_info *revs,
 			     int argc, const char **argv,
 			     struct object_array_entry *ent)
 {
-	const unsigned char *(sha1[2]);
-	int swap = 0;
-
 	if (argc > 1)
 		usage(builtin_diff_usage);
-
-	/* We saw two trees, ent[0] and ent[1].
-	 * if ent[1] is uninteresting, they are swapped
-	 */
-	if (ent[1].item->flags & UNINTERESTING)
-		swap = 1;
-	sha1[swap] = ent[0].item->sha1;
-	sha1[1-swap] = ent[1].item->sha1;
-	diff_tree_sha1(sha1[0], sha1[1], "", &revs->diffopt);
+	diff_tree_sha1(ent[0].item->sha1, ent[1].item->sha1, "", &revs->diffopt);
 	log_tree_diff_flush(revs);
 	return 0;
 }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]