[PATCH 0/6] Optionally restrict range-diff output to "left" or "right" range only

[PATCH 0/6] Optionally restrict range-diff output to "left" or "right" range only

[Johannes Schindelin via GitGitGadget]

One of my quite common workflows is to see whether an ancient topic branch I
have lying about has made it into Git. Since my local commit OIDs have
nothing to do with the OIDs of the corresponding commits in git/git, my only
way is to fire up git range-diff ...upstream/master, but of course that
output contains way more commits than I care about.

To help this use case, here is a patch series that teaches git range-diff
the --left-only and --right-only options in the end, restricting the output
to those commits and commit pairs that correspond to the commits in the
first and the second range, respectively.

The first part of the series contains cleanup patches that are not strictly
related to the feature I implemented here, but since I already have them, I
figured I could just as well contribute them all together.

This patch series is based on js/range-diff-wo-dotdot.

Johannes Schindelin (6):
  range-diff: avoid leaking memory in two error code paths
  range-diff: libify the read_patches() function again
  range-diff: simplify code spawning `git log`
  range-diff: combine all options in a single data structure
  range-diff: move the diffopt initialization down one layer
  range-diff: offer --left-only/--right-only options

 Documentation/git-range-diff.txt |   9 +++
 builtin/log.c                    |  10 ++-
 builtin/range-diff.c             |  21 +++++--
 log-tree.c                       |   8 ++-
 range-diff.c                     | 101 +++++++++++++++++--------------
 range-diff.h                     |  12 +++-
 t/t3206-range-diff.sh            |  15 +++++
 7 files changed, 118 insertions(+), 58 deletions(-)


base-commit: 43718f6741a87f87bd400bdf5264394e980583c5
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-869%2Fdscho%2Frange-diff-left-and-right-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-869/dscho/range-diff-left-and-right-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/869
-- 
gitgitgadget

[PATCH 1/6] range-diff: avoid leaking memory in two error code paths

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

In the code paths in question, we already release a lot of memory, but
the `current_filename` variable was missed. Fix that.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 range-diff.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/range-diff.c b/range-diff.c
index 0c6ac4f954d1..963cdecb4cbb 100644
--- a/range-diff.c
+++ b/range-diff.c
@@ -97,6 +97,7 @@ static int read_patches(const char *range, struct string_list *list,
 			if (get_oid(p, &util->oid)) {
 				error(_("could not parse commit '%s'"), p);
 				free(util);
+				free(current_filename);
 				string_list_clear(list, 1);
 				strbuf_release(&buf);
 				strbuf_release(&contents);
@@ -112,6 +113,7 @@ static int read_patches(const char *range, struct string_list *list,
 			error(_("could not parse first line of `log` output: "
 				"did not start with 'commit ': '%s'"),
 			      line);
+			free(current_filename);
 			string_list_clear(list, 1);
 			strbuf_release(&buf);
 			strbuf_release(&contents);
-- 
gitgitgadget

[PATCH 2/6] range-diff: libify the read_patches() function again

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

In library functions, we do want to avoid the (simple, but rather final)
`die()` calls, instead returning with a value indicating an error.

Let's do exactly that in the code introduced in b66885a30cb8
(range-diff: add section header instead of diff header, 2019-07-11) that
wants to error out if a diff header could not be parsed.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 range-diff.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/range-diff.c b/range-diff.c
index 963cdecb4cbb..6707b728a07f 100644
--- a/range-diff.c
+++ b/range-diff.c
@@ -135,9 +135,16 @@ static int read_patches(const char *range, struct string_list *list,
 			orig_len = len;
 			len = parse_git_diff_header(&root, &linenr, 0, line,
 						    len, size, &patch);
-			if (len < 0)
-				die(_("could not parse git header '%.*s'"),
-				    orig_len, line);
+			if (len < 0) {
+				error(_("could not parse git header '%.*s'"),
+				      orig_len, line);
+				free(util);
+				free(current_filename);
+				string_list_clear(list, 1);
+				strbuf_release(&buf);
+				strbuf_release(&contents);
+				return -1;
+			}
 			strbuf_addstr(&buf, " ## ");
 			if (patch.is_new > 0)
 				strbuf_addf(&buf, "%s (new)", patch.new_name);
-- 
gitgitgadget

[PATCH 3/6] range-diff: simplify code spawning `git log`

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

Previously, we waited for the child process to be finished in every
failing code path as well as at the end of the function
`show_range_diff()`.

However, we do not need to wait that long. Directly after reading the
output of the child process, we can wrap up the child process.

This also has the advantage that we don't do a bunch of unnecessary work
in case `finish_command()` returns with an error anyway.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 range-diff.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/range-diff.c b/range-diff.c
index 6707b728a07f..3919d56e3716 100644
--- a/range-diff.c
+++ b/range-diff.c
@@ -80,6 +80,8 @@ static int read_patches(const char *range, struct string_list *list,
 		finish_command(&cp);
 		return -1;
 	}
+	if (finish_command(&cp))
+		return -1;
 
 	line = contents.buf;
 	size = contents.len;
@@ -101,7 +103,6 @@ static int read_patches(const char *range, struct string_list *list,
 				string_list_clear(list, 1);
 				strbuf_release(&buf);
 				strbuf_release(&contents);
-				finish_command(&cp);
 				return -1;
 			}
 			util->matching = -1;
@@ -117,7 +118,6 @@ static int read_patches(const char *range, struct string_list *list,
 			string_list_clear(list, 1);
 			strbuf_release(&buf);
 			strbuf_release(&contents);
-			finish_command(&cp);
 			return -1;
 		}
 
@@ -227,9 +227,6 @@ static int read_patches(const char *range, struct string_list *list,
 	strbuf_release(&buf);
 	free(current_filename);
 
-	if (finish_command(&cp))
-		return -1;
-
 	return 0;
 }
 
-- 
gitgitgadget

[PATCH 4/6] range-diff: combine all options in a single data structure

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

This will make it easier to implement the `--left-only` and
`--right-only` options.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin/log.c        | 10 ++++++++--
 builtin/range-diff.c | 13 +++++++++----
 log-tree.c           |  8 ++++++--
 range-diff.c         | 18 +++++++++---------
 range-diff.h         | 11 ++++++++---
 5 files changed, 40 insertions(+), 20 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 91466c059c79..a06e5385689b 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1231,14 +1231,20 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
 		 */
 		struct diff_options opts;
 		struct strvec other_arg = STRVEC_INIT;
+		struct range_diff_options range_diff_opts = {
+			.creation_factor = rev->creation_factor,
+			.dual_color = 1,
+			.diffopt = &opts,
+			.other_arg = &other_arg
+		};
+
 		diff_setup(&opts);
 		opts.file = rev->diffopt.file;
 		opts.use_color = rev->diffopt.use_color;
 		diff_setup_done(&opts);
 		fprintf_ln(rev->diffopt.file, "%s", rev->rdiff_title);
 		get_notes_args(&other_arg, rev);
-		show_range_diff(rev->rdiff1, rev->rdiff2,
-				rev->creation_factor, 1, &opts, &other_arg);
+		show_range_diff(rev->rdiff1, rev->rdiff2, &range_diff_opts);
 		strvec_clear(&other_arg);
 	}
 }
diff --git a/builtin/range-diff.c b/builtin/range-diff.c
index 5b1f6326322f..80fcdc6ad42d 100644
--- a/builtin/range-diff.c
+++ b/builtin/range-diff.c
@@ -14,12 +14,17 @@ NULL
 
 int cmd_range_diff(int argc, const char **argv, const char *prefix)
 {
-	int creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;
 	struct diff_options diffopt = { NULL };
 	struct strvec other_arg = STRVEC_INIT;
+	struct range_diff_options range_diff_opts = {
+		.creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT,
+		.diffopt = &diffopt,
+		.other_arg = &other_arg
+	};
 	int simple_color = -1;
 	struct option range_diff_options[] = {
-		OPT_INTEGER(0, "creation-factor", &creation_factor,
+		OPT_INTEGER(0, "creation-factor",
+			    &range_diff_opts.creation_factor,
 			    N_("Percentage by which creation is weighted")),
 		OPT_BOOL(0, "no-dual-color", &simple_color,
 			    N_("use simple diff colors")),
@@ -82,8 +87,8 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)
 	}
 	FREE_AND_NULL(options);
 
-	res = show_range_diff(range1.buf, range2.buf, creation_factor,
-			      simple_color < 1, &diffopt, &other_arg);
+	range_diff_opts.dual_color = simple_color < 1;
+	res = show_range_diff(range1.buf, range2.buf, &range_diff_opts);
 
 	strvec_clear(&other_arg);
 	strbuf_release(&range1);
diff --git a/log-tree.c b/log-tree.c
index fd0dde97ec32..eeacba15dc94 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -808,6 +808,11 @@ void show_log(struct rev_info *opt)
 	if (cmit_fmt_is_mail(ctx.fmt) && opt->rdiff1) {
 		struct diff_queue_struct dq;
 		struct diff_options opts;
+		struct range_diff_options range_diff_opts = {
+			.creation_factor = opt->creation_factor,
+			.dual_color = 1,
+			.diffopt = &opts
+		};
 
 		memcpy(&dq, &diff_queued_diff, sizeof(diff_queued_diff));
 		DIFF_QUEUE_CLEAR(&diff_queued_diff);
@@ -822,8 +827,7 @@ void show_log(struct rev_info *opt)
 		opts.file = opt->diffopt.file;
 		opts.use_color = opt->diffopt.use_color;
 		diff_setup_done(&opts);
-		show_range_diff(opt->rdiff1, opt->rdiff2,
-				opt->creation_factor, 1, &opts, NULL);
+		show_range_diff(opt->rdiff1, opt->rdiff2, &range_diff_opts);
 
 		memcpy(&diff_queued_diff, &dq, sizeof(diff_queued_diff));
 	}
diff --git a/range-diff.c b/range-diff.c
index 3919d56e3716..58528c43a3e8 100644
--- a/range-diff.c
+++ b/range-diff.c
@@ -525,33 +525,32 @@ static struct strbuf *output_prefix_cb(struct diff_options *opt, void *data)
 }
 
 int show_range_diff(const char *range1, const char *range2,
-		    int creation_factor, int dual_color,
-		    const struct diff_options *diffopt,
-		    const struct strvec *other_arg)
+		    struct range_diff_options *range_diff_opts)
 {
 	int res = 0;
 
 	struct string_list branch1 = STRING_LIST_INIT_DUP;
 	struct string_list branch2 = STRING_LIST_INIT_DUP;
 
-	if (read_patches(range1, &branch1, other_arg))
+	if (read_patches(range1, &branch1, range_diff_opts->other_arg))
 		res = error(_("could not parse log for '%s'"), range1);
-	if (!res && read_patches(range2, &branch2, other_arg))
+	if (!res && read_patches(range2, &branch2, range_diff_opts->other_arg))
 		res = error(_("could not parse log for '%s'"), range2);
 
 	if (!res) {
 		struct diff_options opts;
 		struct strbuf indent = STRBUF_INIT;
 
-		if (diffopt)
-			memcpy(&opts, diffopt, sizeof(opts));
+		if (range_diff_opts->diffopt)
+			memcpy(&opts, range_diff_opts->diffopt, sizeof(opts));
 		else
 			diff_setup(&opts);
 
 		if (!opts.output_format)
 			opts.output_format = DIFF_FORMAT_PATCH;
 		opts.flags.suppress_diff_headers = 1;
-		opts.flags.dual_color_diffed_diffs = dual_color;
+		opts.flags.dual_color_diffed_diffs =
+			range_diff_opts->dual_color;
 		opts.flags.suppress_hunk_header_line_count = 1;
 		opts.output_prefix = output_prefix_cb;
 		strbuf_addstr(&indent, "    ");
@@ -559,7 +558,8 @@ int show_range_diff(const char *range1, const char *range2,
 		diff_setup_done(&opts);
 
 		find_exact_matches(&branch1, &branch2);
-		get_correspondences(&branch1, &branch2, creation_factor);
+		get_correspondences(&branch1, &branch2,
+				    range_diff_opts->creation_factor);
 		output(&branch1, &branch2, &opts);
 
 		strbuf_release(&indent);
diff --git a/range-diff.h b/range-diff.h
index c17dbc2e75a8..8fb2ff05865d 100644
--- a/range-diff.h
+++ b/range-diff.h
@@ -6,15 +6,20 @@
 
 #define RANGE_DIFF_CREATION_FACTOR_DEFAULT 60
 
+struct range_diff_options {
+	int creation_factor;
+	unsigned dual_color:1;
+	const struct diff_options *diffopt;
+	const struct strvec *other_arg;
+};
+
 /*
  * Compare series of commits in RANGE1 and RANGE2, and emit to the
  * standard output.  NULL can be passed to DIFFOPT to use the built-in
  * default.
  */
 int show_range_diff(const char *range1, const char *range2,
-		    int creation_factor, int dual_color,
-		    const struct diff_options *diffopt,
-		    const struct strvec *other_arg);
+		    struct range_diff_options *opts);
 
 /*
  * Determine whether the given argument is usable as a range argument of `git
-- 
gitgitgadget

Re: [PATCH 4/6] range-diff: combine all options in a single data structure

[Eric Sunshine]

On Thu, Feb 4, 2021 at 3:24 PM Johannes Schindelin via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> This will make it easier to implement the `--left-only` and
> `--right-only` options.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> diff --git a/range-diff.h b/range-diff.h
> @@ -6,15 +6,20 @@
> +struct range_diff_options {
> +       int creation_factor;
> +       unsigned dual_color:1;
> +       const struct diff_options *diffopt;
> +       const struct strvec *other_arg;
> +};
> +
>  /*
>   * Compare series of commits in RANGE1 and RANGE2, and emit to the
>   * standard output.  NULL can be passed to DIFFOPT to use the built-in
>   * default.
>   */
>  int show_range_diff(const char *range1, const char *range2,
> -                   int creation_factor, int dual_color,
> -                   const struct diff_options *diffopt,
> -                   const struct strvec *other_arg);
> +                   struct range_diff_options *opts);

The function comment's mention of DIFFOPT becomes outdated with this
change. Perhaps update it to say `opts.diffopt` or something.

Re: [PATCH 4/6] range-diff: combine all options in a single data structure

[Johannes Schindelin]

Hi Eric,

On Thu, 4 Feb 2021, Eric Sunshine wrote:

> On Thu, Feb 4, 2021 at 3:24 PM Johannes Schindelin via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
> > This will make it easier to implement the `--left-only` and
> > `--right-only` options.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> > diff --git a/range-diff.h b/range-diff.h
> > @@ -6,15 +6,20 @@
> > +struct range_diff_options {
> > +       int creation_factor;
> > +       unsigned dual_color:1;
> > +       const struct diff_options *diffopt;
> > +       const struct strvec *other_arg;
> > +};
> > +
> >  /*
> >   * Compare series of commits in RANGE1 and RANGE2, and emit to the
> >   * standard output.  NULL can be passed to DIFFOPT to use the built-in
> >   * default.
> >   */
> >  int show_range_diff(const char *range1, const char *range2,
> > -                   int creation_factor, int dual_color,
> > -                   const struct diff_options *diffopt,
> > -                   const struct strvec *other_arg);
> > +                   struct range_diff_options *opts);
>
> The function comment's mention of DIFFOPT becomes outdated with this
> change. Perhaps update it to say `opts.diffopt` or something.

I actually moved the comment to the new `struct`.

Thanks,
Dscho

[PATCH 5/6] range-diff: move the diffopt initialization down one layer

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

It is actually only the `output()` function that uses those diffopts. By
moving the diffopt initialization down into that function, it is
encapsulated better.

Incidentally, it will also make it easier to implement the `--left-only`
and `--right-only` options in `git range-diff` because the `output()`
function is now receiving all range-diff options as a parameter, not
just the diffopts.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 range-diff.c | 64 +++++++++++++++++++++++++---------------------------
 1 file changed, 31 insertions(+), 33 deletions(-)

diff --git a/range-diff.c b/range-diff.c
index 58528c43a3e8..5455cbb9521b 100644
--- a/range-diff.c
+++ b/range-diff.c
@@ -464,12 +464,35 @@ static void patch_diff(const char *a, const char *b,
 	diff_flush(diffopt);
 }
 
+static struct strbuf *output_prefix_cb(struct diff_options *opt, void *data)
+{
+	return data;
+}
+
 static void output(struct string_list *a, struct string_list *b,
-		   struct diff_options *diffopt)
+		   struct range_diff_options *range_diff_opts)
 {
 	struct strbuf buf = STRBUF_INIT, dashes = STRBUF_INIT;
 	int patch_no_width = decimal_width(1 + (a->nr > b->nr ? a->nr : b->nr));
 	int i = 0, j = 0;
+	struct diff_options opts;
+	struct strbuf indent = STRBUF_INIT;
+
+	if (range_diff_opts->diffopt)
+		memcpy(&opts, range_diff_opts->diffopt, sizeof(opts));
+	else
+		diff_setup(&opts);
+
+	if (!opts.output_format)
+		opts.output_format = DIFF_FORMAT_PATCH;
+	opts.flags.suppress_diff_headers = 1;
+	opts.flags.dual_color_diffed_diffs =
+		range_diff_opts->dual_color;
+	opts.flags.suppress_hunk_header_line_count = 1;
+	opts.output_prefix = output_prefix_cb;
+	strbuf_addstr(&indent, "    ");
+	opts.output_prefix_data = &indent;
+	diff_setup_done(&opts);
 
 	/*
 	 * We assume the user is really more interested in the second argument
@@ -490,7 +513,7 @@ static void output(struct string_list *a, struct string_list *b,
 
 		/* Show unmatched LHS commit whose predecessors were shown. */
 		if (i < a->nr && a_util->matching < 0) {
-			output_pair_header(diffopt, patch_no_width,
+			output_pair_header(&opts, patch_no_width,
 					   &buf, &dashes, a_util, NULL);
 			i++;
 			continue;
@@ -498,7 +521,7 @@ static void output(struct string_list *a, struct string_list *b,
 
 		/* Show unmatched RHS commits. */
 		while (j < b->nr && b_util->matching < 0) {
-			output_pair_header(diffopt, patch_no_width,
+			output_pair_header(&opts, patch_no_width,
 					   &buf, &dashes, NULL, b_util);
 			b_util = ++j < b->nr ? b->items[j].util : NULL;
 		}
@@ -506,22 +529,18 @@ static void output(struct string_list *a, struct string_list *b,
 		/* Show matching LHS/RHS pair. */
 		if (j < b->nr) {
 			a_util = a->items[b_util->matching].util;
-			output_pair_header(diffopt, patch_no_width,
+			output_pair_header(&opts, patch_no_width,
 					   &buf, &dashes, a_util, b_util);
-			if (!(diffopt->output_format & DIFF_FORMAT_NO_OUTPUT))
+			if (!(opts.output_format & DIFF_FORMAT_NO_OUTPUT))
 				patch_diff(a->items[b_util->matching].string,
-					   b->items[j].string, diffopt);
+					   b->items[j].string, &opts);
 			a_util->shown = 1;
 			j++;
 		}
 	}
 	strbuf_release(&buf);
 	strbuf_release(&dashes);
-}
-
-static struct strbuf *output_prefix_cb(struct diff_options *opt, void *data)
-{
-	return data;
+	strbuf_release(&indent);
 }
 
 int show_range_diff(const char *range1, const char *range2,
@@ -538,31 +557,10 @@ int show_range_diff(const char *range1, const char *range2,
 		res = error(_("could not parse log for '%s'"), range2);
 
 	if (!res) {
-		struct diff_options opts;
-		struct strbuf indent = STRBUF_INIT;
-
-		if (range_diff_opts->diffopt)
-			memcpy(&opts, range_diff_opts->diffopt, sizeof(opts));
-		else
-			diff_setup(&opts);
-
-		if (!opts.output_format)
-			opts.output_format = DIFF_FORMAT_PATCH;
-		opts.flags.suppress_diff_headers = 1;
-		opts.flags.dual_color_diffed_diffs =
-			range_diff_opts->dual_color;
-		opts.flags.suppress_hunk_header_line_count = 1;
-		opts.output_prefix = output_prefix_cb;
-		strbuf_addstr(&indent, "    ");
-		opts.output_prefix_data = &indent;
-		diff_setup_done(&opts);
-
 		find_exact_matches(&branch1, &branch2);
 		get_correspondences(&branch1, &branch2,
 				    range_diff_opts->creation_factor);
-		output(&branch1, &branch2, &opts);
-
-		strbuf_release(&indent);
+		output(&branch1, &branch2, range_diff_opts);
 	}
 
 	string_list_clear(&branch1, 1);
-- 
gitgitgadget

[PATCH 6/6] range-diff: offer --left-only/--right-only options

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

When comparing commit ranges, one is frequently interested only in one
side, such as asking the question "Has this patch that I submitted to
the Git mailing list been applied?": one would only care about the part
of the output that corresponds to the commits in a local branch.

To make that possible, imitate the `git rev-list` options `--left-only`
and `--right-only`.

This addresses https://github.com/gitgitgadget/git/issues/206

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/git-range-diff.txt |  9 +++++++++
 builtin/range-diff.c             |  8 +++++++-
 range-diff.c                     | 11 ++++++++---
 range-diff.h                     |  1 +
 t/t3206-range-diff.sh            | 15 +++++++++++++++
 5 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt
index 14bffb272a06..e3bd93515819 100644
--- a/Documentation/git-range-diff.txt
+++ b/Documentation/git-range-diff.txt
@@ -10,6 +10,7 @@ SYNOPSIS
 [verse]
 'git range-diff' [--color=[<when>]] [--no-color] [<diff-options>]
 	[--no-dual-color] [--creation-factor=<factor>]
+	[--left-only | --right-only]
 	( <range1> <range2> | <rev1>...<rev2> | <base> <rev1> <rev2> )
 
 DESCRIPTION
@@ -69,6 +70,14 @@ to revert to color all lines according to the outer diff markers
 	See the ``Algorithm`` section below for an explanation why this is
 	needed.
 
+--left-only::
+	Suppress commits that are missing from the first specified range
+	(or the "left range" when using the `<rev1>...<rev2>` format).
+
+--right-only::
+	Suppress commits that are missing from the second specified range
+	(or the "right range" when using the `<rev1>...<rev2>` format).
+
 --[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 80fcdc6ad42d..78bc9fa77062 100644
--- a/builtin/range-diff.c
+++ b/builtin/range-diff.c
@@ -21,7 +21,7 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)
 		.diffopt = &diffopt,
 		.other_arg = &other_arg
 	};
-	int simple_color = -1;
+	int simple_color = -1, left_only = 0, right_only = 0;
 	struct option range_diff_options[] = {
 		OPT_INTEGER(0, "creation-factor",
 			    &range_diff_opts.creation_factor,
@@ -31,6 +31,10 @@ 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_BOOL(0, "left-only", &left_only,
+			 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_END()
 	};
 	struct option *options;
@@ -88,6 +92,8 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)
 	FREE_AND_NULL(options);
 
 	range_diff_opts.dual_color = simple_color < 1;
+	range_diff_opts.left_only = left_only;
+	range_diff_opts.right_only = right_only;
 	res = show_range_diff(range1.buf, range2.buf, &range_diff_opts);
 
 	strvec_clear(&other_arg);
diff --git a/range-diff.c b/range-diff.c
index 5455cbb9521b..6f52dce0f6b6 100644
--- a/range-diff.c
+++ b/range-diff.c
@@ -513,7 +513,8 @@ static void output(struct string_list *a, struct string_list *b,
 
 		/* Show unmatched LHS commit whose predecessors were shown. */
 		if (i < a->nr && a_util->matching < 0) {
-			output_pair_header(&opts, patch_no_width,
+			if (!range_diff_opts->right_only)
+				output_pair_header(&opts, patch_no_width,
 					   &buf, &dashes, a_util, NULL);
 			i++;
 			continue;
@@ -521,7 +522,8 @@ static void output(struct string_list *a, struct string_list *b,
 
 		/* Show unmatched RHS commits. */
 		while (j < b->nr && b_util->matching < 0) {
-			output_pair_header(&opts, patch_no_width,
+			if (!range_diff_opts->left_only)
+				output_pair_header(&opts, patch_no_width,
 					   &buf, &dashes, NULL, b_util);
 			b_util = ++j < b->nr ? b->items[j].util : NULL;
 		}
@@ -551,7 +553,10 @@ 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;
 
-	if (read_patches(range1, &branch1, range_diff_opts->other_arg))
+	if (range_diff_opts->left_only && range_diff_opts->right_only)
+		res = error(_("--left-only and --right-only are mutually exclusive"));
+
+	if (!res && read_patches(range1, &branch1, range_diff_opts->other_arg))
 		res = error(_("could not parse log for '%s'"), range1);
 	if (!res && read_patches(range2, &branch2, range_diff_opts->other_arg))
 		res = error(_("could not parse log for '%s'"), range2);
diff --git a/range-diff.h b/range-diff.h
index 8fb2ff05865d..66aacd47d149 100644
--- a/range-diff.h
+++ b/range-diff.h
@@ -9,6 +9,7 @@
 struct range_diff_options {
 	int creation_factor;
 	unsigned dual_color:1;
+	unsigned left_only:1, right_only:1;
 	const struct diff_options *diffopt;
 	const struct strvec *other_arg;
 };
diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
index e217cecac9ed..41cc1077a18f 100755
--- a/t/t3206-range-diff.sh
+++ b/t/t3206-range-diff.sh
@@ -725,4 +725,19 @@ test_expect_success 'format-patch --range-diff with multiple notes' '
 	test_cmp expect actual
 '
 
+test_expect_success '--left-only/--right-only' '
+	git switch --orphan left-right &&
+	test_commit first &&
+	test_commit unmatched &&
+	test_commit common &&
+	git switch -C left-right first &&
+	git cherry-pick common &&
+
+	git range-diff -s --left-only ...common >actual &&
+	head_oid=$(git rev-parse --short HEAD) &&
+	common_oid=$(git rev-parse --short common) &&
+	echo "1:  $head_oid = 2:  $common_oid common" >expect &&
+	test_cmp expect actual
+'
+
 test_done
-- 
gitgitgadget

Re: [PATCH 0/6] Optionally restrict range-diff output to "left" or "right" range only

[Junio C Hamano]

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> One of my quite common workflows is to see whether an ancient topic branch I
> have lying about has made it into Git. Since my local commit OIDs have
> nothing to do with the OIDs of the corresponding commits in git/git, my only
> way is to fire up git range-diff ...upstream/master, but of course that
> output contains way more commits than I care about.
>
> To help this use case, here is a patch series that teaches git range-diff
> the --left-only and --right-only options in the end, restricting the output
> to those commits and commit pairs that correspond to the commits in the
> first and the second range, respectively.

Makes sense.

Re: [PATCH 0/6] Optionally restrict range-diff output to "left" or "right" range only

[Taylor Blau]

On Thu, Feb 04, 2021 at 02:41:39PM -0800, Junio C Hamano wrote:
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > One of my quite common workflows is to see whether an ancient topic branch I
> > have lying about has made it into Git. Since my local commit OIDs have
> > nothing to do with the OIDs of the corresponding commits in git/git, my only
> > way is to fire up git range-diff ...upstream/master, but of course that
> > output contains way more commits than I care about.
> >
> > To help this use case, here is a patch series that teaches git range-diff
> > the --left-only and --right-only options in the end, restricting the output
> > to those commits and commit pairs that correspond to the commits in the
> > first and the second range, respectively.
>
> Makes sense.

I'd add an additional use-case, which is ignoring new commits from
upstream when displaying a range-diff in rerolled patch series.

Oftentimes I'll find that the automatically-prepared range diff that
'git format-patch --cover-letter --range-diff' generates will include
new commits from upstream, so these new options should help me ignore
those in the output.

As an aside: I am curious if I'm missing something when you say the
"only way" is to ask for a 'git range-diff ...@{u}'. IIUC what you're
describing, I often resort to using 'git cherry' for that exact thing.
But, I may not be quite understanding your use-case (and why git-cherry
doesn't do what you want already).

My latter question is purely for satisfying my own curiosity; I don't
have any problem with a '--{left,right}-only' option in range-diff. From
my quick read of the patches, it all looks pretty sane to me.


Thanks,
Taylor

Re: [PATCH 0/6] Optionally restrict range-diff output to "left" or "right" range only

[Junio C Hamano]

Taylor Blau <me@ttaylorr.com> writes:

> On Thu, Feb 04, 2021 at 02:41:39PM -0800, Junio C Hamano wrote:
>> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
>> writes:
>>
>> > One of my quite common workflows is to see whether an ancient topic branch I
>> > have lying about has made it into Git. Since my local commit OIDs have
>> > nothing to do with the OIDs of the corresponding commits in git/git, my only
>> > way is to fire up git range-diff ...upstream/master, but of course that
>> > output contains way more commits than I care about.
>> > ...
>> Makes sense.
>
> I'd add an additional use-case, which is ignoring new commits from
> upstream when displaying a range-diff in rerolled patch series.
>
> Oftentimes I'll find that the automatically-prepared range diff that
> 'git format-patch --cover-letter --range-diff' generates will include
> new commits from upstream, so these new options should help me ignore
> those in the output.

Do you mean that the new round is based on an updated upstream
commit, while the old series was based on a bit older upstream?
After rebasing your topic, "range-diff @{1}..." would find the
updates in the base (made in the upstream) plus the new round of
your work on the right hand side of the symmetric range, while the
left hand side solely consists of your old round (unless the
upstream rewound their work, which should not happen).  But that
must not be it, I guess, because in such a case, among the commits
in @{1}..HEAD, we cannot (eh, at least range-diff cannot) tell which
one came from upstream and which one came from our fingers.

So I am a bit puzzled there.

> As an aside: I am curious if I'm missing something when you say the
> "only way" is to ask for a 'git range-diff ...@{u}'. IIUC what you're
> describing, I often resort to using 'git cherry' for that exact thing.
> But, I may not be quite understanding your use-case (and why git-cherry
> doesn't do what you want already).
>
> My latter question is purely for satisfying my own curiosity; I don't
> have any problem with a '--{left,right}-only' option in range-diff. From
> my quick read of the patches, it all looks pretty sane to me.

The question is addressed to Dscho, and I am also somewhat curious.
Perhaps the reason would be that the output from cherry is not as
easy to read as range-diff, without any post-processing.

I do find "range-diff ...@{u}" a bit too blunt and heavy a hammer
for that task, but as they say, when you are familiar with and fond
of a hammer, all tasks look like nails ;-).

Re: [PATCH 0/6] Optionally restrict range-diff output to "left" or "right" range only

[Jeff King]

On Thu, Feb 04, 2021 at 04:56:16PM -0800, Junio C Hamano wrote:

> > As an aside: I am curious if I'm missing something when you say the
> > "only way" is to ask for a 'git range-diff ...@{u}'. IIUC what you're
> > describing, I often resort to using 'git cherry' for that exact thing.
> > But, I may not be quite understanding your use-case (and why git-cherry
> > doesn't do what you want already).
> >
> > My latter question is purely for satisfying my own curiosity; I don't
> > have any problem with a '--{left,right}-only' option in range-diff. From
> > my quick read of the patches, it all looks pretty sane to me.
> 
> The question is addressed to Dscho, and I am also somewhat curious.
> Perhaps the reason would be that the output from cherry is not as
> easy to read as range-diff, without any post-processing.

I had the same curiosity; I'd use git-cherry (or rev-list --cherry) for
this.

I suspect the big difference is the quality of the matching. git-cherry
is purely looking at patch-ids. So it is quite likely to say "this was
not applied upstream" when what got applied differed slightly (e.g.,
fixups upstream, applied to a different base, etc). Whereas range-diff
has some cost heuristics for deciding that two patches are basically the
same thing.  So it would find more cases (and as a bonus, give you the
diff to see what tweaks were made upstream).

It does make me wonder if it would be useful for rev-list, etc to have
an option to make "--cherry" use the more clever heuristics instead of
just a patch-id. It would never show the same diff output as range-diff,
but maybe more scripts would find the advanced heuristic useful.

I know it would probably make rebase's "ignore if in upstream" feature
less clunky when I rebase topics. But it would also make it more
dangerous! E.g., right now I see any upstream tweaks as potential
conflicts when I rebase, and I manually review them for sanity.

-Peff

Re: [PATCH 0/6] Optionally restrict range-diff output to "left" or "right" range only

[Johannes Schindelin]

Hi Peff,

On Fri, 5 Feb 2021, Jeff King wrote:

> On Thu, Feb 04, 2021 at 04:56:16PM -0800, Junio C Hamano wrote:
>
> > > As an aside: I am curious if I'm missing something when you say the
> > > "only way" is to ask for a 'git range-diff ...@{u}'. IIUC what you're
> > > describing, I often resort to using 'git cherry' for that exact thing.
> > > But, I may not be quite understanding your use-case (and why git-cherry
> > > doesn't do what you want already).
> > >
> > > My latter question is purely for satisfying my own curiosity; I don't
> > > have any problem with a '--{left,right}-only' option in range-diff. From
> > > my quick read of the patches, it all looks pretty sane to me.
> >
> > The question is addressed to Dscho, and I am also somewhat curious.
> > Perhaps the reason would be that the output from cherry is not as
> > easy to read as range-diff, without any post-processing.
>
> I had the same curiosity; I'd use git-cherry (or rev-list --cherry) for
> this.
>
> I suspect the big difference is the quality of the matching. git-cherry
> is purely looking at patch-ids.

Indeed. Whenever I had tried `git cherry` in the past (which, admittedly,
has been with geometrically decreasing frequency given the results), it
completely failed to help me. And it's not only its reliance on perfect
matches of the diff _with context lines_, it is also that the commit
messages are completely ignored.

`git cherry`'s track record with me is so perfect that I want to put this
line into all my Bash profiles:

	eval "$(set | sed -n '/^__git_main /,/^}$/{s/--list-cmds=list-mainporcelain[^)]*/& | grep -v ^cherry\$/;p}')"

> So it is quite likely to say "this was not applied upstream" when what
> got applied differed slightly (e.g., fixups upstream, applied to a
> different base, etc). Whereas range-diff has some cost heuristics for
> deciding that two patches are basically the same thing.  So it would
> find more cases (and as a bonus, give you the diff to see what tweaks
> were made upstream).
>
> It does make me wonder if it would be useful for rev-list, etc to have
> an option to make "--cherry" use the more clever heuristics instead of
> just a patch-id. It would never show the same diff output as range-diff,
> but maybe more scripts would find the advanced heuristic useful.
>
> I know it would probably make rebase's "ignore if in upstream" feature
> less clunky when I rebase topics. But it would also make it more
> dangerous! E.g., right now I see any upstream tweaks as potential
> conflicts when I rebase, and I manually review them for sanity.

Yeah, I thought the same when I read the paragraphs before this one. It
might sound convenient, but there _are_ false positives in `git
range-diff`'s output, therefore I would recommend never using
`git range-diff --left-only` or `[...] --right-only` with `-s`. IOW
_always_ inspect the differences.

Ciao,
Dscho

Re: [PATCH 0/6] Optionally restrict range-diff output to "left" or "right" range only

[Taylor Blau]

On Thu, Feb 04, 2021 at 04:56:16PM -0800, Junio C Hamano wrote:
> > I'd add an additional use-case, which is ignoring new commits from
> > upstream when displaying a range-diff in rerolled patch series.
> >
> > Oftentimes I'll find that the automatically-prepared range diff that
> > 'git format-patch --cover-letter --range-diff' generates will include
> > new commits from upstream, so these new options should help me ignore
> > those in the output.
>
> Do you mean that the new round is based on an updated upstream
> commit, while the old series was based on a bit older upstream?
> After rebasing your topic, "range-diff @{1}..." would find the
> updates in the base (made in the upstream) plus the new round of
> your work on the right hand side of the symmetric range, while the
> left hand side solely consists of your old round (unless the
> upstream rewound their work, which should not happen).  But that
> must not be it, I guess, because in such a case, among the commits
> in @{1}..HEAD, we cannot (eh, at least range-diff cannot) tell which
> one came from upstream and which one came from our fingers.
>
> So I am a bit puzzled there.

I'm talking about a situation where a later re-roll is based of of a
newer upstream. But your judgement is right: upstream's updates look
like "new" commits on the right-hand side.

I have some scripts built around this, but they all boil down to passing
'--range-diff=@{1}' (where @{1} is the tip of the previous reroll) to
format-patch. See:

    https://github.com/ttaylorr/dotfiles/blob/work-gh/bin/git-mail#L8-L10

for details.

IIUC this series, I think I'd also want to start passing '--left-only'
to ignore the new commits from upstream in a range-diff, no?

Thanks,
Taylor

[PATCH v2 0/6] Optionally restrict range-diff output to "left" or "right" range only

[Johannes Schindelin via GitGitGadget]

One of my quite common workflows is to see whether an ancient topic branch I
have lying about has made it into Git. Since my local commit OIDs have
nothing to do with the OIDs of the corresponding commits in git/git, my only
way is to fire up git range-diff ...upstream/master, but of course that
output contains way more commits than I care about.

To help this use case, here is a patch series that teaches git range-diff
the --left-only and --right-only options in the end, restricting the output
to those commits and commit pairs that correspond to the commits in the
first and the second range, respectively.

The first part of the series contains cleanup patches that are not strictly
related to the feature I implemented here, but since I already have them, I
figured I could just as well contribute them all together.

This patch series is based on js/range-diff-wo-dotdot.

Changes since v1:

 * Adjusted the comment above show_range_diff() to reflect the new
   signature.

Johannes Schindelin (6):
  range-diff: avoid leaking memory in two error code paths
  range-diff: libify the read_patches() function again
  range-diff: simplify code spawning `git log`
  range-diff: combine all options in a single data structure
  range-diff: move the diffopt initialization down one layer
  range-diff: offer --left-only/--right-only options

 Documentation/git-range-diff.txt |   9 +++
 builtin/log.c                    |  10 ++-
 builtin/range-diff.c             |  21 +++++--
 log-tree.c                       |   8 ++-
 range-diff.c                     | 101 +++++++++++++++++--------------
 range-diff.h                     |  17 ++++--
 t/t3206-range-diff.sh            |  15 +++++
 7 files changed, 120 insertions(+), 61 deletions(-)


base-commit: 35fac18ea5ccab7524501bcb4542473ef8195541
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-869%2Fdscho%2Frange-diff-left-and-right-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-869/dscho/range-diff-left-and-right-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/869

Range-diff vs v1:

 1:  5f2f06b79e20 = 1:  15f4f6abdaa2 range-diff: avoid leaking memory in two error code paths
 2:  0d25dd2a9c2d = 2:  99e466ef32d7 range-diff: libify the read_patches() function again
 3:  d6c582c25df0 = 3:  a36631b663e2 range-diff: simplify code spawning `git log`
 4:  b620be042eb3 ! 4:  7367eadfe243 range-diff: combine all options in a single data structure
     @@ range-diff.h
      +struct range_diff_options {
      +	int creation_factor;
      +	unsigned dual_color:1;
     -+	const struct diff_options *diffopt;
     -+	const struct strvec *other_arg;
     ++	const struct diff_options *diffopt; /* may be NULL */
     ++	const struct strvec *other_arg; /* may be NULL */
      +};
      +
       /*
     -  * Compare series of commits in RANGE1 and RANGE2, and emit to the
     -  * standard output.  NULL can be passed to DIFFOPT to use the built-in
     -  * default.
     +- * Compare series of commits in RANGE1 and RANGE2, and emit to the
     +- * standard output.  NULL can be passed to DIFFOPT to use the built-in
     +- * default.
     ++ * Compare series of commits in `range1` and `range2`, and emit to the
     ++ * standard output.
        */
       int show_range_diff(const char *range1, const char *range2,
      -		    int creation_factor, int dual_color,
 5:  9fa945db5f13 = 5:  a6285292b4f1 range-diff: move the diffopt initialization down one layer
 6:  1c599abdbb6f ! 6:  8357d3c94f17 range-diff: offer --left-only/--right-only options
     @@ range-diff.h
       	int creation_factor;
       	unsigned dual_color:1;
      +	unsigned left_only:1, right_only:1;
     - 	const struct diff_options *diffopt;
     - 	const struct strvec *other_arg;
     + 	const struct diff_options *diffopt; /* may be NULL */
     + 	const struct strvec *other_arg; /* may be NULL */
       };
      
       ## t/t3206-range-diff.sh ##

-- 
gitgitgadget

[PATCH v2 1/6] range-diff: avoid leaking memory in two error code paths

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

In the code paths in question, we already release a lot of memory, but
the `current_filename` variable was missed. Fix that.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 range-diff.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/range-diff.c b/range-diff.c
index 7a38dc871543..9972808fe4cf 100644
--- a/range-diff.c
+++ b/range-diff.c
@@ -98,6 +98,7 @@ static int read_patches(const char *range, struct string_list *list,
 			if (get_oid(p, &util->oid)) {
 				error(_("could not parse commit '%s'"), p);
 				free(util);
+				free(current_filename);
 				string_list_clear(list, 1);
 				strbuf_release(&buf);
 				strbuf_release(&contents);
@@ -113,6 +114,7 @@ static int read_patches(const char *range, struct string_list *list,
 			error(_("could not parse first line of `log` output: "
 				"did not start with 'commit ': '%s'"),
 			      line);
+			free(current_filename);
 			string_list_clear(list, 1);
 			strbuf_release(&buf);
 			strbuf_release(&contents);
-- 
gitgitgadget

[PATCH v2 2/6] range-diff: libify the read_patches() function again

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

In library functions, we do want to avoid the (simple, but rather final)
`die()` calls, instead returning with a value indicating an error.

Let's do exactly that in the code introduced in b66885a30cb8
(range-diff: add section header instead of diff header, 2019-07-11) that
wants to error out if a diff header could not be parsed.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 range-diff.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/range-diff.c b/range-diff.c
index 9972808fe4cf..8844359d416f 100644
--- a/range-diff.c
+++ b/range-diff.c
@@ -136,9 +136,16 @@ static int read_patches(const char *range, struct string_list *list,
 			orig_len = len;
 			len = parse_git_diff_header(&root, &linenr, 0, line,
 						    len, size, &patch);
-			if (len < 0)
-				die(_("could not parse git header '%.*s'"),
-				    orig_len, line);
+			if (len < 0) {
+				error(_("could not parse git header '%.*s'"),
+				      orig_len, line);
+				free(util);
+				free(current_filename);
+				string_list_clear(list, 1);
+				strbuf_release(&buf);
+				strbuf_release(&contents);
+				return -1;
+			}
 			strbuf_addstr(&buf, " ## ");
 			if (patch.is_new > 0)
 				strbuf_addf(&buf, "%s (new)", patch.new_name);
-- 
gitgitgadget

[PATCH v2 3/6] range-diff: simplify code spawning `git log`

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

Previously, we waited for the child process to be finished in every
failing code path as well as at the end of the function
`show_range_diff()`.

However, we do not need to wait that long. Directly after reading the
output of the child process, we can wrap up the child process.

This also has the advantage that we don't do a bunch of unnecessary work
in case `finish_command()` returns with an error anyway.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 range-diff.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/range-diff.c b/range-diff.c
index 8844359d416f..d0d941a25add 100644
--- a/range-diff.c
+++ b/range-diff.c
@@ -81,6 +81,8 @@ static int read_patches(const char *range, struct string_list *list,
 		finish_command(&cp);
 		return -1;
 	}
+	if (finish_command(&cp))
+		return -1;
 
 	line = contents.buf;
 	size = contents.len;
@@ -102,7 +104,6 @@ static int read_patches(const char *range, struct string_list *list,
 				string_list_clear(list, 1);
 				strbuf_release(&buf);
 				strbuf_release(&contents);
-				finish_command(&cp);
 				return -1;
 			}
 			util->matching = -1;
@@ -118,7 +119,6 @@ static int read_patches(const char *range, struct string_list *list,
 			string_list_clear(list, 1);
 			strbuf_release(&buf);
 			strbuf_release(&contents);
-			finish_command(&cp);
 			return -1;
 		}
 
@@ -228,9 +228,6 @@ static int read_patches(const char *range, struct string_list *list,
 	strbuf_release(&buf);
 	free(current_filename);
 
-	if (finish_command(&cp))
-		return -1;
-
 	return 0;
 }
 
-- 
gitgitgadget

[PATCH v2 4/6] range-diff: combine all options in a single data structure

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

This will make it easier to implement the `--left-only` and
`--right-only` options.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin/log.c        | 10 ++++++++--
 builtin/range-diff.c | 13 +++++++++----
 log-tree.c           |  8 ++++++--
 range-diff.c         | 18 +++++++++---------
 range-diff.h         | 16 ++++++++++------
 5 files changed, 42 insertions(+), 23 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 91466c059c79..a06e5385689b 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1231,14 +1231,20 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
 		 */
 		struct diff_options opts;
 		struct strvec other_arg = STRVEC_INIT;
+		struct range_diff_options range_diff_opts = {
+			.creation_factor = rev->creation_factor,
+			.dual_color = 1,
+			.diffopt = &opts,
+			.other_arg = &other_arg
+		};
+
 		diff_setup(&opts);
 		opts.file = rev->diffopt.file;
 		opts.use_color = rev->diffopt.use_color;
 		diff_setup_done(&opts);
 		fprintf_ln(rev->diffopt.file, "%s", rev->rdiff_title);
 		get_notes_args(&other_arg, rev);
-		show_range_diff(rev->rdiff1, rev->rdiff2,
-				rev->creation_factor, 1, &opts, &other_arg);
+		show_range_diff(rev->rdiff1, rev->rdiff2, &range_diff_opts);
 		strvec_clear(&other_arg);
 	}
 }
diff --git a/builtin/range-diff.c b/builtin/range-diff.c
index 5b1f6326322f..80fcdc6ad42d 100644
--- a/builtin/range-diff.c
+++ b/builtin/range-diff.c
@@ -14,12 +14,17 @@ NULL
 
 int cmd_range_diff(int argc, const char **argv, const char *prefix)
 {
-	int creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;
 	struct diff_options diffopt = { NULL };
 	struct strvec other_arg = STRVEC_INIT;
+	struct range_diff_options range_diff_opts = {
+		.creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT,
+		.diffopt = &diffopt,
+		.other_arg = &other_arg
+	};
 	int simple_color = -1;
 	struct option range_diff_options[] = {
-		OPT_INTEGER(0, "creation-factor", &creation_factor,
+		OPT_INTEGER(0, "creation-factor",
+			    &range_diff_opts.creation_factor,
 			    N_("Percentage by which creation is weighted")),
 		OPT_BOOL(0, "no-dual-color", &simple_color,
 			    N_("use simple diff colors")),
@@ -82,8 +87,8 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)
 	}
 	FREE_AND_NULL(options);
 
-	res = show_range_diff(range1.buf, range2.buf, creation_factor,
-			      simple_color < 1, &diffopt, &other_arg);
+	range_diff_opts.dual_color = simple_color < 1;
+	res = show_range_diff(range1.buf, range2.buf, &range_diff_opts);
 
 	strvec_clear(&other_arg);
 	strbuf_release(&range1);
diff --git a/log-tree.c b/log-tree.c
index fd0dde97ec32..eeacba15dc94 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -808,6 +808,11 @@ void show_log(struct rev_info *opt)
 	if (cmit_fmt_is_mail(ctx.fmt) && opt->rdiff1) {
 		struct diff_queue_struct dq;
 		struct diff_options opts;
+		struct range_diff_options range_diff_opts = {
+			.creation_factor = opt->creation_factor,
+			.dual_color = 1,
+			.diffopt = &opts
+		};
 
 		memcpy(&dq, &diff_queued_diff, sizeof(diff_queued_diff));
 		DIFF_QUEUE_CLEAR(&diff_queued_diff);
@@ -822,8 +827,7 @@ void show_log(struct rev_info *opt)
 		opts.file = opt->diffopt.file;
 		opts.use_color = opt->diffopt.use_color;
 		diff_setup_done(&opts);
-		show_range_diff(opt->rdiff1, opt->rdiff2,
-				opt->creation_factor, 1, &opts, NULL);
+		show_range_diff(opt->rdiff1, opt->rdiff2, &range_diff_opts);
 
 		memcpy(&diff_queued_diff, &dq, sizeof(diff_queued_diff));
 	}
diff --git a/range-diff.c b/range-diff.c
index d0d941a25add..25d4c244799c 100644
--- a/range-diff.c
+++ b/range-diff.c
@@ -526,33 +526,32 @@ static struct strbuf *output_prefix_cb(struct diff_options *opt, void *data)
 }
 
 int show_range_diff(const char *range1, const char *range2,
-		    int creation_factor, int dual_color,
-		    const struct diff_options *diffopt,
-		    const struct strvec *other_arg)
+		    struct range_diff_options *range_diff_opts)
 {
 	int res = 0;
 
 	struct string_list branch1 = STRING_LIST_INIT_DUP;
 	struct string_list branch2 = STRING_LIST_INIT_DUP;
 
-	if (read_patches(range1, &branch1, other_arg))
+	if (read_patches(range1, &branch1, range_diff_opts->other_arg))
 		res = error(_("could not parse log for '%s'"), range1);
-	if (!res && read_patches(range2, &branch2, other_arg))
+	if (!res && read_patches(range2, &branch2, range_diff_opts->other_arg))
 		res = error(_("could not parse log for '%s'"), range2);
 
 	if (!res) {
 		struct diff_options opts;
 		struct strbuf indent = STRBUF_INIT;
 
-		if (diffopt)
-			memcpy(&opts, diffopt, sizeof(opts));
+		if (range_diff_opts->diffopt)
+			memcpy(&opts, range_diff_opts->diffopt, sizeof(opts));
 		else
 			diff_setup(&opts);
 
 		if (!opts.output_format)
 			opts.output_format = DIFF_FORMAT_PATCH;
 		opts.flags.suppress_diff_headers = 1;
-		opts.flags.dual_color_diffed_diffs = dual_color;
+		opts.flags.dual_color_diffed_diffs =
+			range_diff_opts->dual_color;
 		opts.flags.suppress_hunk_header_line_count = 1;
 		opts.output_prefix = output_prefix_cb;
 		strbuf_addstr(&indent, "    ");
@@ -560,7 +559,8 @@ int show_range_diff(const char *range1, const char *range2,
 		diff_setup_done(&opts);
 
 		find_exact_matches(&branch1, &branch2);
-		get_correspondences(&branch1, &branch2, creation_factor);
+		get_correspondences(&branch1, &branch2,
+				    range_diff_opts->creation_factor);
 		output(&branch1, &branch2, &opts);
 
 		strbuf_release(&indent);
diff --git a/range-diff.h b/range-diff.h
index 4abd70c40fed..a595f4e8db2d 100644
--- a/range-diff.h
+++ b/range-diff.h
@@ -6,15 +6,19 @@
 
 #define RANGE_DIFF_CREATION_FACTOR_DEFAULT 60
 
+struct range_diff_options {
+	int creation_factor;
+	unsigned dual_color:1;
+	const struct diff_options *diffopt; /* may be NULL */
+	const struct strvec *other_arg; /* may be NULL */
+};
+
 /*
- * Compare series of commits in RANGE1 and RANGE2, and emit to the
- * standard output.  NULL can be passed to DIFFOPT to use the built-in
- * default.
+ * Compare series of commits in `range1` and `range2`, and emit to the
+ * standard output.
  */
 int show_range_diff(const char *range1, const char *range2,
-		    int creation_factor, int dual_color,
-		    const struct diff_options *diffopt,
-		    const struct strvec *other_arg);
+		    struct range_diff_options *opts);
 
 /*
  * Determine whether the given argument is usable as a range argument of `git
-- 
gitgitgadget

[PATCH v2 5/6] range-diff: move the diffopt initialization down one layer

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

It is actually only the `output()` function that uses those diffopts. By
moving the diffopt initialization down into that function, it is
encapsulated better.

Incidentally, it will also make it easier to implement the `--left-only`
and `--right-only` options in `git range-diff` because the `output()`
function is now receiving all range-diff options as a parameter, not
just the diffopts.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 range-diff.c | 64 +++++++++++++++++++++++++---------------------------
 1 file changed, 31 insertions(+), 33 deletions(-)

diff --git a/range-diff.c b/range-diff.c
index 25d4c244799c..001b6e174079 100644
--- a/range-diff.c
+++ b/range-diff.c
@@ -465,12 +465,35 @@ static void patch_diff(const char *a, const char *b,
 	diff_flush(diffopt);
 }
 
+static struct strbuf *output_prefix_cb(struct diff_options *opt, void *data)
+{
+	return data;
+}
+
 static void output(struct string_list *a, struct string_list *b,
-		   struct diff_options *diffopt)
+		   struct range_diff_options *range_diff_opts)
 {
 	struct strbuf buf = STRBUF_INIT, dashes = STRBUF_INIT;
 	int patch_no_width = decimal_width(1 + (a->nr > b->nr ? a->nr : b->nr));
 	int i = 0, j = 0;
+	struct diff_options opts;
+	struct strbuf indent = STRBUF_INIT;
+
+	if (range_diff_opts->diffopt)
+		memcpy(&opts, range_diff_opts->diffopt, sizeof(opts));
+	else
+		diff_setup(&opts);
+
+	if (!opts.output_format)
+		opts.output_format = DIFF_FORMAT_PATCH;
+	opts.flags.suppress_diff_headers = 1;
+	opts.flags.dual_color_diffed_diffs =
+		range_diff_opts->dual_color;
+	opts.flags.suppress_hunk_header_line_count = 1;
+	opts.output_prefix = output_prefix_cb;
+	strbuf_addstr(&indent, "    ");
+	opts.output_prefix_data = &indent;
+	diff_setup_done(&opts);
 
 	/*
 	 * We assume the user is really more interested in the second argument
@@ -491,7 +514,7 @@ static void output(struct string_list *a, struct string_list *b,
 
 		/* Show unmatched LHS commit whose predecessors were shown. */
 		if (i < a->nr && a_util->matching < 0) {
-			output_pair_header(diffopt, patch_no_width,
+			output_pair_header(&opts, patch_no_width,
 					   &buf, &dashes, a_util, NULL);
 			i++;
 			continue;
@@ -499,7 +522,7 @@ static void output(struct string_list *a, struct string_list *b,
 
 		/* Show unmatched RHS commits. */
 		while (j < b->nr && b_util->matching < 0) {
-			output_pair_header(diffopt, patch_no_width,
+			output_pair_header(&opts, patch_no_width,
 					   &buf, &dashes, NULL, b_util);
 			b_util = ++j < b->nr ? b->items[j].util : NULL;
 		}
@@ -507,22 +530,18 @@ static void output(struct string_list *a, struct string_list *b,
 		/* Show matching LHS/RHS pair. */
 		if (j < b->nr) {
 			a_util = a->items[b_util->matching].util;
-			output_pair_header(diffopt, patch_no_width,
+			output_pair_header(&opts, patch_no_width,
 					   &buf, &dashes, a_util, b_util);
-			if (!(diffopt->output_format & DIFF_FORMAT_NO_OUTPUT))
+			if (!(opts.output_format & DIFF_FORMAT_NO_OUTPUT))
 				patch_diff(a->items[b_util->matching].string,
-					   b->items[j].string, diffopt);
+					   b->items[j].string, &opts);
 			a_util->shown = 1;
 			j++;
 		}
 	}
 	strbuf_release(&buf);
 	strbuf_release(&dashes);
-}
-
-static struct strbuf *output_prefix_cb(struct diff_options *opt, void *data)
-{
-	return data;
+	strbuf_release(&indent);
 }
 
 int show_range_diff(const char *range1, const char *range2,
@@ -539,31 +558,10 @@ int show_range_diff(const char *range1, const char *range2,
 		res = error(_("could not parse log for '%s'"), range2);
 
 	if (!res) {
-		struct diff_options opts;
-		struct strbuf indent = STRBUF_INIT;
-
-		if (range_diff_opts->diffopt)
-			memcpy(&opts, range_diff_opts->diffopt, sizeof(opts));
-		else
-			diff_setup(&opts);
-
-		if (!opts.output_format)
-			opts.output_format = DIFF_FORMAT_PATCH;
-		opts.flags.suppress_diff_headers = 1;
-		opts.flags.dual_color_diffed_diffs =
-			range_diff_opts->dual_color;
-		opts.flags.suppress_hunk_header_line_count = 1;
-		opts.output_prefix = output_prefix_cb;
-		strbuf_addstr(&indent, "    ");
-		opts.output_prefix_data = &indent;
-		diff_setup_done(&opts);
-
 		find_exact_matches(&branch1, &branch2);
 		get_correspondences(&branch1, &branch2,
 				    range_diff_opts->creation_factor);
-		output(&branch1, &branch2, &opts);
-
-		strbuf_release(&indent);
+		output(&branch1, &branch2, range_diff_opts);
 	}
 
 	string_list_clear(&branch1, 1);
-- 
gitgitgadget

[PATCH v2 6/6] range-diff: offer --left-only/--right-only options

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

When comparing commit ranges, one is frequently interested only in one
side, such as asking the question "Has this patch that I submitted to
the Git mailing list been applied?": one would only care about the part
of the output that corresponds to the commits in a local branch.

To make that possible, imitate the `git rev-list` options `--left-only`
and `--right-only`.

This addresses https://github.com/gitgitgadget/git/issues/206

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/git-range-diff.txt |  9 +++++++++
 builtin/range-diff.c             |  8 +++++++-
 range-diff.c                     | 11 ++++++++---
 range-diff.h                     |  1 +
 t/t3206-range-diff.sh            | 15 +++++++++++++++
 5 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt
index a968d5237dae..fe350d7f4056 100644
--- a/Documentation/git-range-diff.txt
+++ b/Documentation/git-range-diff.txt
@@ -10,6 +10,7 @@ SYNOPSIS
 [verse]
 'git range-diff' [--color=[<when>]] [--no-color] [<diff-options>]
 	[--no-dual-color] [--creation-factor=<factor>]
+	[--left-only | --right-only]
 	( <range1> <range2> | <rev1>...<rev2> | <base> <rev1> <rev2> )
 
 DESCRIPTION
@@ -68,6 +69,14 @@ to revert to color all lines according to the outer diff markers
 	See the ``Algorithm`` section below for an explanation why this is
 	needed.
 
+--left-only::
+	Suppress commits that are missing from the first specified range
+	(or the "left range" when using the `<rev1>...<rev2>` format).
+
+--right-only::
+	Suppress commits that are missing from the second specified range
+	(or the "right range" when using the `<rev1>...<rev2>` format).
+
 --[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 80fcdc6ad42d..78bc9fa77062 100644
--- a/builtin/range-diff.c
+++ b/builtin/range-diff.c
@@ -21,7 +21,7 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)
 		.diffopt = &diffopt,
 		.other_arg = &other_arg
 	};
-	int simple_color = -1;
+	int simple_color = -1, left_only = 0, right_only = 0;
 	struct option range_diff_options[] = {
 		OPT_INTEGER(0, "creation-factor",
 			    &range_diff_opts.creation_factor,
@@ -31,6 +31,10 @@ 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_BOOL(0, "left-only", &left_only,
+			 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_END()
 	};
 	struct option *options;
@@ -88,6 +92,8 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)
 	FREE_AND_NULL(options);
 
 	range_diff_opts.dual_color = simple_color < 1;
+	range_diff_opts.left_only = left_only;
+	range_diff_opts.right_only = right_only;
 	res = show_range_diff(range1.buf, range2.buf, &range_diff_opts);
 
 	strvec_clear(&other_arg);
diff --git a/range-diff.c b/range-diff.c
index 001b6e174079..ed19b4729845 100644
--- a/range-diff.c
+++ b/range-diff.c
@@ -514,7 +514,8 @@ static void output(struct string_list *a, struct string_list *b,
 
 		/* Show unmatched LHS commit whose predecessors were shown. */
 		if (i < a->nr && a_util->matching < 0) {
-			output_pair_header(&opts, patch_no_width,
+			if (!range_diff_opts->right_only)
+				output_pair_header(&opts, patch_no_width,
 					   &buf, &dashes, a_util, NULL);
 			i++;
 			continue;
@@ -522,7 +523,8 @@ static void output(struct string_list *a, struct string_list *b,
 
 		/* Show unmatched RHS commits. */
 		while (j < b->nr && b_util->matching < 0) {
-			output_pair_header(&opts, patch_no_width,
+			if (!range_diff_opts->left_only)
+				output_pair_header(&opts, patch_no_width,
 					   &buf, &dashes, NULL, b_util);
 			b_util = ++j < b->nr ? b->items[j].util : NULL;
 		}
@@ -552,7 +554,10 @@ 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;
 
-	if (read_patches(range1, &branch1, range_diff_opts->other_arg))
+	if (range_diff_opts->left_only && range_diff_opts->right_only)
+		res = error(_("--left-only and --right-only are mutually exclusive"));
+
+	if (!res && read_patches(range1, &branch1, range_diff_opts->other_arg))
 		res = error(_("could not parse log for '%s'"), range1);
 	if (!res && read_patches(range2, &branch2, range_diff_opts->other_arg))
 		res = error(_("could not parse log for '%s'"), range2);
diff --git a/range-diff.h b/range-diff.h
index a595f4e8db2d..04ffe217be67 100644
--- a/range-diff.h
+++ b/range-diff.h
@@ -9,6 +9,7 @@
 struct range_diff_options {
 	int creation_factor;
 	unsigned dual_color:1;
+	unsigned left_only:1, right_only: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 2b518378d4a0..04aa9aed6bde 100755
--- a/t/t3206-range-diff.sh
+++ b/t/t3206-range-diff.sh
@@ -730,4 +730,19 @@ test_expect_success 'format-patch --range-diff with multiple notes' '
 	test_cmp expect actual
 '
 
+test_expect_success '--left-only/--right-only' '
+	git switch --orphan left-right &&
+	test_commit first &&
+	test_commit unmatched &&
+	test_commit common &&
+	git switch -C left-right first &&
+	git cherry-pick common &&
+
+	git range-diff -s --left-only ...common >actual &&
+	head_oid=$(git rev-parse --short HEAD) &&
+	common_oid=$(git rev-parse --short common) &&
+	echo "1:  $head_oid = 2:  $common_oid common" >expect &&
+	test_cmp expect actual
+'
+
 test_done
-- 
gitgitgadget