Hi Philippe, On Sun, 10 Nov 2024, Philippe Blain wrote: > Le 2024-11-08 à 08:43, Johannes Schindelin via GitGitGadget a écrit : > > From: Johannes Schindelin <johannes.schindelin@xxxxxx> > > > > The `git log` command already offers support for including diffs for > > merges, via the `--diff-merges=<format>` option. > > > > Let's add corresponding support for `git range-diff`, too. This makes it > > more convenient to spot differences between iterations of non-linear > > contributions, where so-called "evil merges" are sometimes necessary and > > need to be reviewed, too. > > Maybe "between commit ranges that include merge commits" would be more > workflow-agnostic ? Good idea, this is much clearer than what I wrote, too. > > diff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt > > index fbdbe0befeb..17a85957877 100644 > > --- a/Documentation/git-range-diff.txt > > +++ b/Documentation/git-range-diff.txt > > @@ -81,6 +81,15 @@ to revert to color all lines according to the outer diff markers > > Suppress commits that are missing from the second specified range > > (or the "right range" when using the `<rev1>...<rev2>` format). > > > > +--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: In the common case, the `first-parent` mode will be the most natural one > > +to use, as it is consistent with the idea that a merge is kind of a "meta > > +patch", comprising all the merged commits' patches into a single one. > > I think I agree with Elijah that we probably should also highlight at least > 'remerge'. > > Also, is it worth making this a proper Asciidoc "[NOTE]" ? (I'm not sure, there are > a lot of "Notes:" in our doc that are not Asciidoc "[NOTE]"s. Right, I did not want to deviate too much from the surrounding style. Besides, I am not _so_ versed in AsciiDoc, I consider Markdown to be much more common, so I am much more familiar with that. I had no idea that there was a proper AsciiDoc [NOTE]. > > diff --git a/builtin/range-diff.c b/builtin/range-diff.c > > index 1b33ab66a7b..901de5d133d 100644 > > --- a/builtin/range-diff.c > > +++ b/builtin/range-diff.c > > The changes look good to me. Maybe it would be nice to add a corresponding > 'range-diff.diffMerges' config option to allow users to configure the > behaviour more permanently ? Seeing as there are no existing `rangeDiff.*` options, I am loathe to introduce the first one lest I am asked why I don't balloon this patch series into introducing config settings for the other options, too. Ciao, Johannes
