[PATCH v3 1/2] range-diff: optionally include merge commits' diffs in the analysis
- From
Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>
- Date
- Dec 16, 2024, 14:11 UTC
- Message-ID
- <ef3c243da1b3215000684a6b2d683dc6bb0017ac.1734358282.git.gitgitgadget@gmail.com>
- In-Reply-To
- <pull.1734.v3.git.1734358282.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
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> --- Documentation/git-range-diff.txt | 13 ++++++++++++- builtin/range-diff.c | 10 ++++++++++ range-diff.c | 17 ++++++++++++----- range-diff.h | 1 + t/t3206-range-diff.sh | 16 ++++++++++++++++ 5 files changed, 51 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt index 0b393715d70..150b0acbd86 100644 --- 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 aecfae12d3a..9d6236e5116 100644 --- a/builtin/range-diff.c +++ b/builtin/range-diff.c @@ -16,6 +16,7 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix) { 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, @@ -31,6 +32,8 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix) 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, @@ -57,6 +60,12 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix) 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; @@ -150,6 +159,7 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix) 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 4bd65ab7496..3bd7bff4735 100644 --- a/range-diff.c +++ b/range-diff.c @@ -31,7 +31,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; @@ -42,7 +43,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", /* @@ -57,6 +58,8 @@ static int read_patches(const char *range, struct string_list *list, "--pretty=medium", "--notes", 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); @@ -89,12 +92,15 @@ 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 (get_oid(p, &util->oid)) { + 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); string_list_clear(list, 1); @@ -559,13 +565,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 04ffe217be6..9753922fee9 100644 --- a/range-diff.h +++ b/range-diff.h @@ -10,6 +10,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 b5f4d6a6530..d3c4d2dbb49 100755 --- a/t/t3206-range-diff.sh +++ b/t/t3206-range-diff.sh @@ -866,4 +866,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
-- gitgitgadget