Skip to content

Commit 08b296b

Browse files
committed
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>
1 parent b20cdc0 commit 08b296b

File tree

5 files changed

+50
-5
lines changed

5 files changed

+50
-5
lines changed

Documentation/git-range-diff.txt

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ SYNOPSIS
1010
[verse]
1111
'git range-diff' [--color=[<when>]] [--no-color] [<diff-options>]
1212
[--no-dual-color] [--creation-factor=<factor>]
13-
[--left-only | --right-only]
13+
[--left-only | --right-only] [--diff-merges=<format>]
1414
( <range1> <range2> | <rev1>...<rev2> | <base> <rev1> <rev2> )
1515
[[--] <path>...]
1616

@@ -81,6 +81,17 @@ to revert to color all lines according to the outer diff markers
8181
Suppress commits that are missing from the second specified range
8282
(or the "right range" when using the `<rev1>...<rev2>` format).
8383

84+
--diff-merges=<format>::
85+
Instead of ignoring merge commits, generate diffs for them using the
86+
corresponding `--diff-merges=<format>` option of linkgit:git-log[1],
87+
and include them in the comparison.
88+
+
89+
Note: In the common case, the `remerge` mode will be the most natural one
90+
to use, as it shows only the diff on top of what Git's merge machinery would
91+
have produced. In other words, if a merge commit is the result of a
92+
non-conflicting `git merge`, the `remerge` mode will represent it with an empty
93+
diff.
94+
8495
--[no-]notes[=<ref>]::
8596
This flag is passed to the `git log` program
8697
(see linkgit:git-log[1]) that generates the patches.

builtin/range-diff.c

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ int cmd_range_diff(int argc,
2121
{
2222
struct diff_options diffopt = { NULL };
2323
struct strvec other_arg = STRVEC_INIT;
24+
struct strvec diff_merges_arg = STRVEC_INIT;
2425
struct range_diff_options range_diff_opts = {
2526
.creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT,
2627
.diffopt = &diffopt,
@@ -36,6 +37,8 @@ int cmd_range_diff(int argc,
3637
OPT_PASSTHRU_ARGV(0, "notes", &other_arg,
3738
N_("notes"), N_("passed to 'git log'"),
3839
PARSE_OPT_OPTARG),
40+
OPT_PASSTHRU_ARGV(0, "diff-merges", &diff_merges_arg,
41+
N_("style"), N_("passed to 'git log'"), 0),
3942
OPT_BOOL(0, "left-only", &left_only,
4043
N_("only emit output related to the first range")),
4144
OPT_BOOL(0, "right-only", &right_only,
@@ -62,6 +65,12 @@ int cmd_range_diff(int argc,
6265
if (!simple_color)
6366
diffopt.use_color = 1;
6467

68+
/* If `--diff-merges` was specified, imply `--merges` */
69+
if (diff_merges_arg.nr) {
70+
range_diff_opts.include_merges = 1;
71+
strvec_pushv(&other_arg, diff_merges_arg.v);
72+
}
73+
6574
for (i = 0; i < argc; i++)
6675
if (!strcmp(argv[i], "--")) {
6776
dash_dash = i;
@@ -155,6 +164,7 @@ int cmd_range_diff(int argc,
155164
res = show_range_diff(range1.buf, range2.buf, &range_diff_opts);
156165

157166
strvec_clear(&other_arg);
167+
strvec_clear(&diff_merges_arg);
158168
strbuf_release(&range1);
159169
strbuf_release(&range2);
160170

range-diff.c

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@ struct patch_util {
3838
* as struct object_id (will need to be free()d).
3939
*/
4040
static int read_patches(const char *range, struct string_list *list,
41-
const struct strvec *other_arg)
41+
const struct strvec *other_arg,
42+
unsigned int include_merges)
4243
{
4344
struct child_process cp = CHILD_PROCESS_INIT;
4445
struct strbuf buf = STRBUF_INIT, contents = STRBUF_INIT;
@@ -49,7 +50,7 @@ static int read_patches(const char *range, struct string_list *list,
4950
size_t size;
5051
int ret = -1;
5152

52-
strvec_pushl(&cp.args, "log", "--no-color", "-p", "--no-merges",
53+
strvec_pushl(&cp.args, "log", "--no-color", "-p",
5354
"--reverse", "--date-order", "--decorate=no",
5455
"--no-prefix", "--submodule=short",
5556
/*
@@ -64,6 +65,8 @@ static int read_patches(const char *range, struct string_list *list,
6465
"--pretty=medium",
6566
"--show-notes-by-default",
6667
NULL);
68+
if (!include_merges)
69+
strvec_push(&cp.args, "--no-merges");
6770
strvec_push(&cp.args, range);
6871
if (other_arg)
6972
strvec_pushv(&cp.args, other_arg->v);
@@ -96,11 +99,14 @@ static int read_patches(const char *range, struct string_list *list,
9699
}
97100

98101
if (skip_prefix(line, "commit ", &p)) {
102+
char *q;
99103
if (util) {
100104
string_list_append(list, buf.buf)->util = util;
101105
strbuf_reset(&buf);
102106
}
103107
CALLOC_ARRAY(util, 1);
108+
if (include_merges && (q = strstr(p, " (from ")))
109+
*q = '\0';
104110
if (repo_get_oid(the_repository, p, &util->oid)) {
105111
error(_("could not parse commit '%s'"), p);
106112
FREE_AND_NULL(util);
@@ -571,13 +577,14 @@ int show_range_diff(const char *range1, const char *range2,
571577

572578
struct string_list branch1 = STRING_LIST_INIT_DUP;
573579
struct string_list branch2 = STRING_LIST_INIT_DUP;
580+
unsigned int include_merges = range_diff_opts->include_merges;
574581

575582
if (range_diff_opts->left_only && range_diff_opts->right_only)
576583
res = error(_("options '%s' and '%s' cannot be used together"), "--left-only", "--right-only");
577584

578-
if (!res && read_patches(range1, &branch1, range_diff_opts->other_arg))
585+
if (!res && read_patches(range1, &branch1, range_diff_opts->other_arg, include_merges))
579586
res = error(_("could not parse log for '%s'"), range1);
580-
if (!res && read_patches(range2, &branch2, range_diff_opts->other_arg))
587+
if (!res && read_patches(range2, &branch2, range_diff_opts->other_arg, include_merges))
581588
res = error(_("could not parse log for '%s'"), range2);
582589

583590
if (!res) {

range-diff.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ struct range_diff_options {
1616
int creation_factor;
1717
unsigned dual_color:1;
1818
unsigned left_only:1, right_only:1;
19+
unsigned include_merges:1;
1920
const struct diff_options *diffopt; /* may be NULL */
2021
const struct strvec *other_arg; /* may be NULL */
2122
};

t/t3206-range-diff.sh

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -909,4 +909,20 @@ test_expect_success 'submodule changes are shown irrespective of diff.submodule'
909909
test_cmp expect actual
910910
'
911911

912+
test_expect_success '--diff-merges' '
913+
renamed_oid=$(git rev-parse --short renamed-file) &&
914+
tree=$(git merge-tree unmodified renamed-file) &&
915+
clean=$(git commit-tree -m merge -p unmodified -p renamed-file $tree) &&
916+
clean_oid=$(git rev-parse --short $clean) &&
917+
conflict=$(git commit-tree -m merge -p unmodified -p renamed-file^ $tree) &&
918+
conflict_oid=$(git rev-parse --short $conflict) &&
919+
920+
git range-diff --diff-merges=1 $clean...$conflict >actual &&
921+
cat >expect <<-EOF &&
922+
1: $renamed_oid < -: ------- s/12/B/
923+
2: $clean_oid = 1: $conflict_oid merge
924+
EOF
925+
test_cmp expect actual
926+
'
927+
912928
test_done

0 commit comments

Comments
 (0)