[PATCH] diff: remove dead code that flips arguments order
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 23, 2011, 21:36 UTC
- Message-ID
- <7voc51cydw.fsf@alter.siamese.dyndns.org>
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@pobox.com> ---
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; }