Harald Nordgren via GitGitGadgetSep 11, 2026, 16:41 UTC on loreFrom: Harald Nordgren <haraldnordgren@gmail.com>
Reviewing a range-diff often means scrolling past commits that were simply added or dropped, when only the ones that correspond between the two ranges are of interest.
--left-only and --right-only already each suppress one of those one-sided groups, so give --matched-only its own name for applying both suppressions at once instead of documenting the combination of two options whose names read as contradictory together. Internally it just sets both flags, reusing the existing suppression logic in show_range_diff().
Extend the existing '--left-only'/'--right-only' conflict check in show_range_diff() to also reject any combination with --matched-only, since all three narrow the output in ways that cannot be combined.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
range-diff: add --matched-only to skip one-sided commits
Add git range-diff --matched-only to only show commits that correspond
between the two ranges, skipping ones that were only added or only
removed.Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2401%2FHaraldNordgren%2Frange-diff-matched-only-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2401/HaraldNordgren/range-diff-matched-only-v1
Pull-Request: https://github.com/git/git/pull/2401
Documentation/git-range-diff.adoc | 10 ++++-
builtin/range-diff.c | 5 ++-
range-diff.c | 11 +++++-
range-diff.h | 2 +-
t/t3206-range-diff.sh | 63 +++++++++++++++++++++++++++++++
5 files changed, 86 insertions(+), 5 deletions(-)
Show changes to 5 files +86 −5
Documentation/git-range-diff.adoc, builtin/range-diff.c, range-diff.c, range-diff.h, t/t3206-range-diff.sh
diff --git a/Documentation/git-range-diff.adoc b/Documentation/git-range-diff.adoc
index 5cc5e2ed56..58e59e8e3b 100644
--- a/Documentation/git-range-diff.adoc
+++ b/Documentation/git-range-diff.adoc
@@ -10,7 +10,8 @@ SYNOPSIS
[synopsis]
git range-diff [--color=[<when>]] [--no-color] [<diff-options>]
[--no-dual-color] [--creation-factor=<factor>]
- [--left-only | --right-only] [--diff-merges=<format>]
+ [--left-only | --right-only | --matched-only]
+ [--diff-merges=<format>]
[--remerge-diff] [--no-notes | --notes[=<ref>]]
( <range1> <range2> | <rev1>...<rev2> | <base> <rev1> <rev2> )
[[--] <path>...]
@@ -82,6 +83,13 @@ 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>` form).
+`--matched-only`::
+ Only emit commits that have a corresponding commit in the other
+ range, suppressing any commit that exists on only one side. This is
+ the same as using `--left-only` and `--right-only` together. Useful
+ to skip added or removed commits when reviewing how the commits
+ that survived a rebase changed.
+
`--diff-merges=<format>`::
Instead of ignoring merge commits, generate diffs for them using the
corresponding `--diff-merges=<format>` option of linkgit:git-log[1],
diff --git a/builtin/range-diff.c b/builtin/range-diff.c
index e54c0f7fe1..8059f92eaa 100644
--- a/builtin/range-diff.c
+++ b/builtin/range-diff.c
@@ -46,7 +46,7 @@ int cmd_range_diff(int argc,
.diffopt = &diffopt,
.log_arg = &log_arg
};
- int simple_color = -1, left_only = 0, right_only = 0;
+ int simple_color = -1, left_only = 0, right_only = 0, matched_only = 0;
struct option range_diff_options[] = {
OPT_INTEGER(0, "creation-factor",
&range_diff_opts.creation_factor,
@@ -68,6 +68,8 @@ int cmd_range_diff(int argc,
N_("only emit output related to the first range")),
OPT_BOOL(0, "right-only", &right_only,
N_("only emit output related to the second range")),
+ OPT_BOOL(0, "matched-only", &matched_only,
+ N_("only emit commits that have a corresponding commit in the other range")),
OPT_END()
};
struct option *options;
@@ -186,6 +188,7 @@ int cmd_range_diff(int argc,
range_diff_opts.dual_color = simple_color < 1;
range_diff_opts.left_only = left_only;
range_diff_opts.right_only = right_only;
+ range_diff_opts.matched_only = matched_only;
res = show_range_diff(range1.buf, range2.buf, &range_diff_opts);
strvec_clear(&log_arg);
diff --git a/range-diff.c b/range-diff.c
index 8e2dd2eb19..fa895f5760 100644
--- a/range-diff.c
+++ b/range-diff.c
@@ -591,8 +591,15 @@ int show_range_diff(const char *range1, const char *range2,
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 (range_diff_opts->left_only + range_diff_opts->right_only +
+ range_diff_opts->matched_only > 1)
+ res = error(_("options '%s', '%s', or '%s' cannot be used together"),
+ "--left-only", "--right-only", "--matched-only");
+
+ if (range_diff_opts->matched_only) {
+ range_diff_opts->left_only = 1;
+ range_diff_opts->right_only = 1;
+ }
if (!res && read_patches(range1, &branch1, range_diff_opts->log_arg, include_merges))
res = error(_("could not parse log for '%s'"), range1);
diff --git a/range-diff.h b/range-diff.h
index 9b70a80009..effd10b9b8 100644
--- a/range-diff.h
+++ b/range-diff.h
@@ -19,7 +19,7 @@
struct range_diff_options {
int creation_factor;
unsigned dual_color:1;
- unsigned left_only:1, right_only:1;
+ unsigned left_only:1, right_only:1, matched_only:1;
unsigned include_merges:1;
size_t max_memory;
const struct diff_options *diffopt; /* may be NULL */
diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
index ef92704de3..f85fd0c4ad 100755
--- a/t/t3206-range-diff.sh
+++ b/t/t3206-range-diff.sh
@@ -860,6 +860,69 @@ test_expect_success '--left-only/--right-only' '
test_cmp expect actual
'
+test_expect_success '--left-only, --right-only and --matched-only are incompatible' '
+ test_must_fail git range-diff --left-only --right-only ...common 2>err &&
+ test_grep "cannot be used together" err &&
+
+ test_must_fail git range-diff --left-only --matched-only ...common 2>err &&
+ test_grep "cannot be used together" err &&
+
+ test_must_fail git range-diff --right-only --matched-only ...common 2>err &&
+ test_grep "cannot be used together" err &&
+
+ test_must_fail git range-diff --left-only --right-only --matched-only \
+ ...common 2>err &&
+ test_grep "cannot be used together" err
+'
+
+test_expect_success '--left-only, --right-only and --matched-only each suppress one-sided commits' '
+ test_create_repo matched-only &&
+ (
+ cd matched-only &&
+ git switch --orphan combined-old &&
+ test_commit c-first &&
+ test_commit c-old-only &&
+ test_commit c-common &&
+ git switch -C combined-new c-first &&
+ test_commit c-new-only &&
+ git cherry-pick c-common &&
+
+ old_only_oid=$(git rev-parse --short=7 c-old-only) &&
+ new_only_oid=$(git rev-parse --short=7 c-new-only) &&
+ common_old_oid=$(git rev-parse --short=7 c-common) &&
+ common_new_oid=$(git rev-parse --short=7 HEAD) &&
+
+ git range-diff -s --abbrev=7 combined-old...combined-new >actual &&
+ cat >expect <<-EOF &&
+ 1: $old_only_oid < -: ------- c-old-only
+ -: ------- > 1: $new_only_oid c-new-only
+ 2: $common_old_oid = 2: $common_new_oid c-common
+ EOF
+ test_cmp expect actual &&
+
+ git range-diff -s --abbrev=7 --left-only combined-old...combined-new \
+ >actual &&
+ cat >expect <<-EOF &&
+ 1: $old_only_oid < -: ------- c-old-only
+ 2: $common_old_oid = 2: $common_new_oid c-common
+ EOF
+ test_cmp expect actual &&
+
+ git range-diff -s --abbrev=7 --right-only combined-old...combined-new \
+ >actual &&
+ cat >expect <<-EOF &&
+ -: ------- > 1: $new_only_oid c-new-only
+ 2: $common_old_oid = 2: $common_new_oid c-common
+ EOF
+ test_cmp expect actual &&
+
+ git range-diff -s --abbrev=7 --matched-only combined-old...combined-new \
+ >actual &&
+ echo "2: $common_old_oid = 2: $common_new_oid c-common" >expect &&
+ test_cmp expect actual
+ )
+'
+
test_expect_success 'ranges with pathspecs' '
git range-diff topic...mode-only-change -- other-file >actual &&
test_line_count = 2 actual &&
base-commit: fa7f9290efe2bd22dd736689597b474b93798e11
--
gitgitgadget
Re: [PATCH] range-diff: add --matched-only to skip one-sided commits
"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 24 quoted lines
> diff --git a/Documentation/git-range-diff.adoc b/Documentation/git-range-diff.adoc
> index 5cc5e2ed56..58e59e8e3b 100644
> --- a/Documentation/git-range-diff.adoc
> +++ b/Documentation/git-range-diff.adoc
> @@ -10,7 +10,8 @@ SYNOPSIS
> [synopsis]
> git range-diff [--color=[<when>]] [--no-color] [<diff-options>]
> [--no-dual-color] [--creation-factor=<factor>]
> - [--left-only | --right-only] [--diff-merges=<format>]
> + [--left-only | --right-only | --matched-only]
> + [--diff-merges=<format>]
> [--remerge-diff] [--no-notes | --notes[=<ref>]]
> ( <range1> <range2> | <rev1>...<rev2> | <base> <rev1> <rev2> )
> [[--] <path>...]
> @@ -82,6 +83,13 @@ 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>` form).
>
> +`--matched-only`::
> + Only emit commits that have a corresponding commit in the other
> + range, suppressing any commit that exists on only one side. This is
> + the same as using `--left-only` and `--right-only` together. Useful
> + to skip added or removed commits when reviewing how the commits
> + that survived a rebase changed.
While conceptually it is the same as giving "--hide-right-only" (which would have hidden the right-only entry) and "--hide-left-only" at the same time, because the existing two options are not defined in terms of "hiding" entries that have only one side (which would have logically allowed combining) but instead showing "only" one side (which makes it impossible to give them together, and indeed that is the first thing range-diff.c:show_range_diff() checks and yields an error), this description is not accurate.
I wonder if the implementation actually can be more like
- give "--hide-left-only" and "--hide-right-only" as synonyms to
"--right-only" and "--left-only", and deprecate the original;
- allow them to be given together, which will give the new
behaviour you are introducing, i.e., skip steps without both
sides from the output;
- give a short-hand synonym, "--matched-only", to truly behave the
same as giving "--hide-{left,right}-only" together.which would allow the above explanation to be more accurate? I dunno.
> + if (range_diff_opts->left_only + range_diff_opts->right_only +
> + range_diff_opts->matched_only > 1)
> + res = error(_("options '%s', '%s', or '%s' cannot be used together"),
> + "--left-only", "--right-only", "--matched-only");Don't we have die_for_incompatible_opt3() to do this?
The basic idea sounds good. The unmatched entries do serve as a strong hint that a greater --creation-factor may help. For example,
Show 7 quoted lines
> + git range-diff -s --abbrev=7 combined-old...combined-new >actual &&
> + cat >expect <<-EOF &&
> + 1: $old_only_oid < -: ------- c-old-only
> + -: ------- > 1: $new_only_oid c-new-only
> + 2: $common_old_oid = 2: $common_new_oid c-common
> + EOF
> + test_cmp expect actual &&
the above clearly shows that the command might compare c-old-only and c-new-only with a better creation factor settings.
But because the entries are numbered, gaps in the numbers, like this output
> + git range-diff -s --abbrev=7 --matched-only combined-old...combined-new \
> + >actual &&
> + echo "2: $common_old_oid = 2: $common_new_oid c-common" >expect &&
> + test_cmp expect actual
may be sufficient (we can tell that 1 was omitted), except that somehow we at least need to be aware that there were only 2 commits on both sides (it may be hiding commits 3 thru 99 as unmatching pairs and we lose that hint from the new output), which is not a huge downside.
Thanks.
Re: [PATCH] range-diff: add --matched-only to skip one-sided commits
Show 11 quoted lines
> I wonder if the implementation actually can be more like
>
> - give "--hide-left-only" and "--hide-right-only" as synonyms to
> "--right-only" and "--left-only", and deprecate the original;
>
> - allow them to be given together, which will give the new
> behaviour you are introducing, i.e., skip steps without both
> sides from the output;
>
> - give a short-hand synonym, "--matched-only", to truly behave the
> same as giving "--hide-{left,right}-only" together.Seems like a big change, and deprecated options are a pain in the neck because we can never actually remove them.
If we decide to go this way, we might name them "--hide-{left,right}" and just not introduce a condition that makes them incompatible. Then "--matched-only" would be pure syntactic sugar and wouldn't even be 100% necessary to have to achieve this.
Harald
[PATCH v2] range-diff: add --matched-only to skip one-sided commits
From: Harald Nordgren <haraldnordgren@gmail.com>
Reviewing a range-diff often means scrolling past commits that were simply added or dropped, when only the ones that correspond between the two ranges are of interest.
--left-only and --right-only already each suppress one of those one-sided groups, but they are defined as "only show this side" and so cannot be given together, which is exactly why show_range_diff() already rejected that combination. Give the "show only the commits that correspond on both sides" behavior its own name, --matched-only, instead of asking users to reach for a combination that errors out.
Extend the existing '--left-only'/'--right-only' conflict check to also reject any combination with --matched-only, since all three narrow the output in ways that cannot be combined.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
range-diff: add --matched-only to skip one-sided commits
Add git range-diff --matched-only to only show commits that correspond
between the two ranges, skipping ones that were only added or only
removed.
Changes in v2:
* Update docs and commit message.
* Use die_for_incompatible_opt3.Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2401%2FHaraldNordgren%2Frange-diff-matched-only-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2401/HaraldNordgren/range-diff-matched-only-v2
Pull-Request: https://github.com/git/git/pull/2401
Range-diff vs v1:
1: edb4471088 ! 1: 6d392249a2 range-diff: add --matched-only to skip one-sided commits
@@ Commit message
the two ranges are of interest.
--left-only and --right-only already each suppress one of those
- one-sided groups, so give --matched-only its own name for applying
- both suppressions at once instead of documenting the combination of
- two options whose names read as contradictory together. Internally it
- just sets both flags, reusing the existing suppression logic in
- show_range_diff().
+ one-sided groups, but they are defined as "only show this side" and
+ so cannot be given together, which is exactly why show_range_diff()
+ already rejected that combination. Give the "show only the commits
+ that correspond on both sides" behavior its own name, --matched-only,
+ instead of asking users to reach for a combination that errors out.
- Extend the existing '--left-only'/'--right-only' conflict check in
- show_range_diff() to also reject any combination with --matched-only,
- since all three narrow the output in ways that cannot be combined.
+ Extend the existing '--left-only'/'--right-only' conflict check to
+ also reject any combination with --matched-only, since all three
+ narrow the output in ways that cannot be combined.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
@@ Documentation/git-range-diff.adoc: to revert to color all lines according to the
+`--matched-only`::
+ Only emit commits that have a corresponding commit in the other
-+ range, suppressing any commit that exists on only one side. This is
-+ the same as using `--left-only` and `--right-only` together. Useful
++ range, suppressing any commit that exists on only one side. Useful
+ to skip added or removed commits when reviewing how the commits
+ that survived a rebase changed.
+
@@ builtin/range-diff.c: int cmd_range_diff(int argc,
strvec_clear(&log_arg);
## range-diff.c ##
+@@
+ #include "userdiff.h"
+ #include "apply.h"
+ #include "revision.h"
++#include "parse-options.h"
+
+ struct patch_util {
+ /* For the search for an exact match */
@@ range-diff.c: int show_range_diff(const char *range1, const char *range2,
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 (range_diff_opts->left_only + range_diff_opts->right_only +
-+ range_diff_opts->matched_only > 1)
-+ res = error(_("options '%s', '%s', or '%s' cannot be used together"),
-+ "--left-only", "--right-only", "--matched-only");
-+
++ die_for_incompatible_opt3(range_diff_opts->left_only, "--left-only",
++ range_diff_opts->right_only, "--right-only",
++ range_diff_opts->matched_only, "--matched-only");
+
+- if (!res && read_patches(range1, &branch1, range_diff_opts->log_arg, include_merges))
+ if (range_diff_opts->matched_only) {
+ range_diff_opts->left_only = 1;
+ range_diff_opts->right_only = 1;
+ }
-
- if (!res && read_patches(range1, &branch1, range_diff_opts->log_arg, include_merges))
++
++ if (read_patches(range1, &branch1, range_diff_opts->log_arg, include_merges))
res = error(_("could not parse log for '%s'"), range1);
+ if (!res && read_patches(range2, &branch2, range_diff_opts->log_arg, include_merges))
+ res = error(_("could not parse log for '%s'"), range2);
## range-diff.h ##
@@ Documentation/git-range-diff.adoc | 9 ++++-
builtin/range-diff.c | 5 ++-
range-diff.c | 13 +++++--
range-diff.h | 2 +-
t/t3206-range-diff.sh | 63 +++++++++++++++++++++++++++++++
5 files changed, 86 insertions(+), 6 deletions(-)
Show changes to 5 files +86 −6
Documentation/git-range-diff.adoc, builtin/range-diff.c, range-diff.c, range-diff.h, t/t3206-range-diff.sh
diff --git a/Documentation/git-range-diff.adoc b/Documentation/git-range-diff.adoc
index 5cc5e2ed56..d448f897d6 100644
--- a/Documentation/git-range-diff.adoc
+++ b/Documentation/git-range-diff.adoc
@@ -10,7 +10,8 @@ SYNOPSIS
[synopsis]
git range-diff [--color=[<when>]] [--no-color] [<diff-options>]
[--no-dual-color] [--creation-factor=<factor>]
- [--left-only | --right-only] [--diff-merges=<format>]
+ [--left-only | --right-only | --matched-only]
+ [--diff-merges=<format>]
[--remerge-diff] [--no-notes | --notes[=<ref>]]
( <range1> <range2> | <rev1>...<rev2> | <base> <rev1> <rev2> )
[[--] <path>...]
@@ -82,6 +83,12 @@ 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>` form).
+`--matched-only`::
+ Only emit commits that have a corresponding commit in the other
+ range, suppressing any commit that exists on only one side. Useful
+ to skip added or removed commits when reviewing how the commits
+ that survived a rebase changed.
+
`--diff-merges=<format>`::
Instead of ignoring merge commits, generate diffs for them using the
corresponding `--diff-merges=<format>` option of linkgit:git-log[1],
diff --git a/builtin/range-diff.c b/builtin/range-diff.c
index e54c0f7fe1..8059f92eaa 100644
--- a/builtin/range-diff.c
+++ b/builtin/range-diff.c
@@ -46,7 +46,7 @@ int cmd_range_diff(int argc,
.diffopt = &diffopt,
.log_arg = &log_arg
};
- int simple_color = -1, left_only = 0, right_only = 0;
+ int simple_color = -1, left_only = 0, right_only = 0, matched_only = 0;
struct option range_diff_options[] = {
OPT_INTEGER(0, "creation-factor",
&range_diff_opts.creation_factor,
@@ -68,6 +68,8 @@ int cmd_range_diff(int argc,
N_("only emit output related to the first range")),
OPT_BOOL(0, "right-only", &right_only,
N_("only emit output related to the second range")),
+ OPT_BOOL(0, "matched-only", &matched_only,
+ N_("only emit commits that have a corresponding commit in the other range")),
OPT_END()
};
struct option *options;
@@ -186,6 +188,7 @@ int cmd_range_diff(int argc,
range_diff_opts.dual_color = simple_color < 1;
range_diff_opts.left_only = left_only;
range_diff_opts.right_only = right_only;
+ range_diff_opts.matched_only = matched_only;
res = show_range_diff(range1.buf, range2.buf, &range_diff_opts);
strvec_clear(&log_arg);
diff --git a/range-diff.c b/range-diff.c
index 8e2dd2eb19..67cc751658 100644
--- a/range-diff.c
+++ b/range-diff.c
@@ -20,6 +20,7 @@
#include "userdiff.h"
#include "apply.h"
#include "revision.h"
+#include "parse-options.h"
struct patch_util {
/* For the search for an exact match */
@@ -591,10 +592,16 @@ int show_range_diff(const char *range1, const char *range2,
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");
+ die_for_incompatible_opt3(range_diff_opts->left_only, "--left-only",
+ range_diff_opts->right_only, "--right-only",
+ range_diff_opts->matched_only, "--matched-only");
- if (!res && read_patches(range1, &branch1, range_diff_opts->log_arg, include_merges))
+ if (range_diff_opts->matched_only) {
+ range_diff_opts->left_only = 1;
+ range_diff_opts->right_only = 1;
+ }
+
+ if (read_patches(range1, &branch1, range_diff_opts->log_arg, include_merges))
res = error(_("could not parse log for '%s'"), range1);
if (!res && read_patches(range2, &branch2, range_diff_opts->log_arg, include_merges))
res = error(_("could not parse log for '%s'"), range2);
diff --git a/range-diff.h b/range-diff.h
index 9b70a80009..effd10b9b8 100644
--- a/range-diff.h
+++ b/range-diff.h
@@ -19,7 +19,7 @@
struct range_diff_options {
int creation_factor;
unsigned dual_color:1;
- unsigned left_only:1, right_only:1;
+ unsigned left_only:1, right_only:1, matched_only:1;
unsigned include_merges:1;
size_t max_memory;
const struct diff_options *diffopt; /* may be NULL */
diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
index ef92704de3..f85fd0c4ad 100755
--- a/t/t3206-range-diff.sh
+++ b/t/t3206-range-diff.sh
@@ -860,6 +860,69 @@ test_expect_success '--left-only/--right-only' '
test_cmp expect actual
'
+test_expect_success '--left-only, --right-only and --matched-only are incompatible' '
+ test_must_fail git range-diff --left-only --right-only ...common 2>err &&
+ test_grep "cannot be used together" err &&
+
+ test_must_fail git range-diff --left-only --matched-only ...common 2>err &&
+ test_grep "cannot be used together" err &&
+
+ test_must_fail git range-diff --right-only --matched-only ...common 2>err &&
+ test_grep "cannot be used together" err &&
+
+ test_must_fail git range-diff --left-only --right-only --matched-only \
+ ...common 2>err &&
+ test_grep "cannot be used together" err
+'
+
+test_expect_success '--left-only, --right-only and --matched-only each suppress one-sided commits' '
+ test_create_repo matched-only &&
+ (
+ cd matched-only &&
+ git switch --orphan combined-old &&
+ test_commit c-first &&
+ test_commit c-old-only &&
+ test_commit c-common &&
+ git switch -C combined-new c-first &&
+ test_commit c-new-only &&
+ git cherry-pick c-common &&
+
+ old_only_oid=$(git rev-parse --short=7 c-old-only) &&
+ new_only_oid=$(git rev-parse --short=7 c-new-only) &&
+ common_old_oid=$(git rev-parse --short=7 c-common) &&
+ common_new_oid=$(git rev-parse --short=7 HEAD) &&
+
+ git range-diff -s --abbrev=7 combined-old...combined-new >actual &&
+ cat >expect <<-EOF &&
+ 1: $old_only_oid < -: ------- c-old-only
+ -: ------- > 1: $new_only_oid c-new-only
+ 2: $common_old_oid = 2: $common_new_oid c-common
+ EOF
+ test_cmp expect actual &&
+
+ git range-diff -s --abbrev=7 --left-only combined-old...combined-new \
+ >actual &&
+ cat >expect <<-EOF &&
+ 1: $old_only_oid < -: ------- c-old-only
+ 2: $common_old_oid = 2: $common_new_oid c-common
+ EOF
+ test_cmp expect actual &&
+
+ git range-diff -s --abbrev=7 --right-only combined-old...combined-new \
+ >actual &&
+ cat >expect <<-EOF &&
+ -: ------- > 1: $new_only_oid c-new-only
+ 2: $common_old_oid = 2: $common_new_oid c-common
+ EOF
+ test_cmp expect actual &&
+
+ git range-diff -s --abbrev=7 --matched-only combined-old...combined-new \
+ >actual &&
+ echo "2: $common_old_oid = 2: $common_new_oid c-common" >expect &&
+ test_cmp expect actual
+ )
+'
+
test_expect_success 'ranges with pathspecs' '
git range-diff topic...mode-only-change -- other-file >actual &&
test_line_count = 2 actual &&
base-commit: 47ce80527c56f462cb97db4ca8125342204d3783
--
gitgitgadget
Re: [PATCH v2] range-diff: add --matched-only to skip one-sided commits
"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 5 quoted lines
> - 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");
> + die_for_incompatible_opt3(range_diff_opts->left_only, "--left-only",
> + range_diff_opts->right_only, "--right-only",
> + range_diff_opts->matched_only, "--matched-only");Sorry for suggesting this change during the review. After thinking about it a bit more, I think it is a bad change.
Top-level range-diff.c should be kept reusable from different front-ends, some of which may invent different ways to deal with errors coming from incorrect end-user settings. It would rob them of an opportunity to recover if we called die() from here like this. Calling error() and signaling an error to the caller, as the original code did, was the right thing to do.
In other words, this adds an unnecessary layering violation, and the original code was much better from that point of view.
We might eventually want to have error_for_incompatible_optN() so that this hunk becomes something like:
- 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"); + res |= incompatible_opt_error("--left-only", range_diff_opts->left_only, + "--right-only", range_diff_opts->right_only, + "--matched-only", range_diff_opts->matched_only, + NULL);
but I think we should leave that outside this topic.
In general, whenever I need to say "something like" (as opposed to "exactly this") in a suggestion, it is a sign that the overall idea might be OK, but the details need to be worked out and would take a lot of time. I think this is one of those cases that are better handled as a separate topic without holding this change hostage.
Thanks.
[PATCH v3] range-diff: add --matched-only to skip one-sided commits
From: Harald Nordgren <haraldnordgren@gmail.com>
Reviewing a range-diff often means scrolling past commits that were simply added or dropped, when only the ones that correspond between the two ranges are of interest.
--left-only and --right-only already each suppress one of those one-sided groups, but they are defined as "only show this side" and so cannot be given together, which is exactly why show_range_diff() already rejected that combination. Give the "show only the commits that correspond on both sides" behavior its own name, --matched-only, instead of asking users to reach for a combination that errors out.
Extend the existing '--left-only'/'--right-only' conflict check to also reject any combination with --matched-only, since all three narrow the output in ways that cannot be combined.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
range-diff: add --matched-only to skip one-sided commits
Add git range-diff --matched-only to only show commits that correspond
between the two ranges, skipping ones that were only added or only
removed.
Changes in v2:
* Update docs and commit message.
* Use die_for_incompatible_opt3.Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2401%2FHaraldNordgren%2Frange-diff-matched-only-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2401/HaraldNordgren/range-diff-matched-only-v3
Pull-Request: https://github.com/git/git/pull/2401
Range-diff vs v2:
1: 6d392249a2 ! 1: ee195c14fc range-diff: add --matched-only to skip one-sided commits
@@ builtin/range-diff.c: int cmd_range_diff(int argc,
strvec_clear(&log_arg);
## range-diff.c ##
-@@
- #include "userdiff.h"
- #include "apply.h"
- #include "revision.h"
-+#include "parse-options.h"
-
- struct patch_util {
- /* For the search for an exact match */
@@ range-diff.c: int show_range_diff(const char *range1, const char *range2,
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");
-+ die_for_incompatible_opt3(range_diff_opts->left_only, "--left-only",
-+ range_diff_opts->right_only, "--right-only",
-+ range_diff_opts->matched_only, "--matched-only");
-
-- if (!res && read_patches(range1, &branch1, range_diff_opts->log_arg, include_merges))
++ if (range_diff_opts->left_only + range_diff_opts->right_only +
++ range_diff_opts->matched_only > 1)
++ res = error(_("options '%s', '%s', or '%s' cannot be used together"),
++ "--left-only", "--right-only", "--matched-only");
++
+ if (range_diff_opts->matched_only) {
+ range_diff_opts->left_only = 1;
+ range_diff_opts->right_only = 1;
+ }
-+
-+ if (read_patches(range1, &branch1, range_diff_opts->log_arg, include_merges))
+
+ if (!res && read_patches(range1, &branch1, range_diff_opts->log_arg, include_merges))
res = error(_("could not parse log for '%s'"), range1);
- if (!res && read_patches(range2, &branch2, range_diff_opts->log_arg, include_merges))
- res = error(_("could not parse log for '%s'"), range2);
## range-diff.h ##
@@ Documentation/git-range-diff.adoc | 9 ++++-
builtin/range-diff.c | 5 ++-
range-diff.c | 11 +++++-
range-diff.h | 2 +-
t/t3206-range-diff.sh | 63 +++++++++++++++++++++++++++++++
5 files changed, 85 insertions(+), 5 deletions(-)
Show changes to 5 files +85 −5
Documentation/git-range-diff.adoc, builtin/range-diff.c, range-diff.c, range-diff.h, t/t3206-range-diff.sh
diff --git a/Documentation/git-range-diff.adoc b/Documentation/git-range-diff.adoc
index 5cc5e2ed56..d448f897d6 100644
--- a/Documentation/git-range-diff.adoc
+++ b/Documentation/git-range-diff.adoc
@@ -10,7 +10,8 @@ SYNOPSIS
[synopsis]
git range-diff [--color=[<when>]] [--no-color] [<diff-options>]
[--no-dual-color] [--creation-factor=<factor>]
- [--left-only | --right-only] [--diff-merges=<format>]
+ [--left-only | --right-only | --matched-only]
+ [--diff-merges=<format>]
[--remerge-diff] [--no-notes | --notes[=<ref>]]
( <range1> <range2> | <rev1>...<rev2> | <base> <rev1> <rev2> )
[[--] <path>...]
@@ -82,6 +83,12 @@ 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>` form).
+`--matched-only`::
+ Only emit commits that have a corresponding commit in the other
+ range, suppressing any commit that exists on only one side. Useful
+ to skip added or removed commits when reviewing how the commits
+ that survived a rebase changed.
+
`--diff-merges=<format>`::
Instead of ignoring merge commits, generate diffs for them using the
corresponding `--diff-merges=<format>` option of linkgit:git-log[1],
diff --git a/builtin/range-diff.c b/builtin/range-diff.c
index e54c0f7fe1..8059f92eaa 100644
--- a/builtin/range-diff.c
+++ b/builtin/range-diff.c
@@ -46,7 +46,7 @@ int cmd_range_diff(int argc,
.diffopt = &diffopt,
.log_arg = &log_arg
};
- int simple_color = -1, left_only = 0, right_only = 0;
+ int simple_color = -1, left_only = 0, right_only = 0, matched_only = 0;
struct option range_diff_options[] = {
OPT_INTEGER(0, "creation-factor",
&range_diff_opts.creation_factor,
@@ -68,6 +68,8 @@ int cmd_range_diff(int argc,
N_("only emit output related to the first range")),
OPT_BOOL(0, "right-only", &right_only,
N_("only emit output related to the second range")),
+ OPT_BOOL(0, "matched-only", &matched_only,
+ N_("only emit commits that have a corresponding commit in the other range")),
OPT_END()
};
struct option *options;
@@ -186,6 +188,7 @@ int cmd_range_diff(int argc,
range_diff_opts.dual_color = simple_color < 1;
range_diff_opts.left_only = left_only;
range_diff_opts.right_only = right_only;
+ range_diff_opts.matched_only = matched_only;
res = show_range_diff(range1.buf, range2.buf, &range_diff_opts);
strvec_clear(&log_arg);
diff --git a/range-diff.c b/range-diff.c
index 8e2dd2eb19..fa895f5760 100644
--- a/range-diff.c
+++ b/range-diff.c
@@ -591,8 +591,15 @@ int show_range_diff(const char *range1, const char *range2,
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 (range_diff_opts->left_only + range_diff_opts->right_only +
+ range_diff_opts->matched_only > 1)
+ res = error(_("options '%s', '%s', or '%s' cannot be used together"),
+ "--left-only", "--right-only", "--matched-only");
+
+ if (range_diff_opts->matched_only) {
+ range_diff_opts->left_only = 1;
+ range_diff_opts->right_only = 1;
+ }
if (!res && read_patches(range1, &branch1, range_diff_opts->log_arg, include_merges))
res = error(_("could not parse log for '%s'"), range1);
diff --git a/range-diff.h b/range-diff.h
index 9b70a80009..effd10b9b8 100644
--- a/range-diff.h
+++ b/range-diff.h
@@ -19,7 +19,7 @@
struct range_diff_options {
int creation_factor;
unsigned dual_color:1;
- unsigned left_only:1, right_only:1;
+ unsigned left_only:1, right_only:1, matched_only:1;
unsigned include_merges:1;
size_t max_memory;
const struct diff_options *diffopt; /* may be NULL */
diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
index ef92704de3..f85fd0c4ad 100755
--- a/t/t3206-range-diff.sh
+++ b/t/t3206-range-diff.sh
@@ -860,6 +860,69 @@ test_expect_success '--left-only/--right-only' '
test_cmp expect actual
'
+test_expect_success '--left-only, --right-only and --matched-only are incompatible' '
+ test_must_fail git range-diff --left-only --right-only ...common 2>err &&
+ test_grep "cannot be used together" err &&
+
+ test_must_fail git range-diff --left-only --matched-only ...common 2>err &&
+ test_grep "cannot be used together" err &&
+
+ test_must_fail git range-diff --right-only --matched-only ...common 2>err &&
+ test_grep "cannot be used together" err &&
+
+ test_must_fail git range-diff --left-only --right-only --matched-only \
+ ...common 2>err &&
+ test_grep "cannot be used together" err
+'
+
+test_expect_success '--left-only, --right-only and --matched-only each suppress one-sided commits' '
+ test_create_repo matched-only &&
+ (
+ cd matched-only &&
+ git switch --orphan combined-old &&
+ test_commit c-first &&
+ test_commit c-old-only &&
+ test_commit c-common &&
+ git switch -C combined-new c-first &&
+ test_commit c-new-only &&
+ git cherry-pick c-common &&
+
+ old_only_oid=$(git rev-parse --short=7 c-old-only) &&
+ new_only_oid=$(git rev-parse --short=7 c-new-only) &&
+ common_old_oid=$(git rev-parse --short=7 c-common) &&
+ common_new_oid=$(git rev-parse --short=7 HEAD) &&
+
+ git range-diff -s --abbrev=7 combined-old...combined-new >actual &&
+ cat >expect <<-EOF &&
+ 1: $old_only_oid < -: ------- c-old-only
+ -: ------- > 1: $new_only_oid c-new-only
+ 2: $common_old_oid = 2: $common_new_oid c-common
+ EOF
+ test_cmp expect actual &&
+
+ git range-diff -s --abbrev=7 --left-only combined-old...combined-new \
+ >actual &&
+ cat >expect <<-EOF &&
+ 1: $old_only_oid < -: ------- c-old-only
+ 2: $common_old_oid = 2: $common_new_oid c-common
+ EOF
+ test_cmp expect actual &&
+
+ git range-diff -s --abbrev=7 --right-only combined-old...combined-new \
+ >actual &&
+ cat >expect <<-EOF &&
+ -: ------- > 1: $new_only_oid c-new-only
+ 2: $common_old_oid = 2: $common_new_oid c-common
+ EOF
+ test_cmp expect actual &&
+
+ git range-diff -s --abbrev=7 --matched-only combined-old...combined-new \
+ >actual &&
+ echo "2: $common_old_oid = 2: $common_new_oid c-common" >expect &&
+ test_cmp expect actual
+ )
+'
+
test_expect_success 'ranges with pathspecs' '
git range-diff topic...mode-only-change -- other-file >actual &&
test_line_count = 2 actual &&
base-commit: 3699d22b59a6ea467ce13edb81b6bdea0398c803
--
gitgitgadget