Re: [PATCH] "git diff <tree>{3,}": do not reverse order of arguments
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 11, 2008, 11:29 UTC
- Message-ID
- <7vwsgfjrp6.fsf@gitster.siamese.dyndns.org>
- In-Reply-To
- <1223690175.2828.26.camel@mattlaptop2.local>
Matt McCutchen <matt@mattmccutchen.net> writes:
Show 16 quoted lines
> diff --git a/builtin-diff.c b/builtin-diff.c > index 35da366..9c8c295 100644 > --- a/builtin-diff.c > +++ b/builtin-diff.c > @@ -177,10 +177,8 @@ static int builtin_diff_combined(struct rev_info *revs, > if (!revs->dense_combined_merges && !revs->combine_merges) > revs->dense_combined_merges = revs->combine_merges = 1; > parent = xmalloc(ents * sizeof(*parent)); > - /* Again, the revs are all reverse */ > for (i = 0; i < ents; i++) > - hashcpy((unsigned char *)(parent + i), > - ent[ents - 1 - i].item->sha1); > + hashcpy((unsigned char *)(parent + i), ent[i].item->sha1); > diff_tree_combined(parent[0], parent + 1, ents - 1, > revs->dense_combined_merges, revs); > return 0;
Interesting.
Somehow 0fe7c1d (built-in diff: assorted updates., 2006-04-29) thought that setup_revisions() give the ent[] list in reverse order. This was a thinko. I somehow thought that setup_revisions() queued the revs in reverse back then, and kept that misconception to this day, so I had to look it up in the history. It (meaning, the callchain from setup_revisions to add_object) never did.
Perhaps the thinko was caused by discrepancy between the way internal revision parser handles A..B and the way git-rev-parse parses it. While the internal revision parser parses it into "A ^B", rev-parse gives them in reverse order, i.e. "B ^A" (which is not going to change). In any case, thanks for spotting this.
Show 9 quoted lines
> diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh > index 1a6b522..fe6080d 100755 > --- a/t/t4013-diff-various.sh > +++ b/t/t4013-diff-various.sh > @@ -258,6 +258,7 @@ diff --patch-with-stat -r initial..side > diff --patch-with-raw -r initial..side > diff --name-status dir2 dir > diff --no-index --name-status dir2 dir > +diff master master^ side
As "diff master master^ side" would mean "the tip of master was made by merging side into it, show that tip of master like 'git show' does", I think this makes sense.