Re: [PATCH] range-diff: optionally include merge commits' diffs in the analysis

From: Elijah Newren <newren@gmail.com>
Date: Nov 8, 2024, 15:53 UTC
Message-ID: <CABPp-BFCe6wSQDGJ-Bo4+u1OE9Py2mYroX9qsvxFXJ_kePmTfg@mail.gmail.com>
In-Reply-To: <abc2453b-f7df-4601-8834-595881f9ceca@kdbg.org>

On Thu, Nov 7, 2024 at 10:54 PM Johannes Sixt <j6t@kdbg.org> wrote:

Show 14 quoted lines

>
> Am 07.11.24 um 18:20 schrieb Johannes Schindelin via GitGitGadget:
> > +--diff-merges=<format>::
> > +     Instead of ignoring merge commits, generate diffs for them using the
> > +     corresponding `--diff-merges=<format>` option of linkgit:git-log[1],
> > +     and include them in the comparison.
> > ++
> > +Note: Some of the formats supported by linkgit:git-log[1] make less sense in
> > +the context of the `range-diff` command than other formats, so choose wisely!
>
> Can we please be a bit more specific which options are usable or which
> are not usable? Perhaps you even mean to say that 'first-parent' is the
> only one that makes sense? At a minimum, we should mention that it is
> the one that makes most sense (if that is the case).

Here's my guesses I mostly typed up last night before getting tired and going to bed:

separate: makes no sense; showing two diffs for each merge and then trying to diff the two sets of two diffs seems crazy, especially since the whole point of rebasing is usually to change the first parent somewhere in the ancestry; that'll cascade up and cause the diffs to the second parents to differ wildly and make this format uselessly noisy in general.

combined: much better than separate, but probably still quite noisy and confusing, especially since combined-diff and range-diff are both complicated in that they have two diffs each, and combining the two means you have a diff of a dual diff. I suspect that might be hard for users to process.

dense-combined: much better than combined, you at least have the diff down to the smallest relevant pieces. You may still deal with the diff of a dual diff problem.

first-parent: useful, but only when keeping the series of patches largely intact and transplanting them; any insertion, deletion, or modification of intermediate patches will result in those modifications being shown not just for the patch in question but rolled up into the merge commit and shown there as well, making the diff for the merge very noisy (possibly to the point of uselessness). When keeping the series of patches intact and transplanting them, though, likely a useful mode.

remerge-diff: generally useful, even if you insert, delete, or modify intermediate patches while rebasing. Since it also shows how conflict resolutions are done, you'd see how conflicts are resolved differently. Actually, that might be one drawback to this method; since conflict markers try to provide labels involving a short commit ID and the commit message, and those short commit IDs will differ, this mode would just end up showing how the two merges had different parents, which you already know. We could probably make this option much more useful by surpressing the conflict labels, or at least the short commit ID parts of those labels. If we do that, and take Johannes' nice fix he sent to the list separately, then I suspect this mode is generally the most useful one to use with range-diff.