Re: [PATCH 6/6] revision: add parents after child for ^!

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 16, 2022, 15:55 UTC
Message-ID: <xmqqh717jou6.fsf@gitster.g>
In-Reply-To: <29d50baa-1923-38e1-6ecb-73840376d28e@web.de>

René Scharfe <l.s.r@web.de> 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).

Show 19 quoted lines

> --- >8 ---
> Subject: [PATCH] diff: support ^! for merges
>
> revision.c::handle_revision_arg_1() resolves <rev>^! by first adding the
> negated parents and then <rev> 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 <rev>^!"
> consistent with "git show <rev>^!".
>
> Reported-by: Tim Jaacks <tim.jaacks@garz-fricke.com>
> Suggested-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
> 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?