range-diff: optionally include merge commits' diffs in the analysis

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 commit ranges that contain
merges.

This is especially true in scenarios with non-trivial merges, i.e.
merges introducing changes other than, or in addition to, what merge ORT
would have produced. Merging a topic branch that changes a function
signature into a branch that added a caller of that function, for
example, would require the merge commit itself to adjust that caller to
the modified signature.

In my code reviews, I found the `--diff-merges=remerge` option
particularly useful.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt
index fbdbe0befebab63b0316a29aaf420f9538cce43c..00c649b140e516f6eb67c65b532308ff8af372e3 100644 (file)
--- a/Documentation/git-range-diff.txt
+++ b/Documentation/git-range-diff.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 [verse]
 'git range-diff' [--color=[<when>]] [--no-color] [<diff-options>]
        [--no-dual-color] [--creation-factor=<factor>]
-       [--left-only | --right-only]
+       [--left-only | --right-only] [--diff-merges=<format>]
        ( <range1> <range2> | <rev1>...<rev2> | <base> <rev1> <rev2> )
        [[--] <path>...]
 
@@ -81,6 +81,17 @@ 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 `remerge` mode will be the most natural one
+to use, as it shows only the diff on top of what Git's merge machinery would
+have produced. In other words, if a merge commit is the result of a
+non-conflicting `git merge`, the `remerge` mode will represent it with an empty
+diff.
+
 --[no-]notes[=<ref>]::
        This flag is passed to the `git log` program
        (see linkgit:git-log[1]) that generates the patches.

diff --git a/builtin/range-diff.c b/builtin/range-diff.c
index 1b33ab66a7b2013224f1c8f29854433d76776a0e..901de5d133dac8fcae2561d1bf48240ff42c8ed5 100644 (file)
--- a/builtin/range-diff.c
+++ b/builtin/range-diff.c
@@ -21,6 +21,7 @@ int cmd_range_diff(int argc,
 {
        struct diff_options diffopt = { NULL };
        struct strvec other_arg = STRVEC_INIT;
+       struct strvec diff_merges_arg = STRVEC_INIT;
        struct range_diff_options range_diff_opts = {
                .creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT,
                .diffopt = &diffopt,
@@ -36,6 +37,8 @@ int cmd_range_diff(int argc,
                OPT_PASSTHRU_ARGV(0, "notes", &other_arg,
                                  N_("notes"), N_("passed to 'git log'"),
                                  PARSE_OPT_OPTARG),
+               OPT_PASSTHRU_ARGV(0, "diff-merges", &diff_merges_arg,
+                                 N_("style"), N_("passed to 'git log'"), 0),
                OPT_BOOL(0, "left-only", &left_only,
                         N_("only emit output related to the first range")),
                OPT_BOOL(0, "right-only", &right_only,
@@ -62,6 +65,12 @@ int cmd_range_diff(int argc,
        if (!simple_color)
                diffopt.use_color = 1;
 
+       /* If `--diff-merges` was specified, imply `--merges` */
+       if (diff_merges_arg.nr) {
+               range_diff_opts.include_merges = 1;
+               strvec_pushv(&other_arg, diff_merges_arg.v);
+       }
+
        for (i = 0; i < argc; i++)
                if (!strcmp(argv[i], "--")) {
                        dash_dash = i;
@@ -155,6 +164,7 @@ int cmd_range_diff(int argc,
        res = show_range_diff(range1.buf, range2.buf, &range_diff_opts);
 
        strvec_clear(&other_arg);
+       strvec_clear(&diff_merges_arg);
        strbuf_release(&range1);
        strbuf_release(&range2);
 

diff --git a/range-diff.c b/range-diff.c
index 10885ba301389754869b7cdc73f7596e6cb069ea..19673d47d9394ffebbf1fa1b45059a3d7703d63e 100644 (file)
--- a/range-diff.c
+++ b/range-diff.c
@@ -38,7 +38,8 @@ struct patch_util {
  * as struct object_id (will need to be free()d).
  */
 static int read_patches(const char *range, struct string_list *list,
-                       const struct strvec *other_arg)
+                       const struct strvec *other_arg,
+                       unsigned int include_merges)
 {
        struct child_process cp = CHILD_PROCESS_INIT;
        struct strbuf buf = STRBUF_INIT, contents = STRBUF_INIT;
@@ -49,7 +50,7 @@ static int read_patches(const char *range, struct string_list *list,
        size_t size;
        int ret = -1;
 
-       strvec_pushl(&cp.args, "log", "--no-color", "-p", "--no-merges",
+       strvec_pushl(&cp.args, "log", "--no-color", "-p",
                     "--reverse", "--date-order", "--decorate=no",
                     "--no-prefix", "--submodule=short",
                     /*
@@ -64,6 +65,8 @@ static int read_patches(const char *range, struct string_list *list,
                     "--pretty=medium",
                     "--show-notes-by-default",
                     NULL);
+       if (!include_merges)
+               strvec_push(&cp.args, "--no-merges");
        strvec_push(&cp.args, range);
        if (other_arg)
                strvec_pushv(&cp.args, other_arg->v);
@@ -96,11 +99,14 @@ static int read_patches(const char *range, struct string_list *list,
                }
 
                if (skip_prefix(line, "commit ", &p)) {
+                       char *q;
                        if (util) {
                                string_list_append(list, buf.buf)->util = util;
                                strbuf_reset(&buf);
                        }
                        CALLOC_ARRAY(util, 1);
+                       if (include_merges && (q = strstr(p, " (from ")))
+                               *q = '\0';
                        if (repo_get_oid(the_repository, p, &util->oid)) {
                                error(_("could not parse commit '%s'"), p);
                                FREE_AND_NULL(util);
@@ -571,13 +577,14 @@ int show_range_diff(const char *range1, const char *range2,
 
        struct string_list branch1 = STRING_LIST_INIT_DUP;
        struct string_list branch2 = STRING_LIST_INIT_DUP;
+       unsigned int include_merges = range_diff_opts->include_merges;
 
        if (range_diff_opts->left_only && range_diff_opts->right_only)
                res = error(_("options '%s' and '%s' cannot be used together"), "--left-only", "--right-only");
 
-       if (!res && read_patches(range1, &branch1, range_diff_opts->other_arg))
+       if (!res && read_patches(range1, &branch1, range_diff_opts->other_arg, include_merges))
                res = error(_("could not parse log for '%s'"), range1);
-       if (!res && read_patches(range2, &branch2, range_diff_opts->other_arg))
+       if (!res && read_patches(range2, &branch2, range_diff_opts->other_arg, include_merges))
                res = error(_("could not parse log for '%s'"), range2);
 
        if (!res) {

diff --git a/range-diff.h b/range-diff.h
index 2f69f6a434d7d05877ebf8d181475cb6848aee28..cd85000b5a0da023f81b0c9388c4d25815e2faa6 100644 (file)
--- a/range-diff.h
+++ b/range-diff.h
@@ -16,6 +16,7 @@ struct range_diff_options {
        int creation_factor;
        unsigned dual_color:1;
        unsigned left_only:1, right_only:1;
+       unsigned include_merges:1;
        const struct diff_options *diffopt; /* may be NULL */
        const struct strvec *other_arg; /* may be NULL */
 };

diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
index 86010931ab6243e0e617b14cae0eb63975edee7d..c18a3fdab83e6a5f772a39ff5d319a90e44aa871 100755 (executable)
--- a/t/t3206-range-diff.sh
+++ b/t/t3206-range-diff.sh
@@ -909,4 +909,20 @@ test_expect_success 'submodule changes are shown irrespective of diff.submodule'
        test_cmp expect actual
 '
 
+test_expect_success '--diff-merges' '
+       renamed_oid=$(git rev-parse --short renamed-file) &&
+       tree=$(git merge-tree unmodified renamed-file) &&
+       clean=$(git commit-tree -m merge -p unmodified -p renamed-file $tree) &&
+       clean_oid=$(git rev-parse --short $clean) &&
+       conflict=$(git commit-tree -m merge -p unmodified -p renamed-file^ $tree) &&
+       conflict_oid=$(git rev-parse --short $conflict) &&
+
+       git range-diff --diff-merges=1 $clean...$conflict >actual &&
+       cat >expect <<-EOF &&
+       1:  $renamed_oid < -:  ------- s/12/B/
+       2:  $clean_oid = 1:  $conflict_oid merge
+       EOF
+       test_cmp expect actual
+'
+
 test_done