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

STALE2016d

Revision v1 of 2 in this series.

16 messages, 3 authors, 2021-02-05 · open the first message on its own page

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

From: Johannes Schindelin via GitGitGadget <hidden>
Date: 2021-02-04 20:15:35

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 3/6] range-diff: simplify code spawning `git log`

From: Johannes Schindelin via GitGitGadget <hidden>
Date: 2021-02-04 20:15:08

From: Johannes Schindelin <redacted>

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 <redacted>
---
 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 1/6] range-diff: avoid leaking memory in two error code paths

From: Johannes Schindelin via GitGitGadget <hidden>
Date: 2021-02-04 20:15:35

From: Johannes Schindelin <redacted>

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 <redacted>
---
 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

From: Johannes Schindelin via GitGitGadget <hidden>
Date: 2021-02-04 20:15:35

From: Johannes Schindelin <redacted>

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 <redacted>
---
 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 4/6] range-diff: combine all options in a single data structure

From: Johannes Schindelin via GitGitGadget <hidden>
Date: 2021-02-04 20:22:43

From: Johannes Schindelin <redacted>

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

Signed-off-by: Johannes Schindelin <redacted>
---
 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

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

From: Johannes Schindelin via GitGitGadget <hidden>
Date: 2021-02-04 20:23:20

From: Johannes Schindelin <redacted>

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 <redacted>
---
 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

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

From: Johannes Schindelin via GitGitGadget <hidden>
Date: 2021-02-04 20:24:05

From: Johannes Schindelin <redacted>

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 <redacted>
---
 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

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

From: Eric Sunshine <hidden>
Date: 2021-02-04 23:57:46

On Thu, Feb 4, 2021 at 3:24 PM Johannes Schindelin via GitGitGadget
[off-list ref] wrote:
quoted hunk
This will make it easier to implement the `--left-only` and
`--right-only` options.

Signed-off-by: Johannes Schindelin <redacted>
---
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.

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

From: Johannes Schindelin via GitGitGadget <hidden>
Date: 2021-02-05 20:40:11

From: Johannes Schindelin <redacted>

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 <redacted>
---
 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 5/6] range-diff: move the diffopt initialization down one layer

From: Johannes Schindelin via GitGitGadget <hidden>
Date: 2021-02-05 21:52:15

From: Johannes Schindelin <redacted>

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 <redacted>
---
 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 2/6] range-diff: libify the read_patches() function again

From: Johannes Schindelin via GitGitGadget <hidden>
Date: 2021-02-05 22:01:36

From: Johannes Schindelin <redacted>

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 <redacted>
---
 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 0/6] Optionally restrict range-diff output to "left" or "right" range only

From: Johannes Schindelin via GitGitGadget <hidden>
Date: 2021-02-05 22:05:01

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 6/6] range-diff: offer --left-only/--right-only options

From: Johannes Schindelin via GitGitGadget <hidden>
Date: 2021-02-05 22:08:06

From: Johannes Schindelin <redacted>

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 <redacted>
---
 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

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

From: Johannes Schindelin via GitGitGadget <hidden>
Date: 2021-02-05 22:08:16

From: Johannes Schindelin <redacted>

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 <redacted>
---
 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

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

From: Johannes Schindelin <hidden>
Date: 2021-02-05 22:28:45

Hi Eric,

On Thu, 4 Feb 2021, Eric Sunshine wrote:
On Thu, Feb 4, 2021 at 3:24 PM Johannes Schindelin via GitGitGadget
[off-list ref] wrote:
quoted
This will make it easier to implement the `--left-only` and
`--right-only` options.

Signed-off-by: Johannes Schindelin <redacted>
---
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 v2 4/6] range-diff: combine all options in a single data structure

From: Johannes Schindelin via GitGitGadget <hidden>
Date: 2021-02-05 22:29:51

From: Johannes Schindelin <redacted>

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

Signed-off-by: Johannes Schindelin <redacted>
---
 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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help