Re: Merge commit diff results are confusing and inconsistent

[Elijah Newren <newren@gmail.com>]

On Tue, May 7, 2019 at 7:12 AM Robert Dailey <rcdailey.lists@gmail.com> wrote:
> On Mon, May 6, 2019 at 6:52 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

> The majority use case I'm interested in is seeing net-positive changes
> that happen in merge commits. Normally I take for granted that merge
> commits have nothing meaningful in them (meaningful here defined as
> something unexpected for a merge commit). But what if someone makes a
> poor decision and does some crazy refactoring in 1 file and amends it
> into a merge commit? Let's also say that these changes are done to a
> file that wasn't modified in any parent (say a unrelated.txt next to
> your color.txt). Since neither parent cares about that file for the
> purposes of the merge, I am trying to make sense of a revision
> specification that can be used to see what they did to that file.

An ability to find evil merges/cherry-picks/reverts by diffing a merge
commit to an auto-merge of some sort is something I am planning to
tackle; I'm tracking it at
https://bugs.chromium.org/p/git/issues/detail?id=12.  I'm working on
filter-repo first, then the merge rewrite, and then if I'm not
distracted by other stuff (a big if), then I'll tackle this.  Both
those other projects that are first in the list are rather large,
though, so it may be a while...

> Even ignoring that issue, the more concerning observation of mine is
> that `diff @^!` produces any output at all. If you exclude both
> parents, why do I see a diff for parent 2 (I see the complete diff of
> the branch that was merged in)?

I think using ^! on merge commits with diff ought to be an error,
personally.  Not that I'd want to get into the battle of figuring out
if someone did figure out what it means and find a valid use for it to
determine if we need to deprecate or provide alternate syntax or who
knows what for it.  I know it can be explained to be a combined diff
of some sort, but that's it.  ^! as a post-fix operator on a non-merge
commit makes sense to me and is useful, even though it's an ugly hack
and hideous usability-wise from a teaching or explaining perspective.
Without digging into the code in detail, I wouldn't for the life of me
be able to explain how ^! will work on merge commits with diff.

> Again, thank you for your example, you definitely made things very
> clear for me. I see where the confusion is. And I think --cc is a good
> way to get more context. At this point I'm just concerned about the
> @^! behavior with merge commits & diff.

I am too, though my suggestion for that case is: please, for all that
is good in the world, DON'T USE THAT.  EVER.  Whatever it might do, I
certainly don't want to waste any brain cycles supporting it or making
sure it still works.  diff is an endpoint operation and people should
pass endpoints to diff, not ranges (with only two exceptions I can
think of: '^!' on non-merge commits and usage of the super confusing
and ugly but also necessary '...' for diff against a merge-base.)

Honestly, I am tempted to make git throw a warning whenever folks use
a range operator with diff other than the exceptions noted above.
Hmm...

Elijah