From: Junio C Hamano Date: Fri, 16 Sep 2022 15:55:45 GMT Subject: Re: [PATCH 6/6] revision: add parents after child for ^! Message-ID: In-Reply-To: <29d50baa-1923-38e1-6ecb-73840376d28e@web.de> René Scharfe writes: > "git diff A B", "git diff A..B", "git diff ^A B" and "git diff "B ^A" > all produce the same output. So I suspect we could reverse the order > for A..B as well without ill effect if we wanted. Yup. >> Can't we "fix" it at the consumer end, perhaps by checking where >> these commits came from by looking at rev.cmdline? > > We could. Yeah, doing so may be more in line with how two-tree comparison is parsed (your observation above on four combinations). > --- >8 --- > Subject: [PATCH] diff: support ^! for merges > > revision.c::handle_revision_arg_1() resolves ^! by first adding the > negated parents and then itself. builtin_diff_combined() expects > the first tree to be the merge and the remaining ones to be the parents, > though. This mismatch results in bogus diff output. > > Remember the first tree that doesn't belong to a parent and use it > instead of blindly picking the first one. This makes "git diff ^!" > consistent with "git show ^!". > > Reported-by: Tim Jaacks > Suggested-by: Junio C Hamano > Signed-off-by: René Scharfe > --- > The check "i < rev.cmdline.nr" is necessary to avoid segfaults e.g. > in t0005. I wonder why. Shouldn't rev.pending.objects and > rev.cmdline.rev always have the same number of entries? Things that did not come from command line can go into pending, can't they? E.g. add_head_to_pending(), when lack of rev defaults to "HEAD", touches pending without touching cmdline?