Skip to content

Commit

Permalink
range-diff: optionally include merge commits' diffs in the analysis
Browse files Browse the repository at this point in the history
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.

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

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
  • Loading branch information
dscho committed Nov 7, 2024
1 parent 777489f commit 8baf3fa
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 5 deletions.
10 changes: 9 additions & 1 deletion Documentation/git-range-diff.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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>...]

Expand Down Expand Up @@ -81,6 +81,14 @@ 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: 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!

--[no-]notes[=<ref>]::
This flag is passed to the `git log` program
(see linkgit:git-log[1]) that generates the patches.
Expand Down
11 changes: 11 additions & 0 deletions builtin/range-diff.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -36,6 +37,9 @@ 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'"),
PARSE_OPT_OPTARG),
OPT_BOOL(0, "left-only", &left_only,
N_("only emit output related to the first range")),
OPT_BOOL(0, "right-only", &right_only,
Expand All @@ -62,6 +66,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;
Expand Down Expand Up @@ -155,6 +165,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);

Expand Down
15 changes: 11 additions & 4 deletions range-diff.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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",
/*
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions range-diff.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
};
Expand Down
15 changes: 15 additions & 0 deletions t/t3206-range-diff.sh
Original file line number Diff line number Diff line change
Expand Up @@ -909,4 +909,19 @@ test_expect_success 'submodule changes are shown irrespective of diff.submodule'
test_cmp expect actual
'

test_expect_success '--diff-merges' '
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: 1e6226b < -: ------- s/12/B/
2: $clean_oid = 1: $conflict_oid merge
EOF
test_cmp expect actual
'

test_done

0 comments on commit 8baf3fa

Please sign in to comment.