[PATCH 00/14] format-patch: add --interdiff and --range-diff options

[PATCH 00/14] format-patch: add --interdiff and --range-diff options

[Eric Sunshine]

When re-submitting a patch series, it is often helpful (for reviewers)
to include an interdiff or range-diff against the previous version.
Doing so requires manually running git-diff or git-range-diff and
copy/pasting the result into the cover letter of the new version.

This series automates the process by introducing git-format-patch
options --interdiff and --range-diff which insert such a diff into the
cover-letter or into the commentary section of the lone patch of a
1-patch series. In the latter case, the interdiff or range-diff is
indented to avoid confusing git-am and human readers.

Patches 1-6 add --interdiff and can apply directly on 'master'.
Patches 7-14 add --range-diff and apply atop js/range-diff v4[1].

An earlier RFC[2] implemented only --range-diff, and only for the
cover-letter.

Changes since the RFC:

* add --interdiff option for cover-letter and lone patch

* based on js/range-diff v4[1]

* --range-diff works with lone patch (no longer limited to cover
  letter)

* --range-diff colors output when used with --stdout, just as patches
  themselves are already colored

* --range-diff takes advantage of libified range-diff mechanism in v4
  rather than invoking git-range-diff command

No interdiff or range-diff is included in this cover-letter since the
implementation changed dramatically.

[1]: https://public-inbox.org/git/pull.1.v4.git.gitgitgadget@gmail.com/
[2]: https://public-inbox.org/git/20180530080325.37520-1-sunshine@sunshineco.com/

Eric Sunshine (14):
  format-patch: allow additional generated content in
    make_cover_letter()
  format-patch: add --interdiff option to embed diff in cover letter
  format-patch: teach --interdiff to respect -v/--reroll-count
  interdiff: teach show_interdiff() to indent interdiff
  log-tree: show_log: make commentary block delimiting reusable
  format-patch: allow --interdiff to apply to a lone-patch
  range-diff: respect diff_option.file rather than assuming 'stdout'
  range-diff: publish default creation factor
  range-diff: relieve callers of low-level configuration burden
  format-patch: add --range-diff option to embed diff in cover letter
  format-patch: extend --range-diff to accept revision range
  format-patch: teach --range-diff to respect -v/--reroll-count
  format-patch: add --creation-factor tweak for --range-diff
  format-patch: allow --range-diff to apply to a lone-patch

 Documentation/git-format-patch.txt |  29 ++++++
 Makefile                           |   1 +
 builtin/log.c                      | 139 ++++++++++++++++++++++++-----
 builtin/range-diff.c               |  25 ++----
 interdiff.c                        |  28 ++++++
 interdiff.h                        |   8 ++
 log-tree.c                         |  52 +++++++++--
 range-diff.c                       |  26 +++++-
 range-diff.h                       |   5 +-
 revision.h                         |  11 +++
 t/t3206-range-diff.sh              |  12 +++
 t/t4014-format-patch.sh            |  34 +++++++
 12 files changed, 319 insertions(+), 51 deletions(-)
 create mode 100644 interdiff.c
 create mode 100644 interdiff.h

-- 
2.18.0.345.g5c9ce644c3

[PATCH 01/14] format-patch: allow additional generated content in make_cover_letter()

[Eric Sunshine]

make_cover_letter() returns early when it lacks sufficient state to emit
a diffstat, which makes it difficult to extend the function to reliably
emit additional generated content. Work around this shortcoming by
factoring diffstat-printing logic out to its own function and calling it
as needed without otherwise inhibiting normal control flow.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 builtin/log.c | 43 +++++++++++++++++++++++--------------------
 1 file changed, 23 insertions(+), 20 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 805f89d7e1..873aabcf40 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -997,6 +997,26 @@ static char *find_branch_name(struct rev_info *rev)
 	return branch;
 }
 
+static void show_diffstat(struct rev_info *rev,
+			  struct commit *origin, struct commit *head)
+{
+	struct diff_options opts;
+
+	memcpy(&opts, &rev->diffopt, sizeof(opts));
+	opts.output_format = DIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT;
+	opts.stat_width = MAIL_DEFAULT_WRAP;
+
+	diff_setup_done(&opts);
+
+	diff_tree_oid(get_commit_tree_oid(origin),
+		      get_commit_tree_oid(head),
+		      "", &opts);
+	diffcore_std(&opts);
+	diff_flush(&opts);
+
+	fprintf(rev->diffopt.file, "\n");
+}
+
 static void make_cover_letter(struct rev_info *rev, int use_stdout,
 			      struct commit *origin,
 			      int nr, struct commit **list,
@@ -1010,7 +1030,6 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 	struct strbuf sb = STRBUF_INIT;
 	int i;
 	const char *encoding = "UTF-8";
-	struct diff_options opts;
 	int need_8bit_cte = 0;
 	struct pretty_print_context pp = {0};
 	struct commit *head = list[0];
@@ -1060,25 +1079,9 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 
 	shortlog_output(&log);
 
-	/*
-	 * We can only do diffstat with a unique reference point
-	 */
-	if (!origin)
-		return;
-
-	memcpy(&opts, &rev->diffopt, sizeof(opts));
-	opts.output_format = DIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT;
-	opts.stat_width = MAIL_DEFAULT_WRAP;
-
-	diff_setup_done(&opts);
-
-	diff_tree_oid(get_commit_tree_oid(origin),
-		      get_commit_tree_oid(head),
-		      "", &opts);
-	diffcore_std(&opts);
-	diff_flush(&opts);
-
-	fprintf(rev->diffopt.file, "\n");
+	/* We can only do diffstat with a unique reference point */
+	if (origin)
+		show_diffstat(rev, origin, head);
 }
 
 static const char *clean_message_id(const char *msg_id)
-- 
2.18.0.345.g5c9ce644c3

[PATCH 02/14] format-patch: add --interdiff option to embed diff in cover letter

[Eric Sunshine]

When submitting a revised version of a patch series, it can be helpful
(to reviewers) to include a summary of changes since the previous
attempt in the form of an interdiff, however, doing so involves manually
copy/pasting the diff into the cover letter.

Add an --interdiff option to automate this process. The argument to
--interdiff specifies the tip of the previous attempt against which to
generate the interdiff. For example:

    git format-patch --cover-letter --interdiff=v1 -3 v2

The previous attempt and the patch series being formatted must share a
common base.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 Documentation/git-format-patch.txt |  9 +++++++++
 Makefile                           |  1 +
 builtin/log.c                      | 24 ++++++++++++++++++++++--
 interdiff.c                        | 17 +++++++++++++++++
 interdiff.h                        |  8 ++++++++
 revision.h                         |  4 ++++
 t/t4014-format-patch.sh            | 17 +++++++++++++++++
 7 files changed, 78 insertions(+), 2 deletions(-)
 create mode 100644 interdiff.c
 create mode 100644 interdiff.h

diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index b41e1329a7..a1b1bafee7 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -23,6 +23,7 @@ SYNOPSIS
 		   [(--reroll-count|-v) <n>]
 		   [--to=<email>] [--cc=<email>]
 		   [--[no-]cover-letter] [--quiet] [--notes[=<ref>]]
+		   [--interdiff=<previous>]
 		   [--progress]
 		   [<common diff options>]
 		   [ <since> | <revision range> ]
@@ -228,6 +229,14 @@ feeding the result to `git send-email`.
 	containing the branch description, shortlog and the overall diffstat.  You can
 	fill in a description in the file before sending it out.
 
+--interdiff=<previous>::
+	As a reviewer aid, insert an interdiff into the cover letter showing
+	the differences between the previous version of the patch series and
+	the series currently being formatted. `previous` is a single revision
+	naming the tip of the previous series which shares a common base with
+	the series being formatted (for example `git format-patch
+	--cover-letter --interdiff=feature/v1 -3 feature/v2`).
+
 --notes[=<ref>]::
 	Append the notes (see linkgit:git-notes[1]) for the commit
 	after the three-dash line.
diff --git a/Makefile b/Makefile
index 41b93689ad..2af389c0d9 100644
--- a/Makefile
+++ b/Makefile
@@ -872,6 +872,7 @@ LIB_OBJS += linear-assignment.o
 LIB_OBJS += help.o
 LIB_OBJS += hex.o
 LIB_OBJS += ident.o
+LIB_OBJS += interdiff.o
 LIB_OBJS += kwset.o
 LIB_OBJS += levenshtein.o
 LIB_OBJS += line-log.o
diff --git a/builtin/log.c b/builtin/log.c
index 873aabcf40..1020b78477 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -30,6 +30,7 @@
 #include "gpg-interface.h"
 #include "progress.h"
 #include "commit-slab.h"
+#include "interdiff.h"
 
 #define MAIL_DEFAULT_WRAP 72
 
@@ -1082,6 +1083,11 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 	/* We can only do diffstat with a unique reference point */
 	if (origin)
 		show_diffstat(rev, origin, head);
+
+	if (rev->idiff_oid1) {
+		fprintf_ln(rev->diffopt.file, "%s", _("Interdiff:"));
+		show_interdiff(rev);
+	}
 }
 
 static const char *clean_message_id(const char *msg_id)
@@ -1448,6 +1454,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	struct base_tree_info bases;
 	int show_progress = 0;
 	struct progress *progress = NULL;
+	struct oid_array idiff_prev = OID_ARRAY_INIT;
 
 	const struct option builtin_format_patch_options[] = {
 		{ OPTION_CALLBACK, 'n', "numbered", &numbered, NULL,
@@ -1521,6 +1528,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		OPT__QUIET(&quiet, N_("don't print the patch filenames")),
 		OPT_BOOL(0, "progress", &show_progress,
 			 N_("show progress while generating patches")),
+		OPT_CALLBACK(0, "interdiff", &idiff_prev, N_("rev"),
+			     N_("show changes against <rev> in cover letter"),
+			     parse_opt_object_name),
 		OPT_END()
 	};
 
@@ -1706,7 +1716,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		if (rev.pending.nr == 2) {
 			struct object_array_entry *o = rev.pending.objects;
 			if (oidcmp(&o[0].item->oid, &o[1].item->oid) == 0)
-				return 0;
+				goto done;
 		}
 		get_patch_ids(&rev, &ids);
 	}
@@ -1730,7 +1740,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	}
 	if (nr == 0)
 		/* nothing to do */
-		return 0;
+		goto done;
 	total = nr;
 	if (cover_letter == -1) {
 		if (config_cover_letter == COVER_AUTO)
@@ -1743,6 +1753,13 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	if (numbered)
 		rev.total = total + start_number - 1;
 
+	if (idiff_prev.nr) {
+		if (!cover_letter)
+			die(_("--interdiff requires --cover-letter"));
+		rev.idiff_oid1 = &idiff_prev.oid[idiff_prev.nr - 1];
+		rev.idiff_oid2 = get_commit_tree_oid(list[0]);
+	}
+
 	if (!signature) {
 		; /* --no-signature inhibits all signatures */
 	} else if (signature && signature != git_version_string) {
@@ -1860,6 +1877,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	string_list_clear(&extra_hdr, 0);
 	if (ignore_if_in_upstream)
 		free_patch_ids(&ids);
+
+done:
+	oid_array_clear(&idiff_prev);
 	return 0;
 }
 
diff --git a/interdiff.c b/interdiff.c
new file mode 100644
index 0000000000..d0fac10c7c
--- /dev/null
+++ b/interdiff.c
@@ -0,0 +1,17 @@
+#include "cache.h"
+#include "commit.h"
+#include "revision.h"
+#include "interdiff.h"
+
+void show_interdiff(struct rev_info *rev)
+{
+	struct diff_options opts;
+
+	memcpy(&opts, &rev->diffopt, sizeof(opts));
+	opts.output_format = DIFF_FORMAT_PATCH;
+	diff_setup_done(&opts);
+
+	diff_tree_oid(rev->idiff_oid1, rev->idiff_oid2, "", &opts);
+	diffcore_std(&opts);
+	diff_flush(&opts);
+}
diff --git a/interdiff.h b/interdiff.h
new file mode 100644
index 0000000000..793c0144fe
--- /dev/null
+++ b/interdiff.h
@@ -0,0 +1,8 @@
+#ifndef INTERDIFF_H
+#define INTERDIFF_H
+
+struct rev_info;
+
+void show_interdiff(struct rev_info *);
+
+#endif
diff --git a/revision.h b/revision.h
index bf2239f876..61931fbac5 100644
--- a/revision.h
+++ b/revision.h
@@ -212,6 +212,10 @@ struct rev_info {
 	/* notes-specific options: which refs to show */
 	struct display_notes_opt notes_opt;
 
+	/* interdiff */
+	const struct object_id *idiff_oid1;
+	const struct object_id *idiff_oid2;
+
 	/* commit counts */
 	int count_left;
 	int count_right;
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 53880da7bb..57b46322aa 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1717,4 +1717,21 @@ test_expect_success 'format-patch --pretty=mboxrd' '
 	test_cmp expect actual
 '
 
+test_expect_success 'interdiff: setup' '
+	git checkout -b boop master &&
+	test_commit fnorp blorp &&
+	test_commit fleep blorp
+'
+
+test_expect_success 'interdiff: cover-letter' '
+	sed "y/q/ /" >expect <<-\EOF &&
+	+fleep
+	--q
+	EOF
+	git format-patch --cover-letter --interdiff=boop~2 -1 boop &&
+	test_i18ngrep "^Interdiff:$" 0000-cover-letter.patch &&
+	sed "1,/^@@ /d; /^-- $/q" <0000-cover-letter.patch >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.18.0.345.g5c9ce644c3

[PATCH 03/14] format-patch: teach --interdiff to respect -v/--reroll-count

[Eric Sunshine]

The --interdiff option introduces the embedded interdiff generically as
"Interdiff:", however, we can do better when --reroll-count is specified
by emitting "Interdiff against v{n}:" instead.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 builtin/log.c           | 17 ++++++++++++++++-
 revision.h              |  1 +
 t/t4014-format-patch.sh |  5 +++++
 3 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/builtin/log.c b/builtin/log.c
index 1020b78477..99ddfe8bb0 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1085,7 +1085,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 		show_diffstat(rev, origin, head);
 
 	if (rev->idiff_oid1) {
-		fprintf_ln(rev->diffopt.file, "%s", _("Interdiff:"));
+		fprintf_ln(rev->diffopt.file, "%s", rev->idiff_title);
 		show_interdiff(rev);
 	}
 }
@@ -1427,6 +1427,16 @@ static void print_bases(struct base_tree_info *bases, FILE *file)
 	oidclr(&bases->base_commit);
 }
 
+static const char *diff_title(struct strbuf *sb, int reroll_count,
+		       const char *generic, const char *rerolled)
+{
+	if (reroll_count <= 0)
+		strbuf_addstr(sb, generic);
+	else /* RFC may be v0, so allow -v1 to diff against v0 */
+		strbuf_addf(sb, rerolled, reroll_count - 1);
+	return sb->buf;
+}
+
 int cmd_format_patch(int argc, const char **argv, const char *prefix)
 {
 	struct commit *commit;
@@ -1455,6 +1465,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	int show_progress = 0;
 	struct progress *progress = NULL;
 	struct oid_array idiff_prev = OID_ARRAY_INIT;
+	struct strbuf idiff_title = STRBUF_INIT;
 
 	const struct option builtin_format_patch_options[] = {
 		{ OPTION_CALLBACK, 'n', "numbered", &numbered, NULL,
@@ -1758,6 +1769,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			die(_("--interdiff requires --cover-letter"));
 		rev.idiff_oid1 = &idiff_prev.oid[idiff_prev.nr - 1];
 		rev.idiff_oid2 = get_commit_tree_oid(list[0]);
+		rev.idiff_title = diff_title(&idiff_title, reroll_count,
+					     _("Interdiff:"),
+					     _("Interdiff against v%d:"));
 	}
 
 	if (!signature) {
@@ -1880,6 +1894,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 
 done:
 	oid_array_clear(&idiff_prev);
+	strbuf_release(&idiff_title);
 	return 0;
 }
 
diff --git a/revision.h b/revision.h
index 61931fbac5..ffeadc261a 100644
--- a/revision.h
+++ b/revision.h
@@ -215,6 +215,7 @@ struct rev_info {
 	/* interdiff */
 	const struct object_id *idiff_oid1;
 	const struct object_id *idiff_oid2;
+	const char *idiff_title;
 
 	/* commit counts */
 	int count_left;
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 57b46322aa..5950890d30 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1734,4 +1734,9 @@ test_expect_success 'interdiff: cover-letter' '
 	test_cmp expect actual
 '
 
+test_expect_success 'interdiff: reroll-count' '
+	git format-patch --cover-letter --interdiff=boop~2 -v2 -1 boop &&
+	test_i18ngrep "^Interdiff ..* v1:$" v2-0000-cover-letter.patch
+'
+
 test_done
-- 
2.18.0.345.g5c9ce644c3

[PATCH 04/14] interdiff: teach show_interdiff() to indent interdiff

[Eric Sunshine]

A future change will allow "git format-patch --interdiff=<prev> -1" to
insert an interdiff into the commentary section of the lone patch of a
1-patch series. However, to prevent the inserted interdiff from
confusing git-am, as well as human readers, it needs to be indented.
Therefore, teach show_interdiff() how to indent.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 builtin/log.c |  2 +-
 interdiff.c   | 13 ++++++++++++-
 interdiff.h   |  2 +-
 3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 99ddfe8bb0..8078a43d14 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1086,7 +1086,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 
 	if (rev->idiff_oid1) {
 		fprintf_ln(rev->diffopt.file, "%s", rev->idiff_title);
-		show_interdiff(rev);
+		show_interdiff(rev, 0);
 	}
 }
 
diff --git a/interdiff.c b/interdiff.c
index d0fac10c7c..c81d680a6c 100644
--- a/interdiff.c
+++ b/interdiff.c
@@ -3,15 +3,26 @@
 #include "revision.h"
 #include "interdiff.h"
 
-void show_interdiff(struct rev_info *rev)
+static struct strbuf *idiff_prefix_cb(struct diff_options *opt, void *data)
+{
+	return data;
+}
+
+void show_interdiff(struct rev_info *rev, int indent)
 {
 	struct diff_options opts;
+	struct strbuf prefix = STRBUF_INIT;
 
 	memcpy(&opts, &rev->diffopt, sizeof(opts));
 	opts.output_format = DIFF_FORMAT_PATCH;
+	opts.output_prefix = idiff_prefix_cb;
+	strbuf_addchars(&prefix, ' ', indent);
+	opts.output_prefix_data = &prefix;
 	diff_setup_done(&opts);
 
 	diff_tree_oid(rev->idiff_oid1, rev->idiff_oid2, "", &opts);
 	diffcore_std(&opts);
 	diff_flush(&opts);
+
+	strbuf_release(&prefix);
 }
diff --git a/interdiff.h b/interdiff.h
index 793c0144fe..01c730a5c9 100644
--- a/interdiff.h
+++ b/interdiff.h
@@ -3,6 +3,6 @@
 
 struct rev_info;
 
-void show_interdiff(struct rev_info *);
+void show_interdiff(struct rev_info *, int indent);
 
 #endif
-- 
2.18.0.345.g5c9ce644c3

[PATCH 05/14] log-tree: show_log: make commentary block delimiting reusable

[Eric Sunshine]

In patches generated by git-format-patch, the area below the "---" line
following the commit message and before the actual 'diff' can be used
for commentary which the patch author wants to convey to readers of the
patch itself but not include in the commit message proper.

By default, the commentary area is empty, however, the --notes option
causes it to be populated with notes associated with the commit. In the
future, other options may be added which also insert content into the
commentary section.

To accommodate this, factor out the logic which delimits commentary
blocks from the commit message so that it can be re-used for upcoming
optional inserted content.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 log-tree.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/log-tree.c b/log-tree.c
index 4a3907fea0..9d38f1cf79 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -541,6 +541,16 @@ static int show_mergetag(struct rev_info *opt, struct commit *commit)
 	return for_each_mergetag(show_one_mergetag, commit, opt);
 }
 
+static void next_commentary_block(struct rev_info *opt, struct strbuf *sb)
+{
+	const char *x = opt->shown_dashes ? "\n" : "---\n";
+	if (sb)
+		strbuf_addstr(sb, x);
+	else
+		fputs(x, opt->diffopt.file);
+	opt->shown_dashes = 1;
+}
+
 void show_log(struct rev_info *opt)
 {
 	struct strbuf msgbuf = STRBUF_INIT;
@@ -698,10 +708,8 @@ void show_log(struct rev_info *opt)
 
 	if ((ctx.fmt != CMIT_FMT_USERFORMAT) &&
 	    ctx.notes_message && *ctx.notes_message) {
-		if (cmit_fmt_is_mail(ctx.fmt)) {
-			strbuf_addstr(&msgbuf, "---\n");
-			opt->shown_dashes = 1;
-		}
+		if (cmit_fmt_is_mail(ctx.fmt))
+			next_commentary_block(opt, &msgbuf);
 		strbuf_addstr(&msgbuf, ctx.notes_message);
 	}
 
@@ -765,9 +773,10 @@ int log_tree_diff_flush(struct rev_info *opt)
 
 			/*
 			 * We may have shown three-dashes line early
-			 * between notes and the log message, in which
-			 * case we only want a blank line after the
-			 * notes without (an extra) three-dashes line.
+			 * between generated commentary (notes, etc.)
+			 * and the log message, in which case we only
+			 * want a blank line after the commentary
+			 * without (an extra) three-dashes line.
 			 * Otherwise, we show the three-dashes line if
 			 * we are showing the patch with diffstat, but
 			 * in that case, there is no extra blank line
-- 
2.18.0.345.g5c9ce644c3

[PATCH 06/14] format-patch: allow --interdiff to apply to a lone-patch

[Eric Sunshine]

When submitting a revised version of a patch or series, it can be
helpful (to reviewers) to include a summary of changes since the
previous attempt in the form of an interdiff, typically in the cover
letter. However, it is occasionally useful, despite making for a noisy
read, to insert an interdiff into the commentary section of the lone
patch of a 1-patch series.

Therefore, extend "git format-patch --interdiff=<prev>" to insert an
interdiff into the commentary section of a lone patch rather than
requiring a cover letter. The interdiff is indented to avoid confusing
git-am and human readers into considering it part of the patch proper.

Implementation note: Generating an interdiff for insertion into the
commentary section of a patch which itself is currently being generated
requires invoking the diffing machinery recursively. However, the
machinery does not (presently) support this since it uses global state.
Consequently, we need to take care to stash away the state of the
in-progress operation while generating the interdiff, and restore it
after.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 Documentation/git-format-patch.txt |  3 ++-
 builtin/log.c                      |  8 +++++---
 log-tree.c                         | 14 ++++++++++++++
 t/t4014-format-patch.sh            | 12 ++++++++++++
 4 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index a1b1bafee7..f8a061794d 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -230,7 +230,8 @@ feeding the result to `git send-email`.
 	fill in a description in the file before sending it out.
 
 --interdiff=<previous>::
-	As a reviewer aid, insert an interdiff into the cover letter showing
+	As a reviewer aid, insert an interdiff into the cover letter,
+	or as commentary of the lone patch of a 1-patch series, showing
 	the differences between the previous version of the patch series and
 	the series currently being formatted. `previous` is a single revision
 	naming the tip of the previous series which shares a common base with
diff --git a/builtin/log.c b/builtin/log.c
index 8078a43d14..e990027c28 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1540,7 +1540,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		OPT_BOOL(0, "progress", &show_progress,
 			 N_("show progress while generating patches")),
 		OPT_CALLBACK(0, "interdiff", &idiff_prev, N_("rev"),
-			     N_("show changes against <rev> in cover letter"),
+			     N_("show changes against <rev> in cover letter or single patch"),
 			     parse_opt_object_name),
 		OPT_END()
 	};
@@ -1765,8 +1765,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		rev.total = total + start_number - 1;
 
 	if (idiff_prev.nr) {
-		if (!cover_letter)
-			die(_("--interdiff requires --cover-letter"));
+		if (!cover_letter && total != 1)
+			die(_("--interdiff requires --cover-letter or single patch"));
 		rev.idiff_oid1 = &idiff_prev.oid[idiff_prev.nr - 1];
 		rev.idiff_oid2 = get_commit_tree_oid(list[0]);
 		rev.idiff_title = diff_title(&idiff_title, reroll_count,
@@ -1811,6 +1811,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		print_signature(rev.diffopt.file);
 		total++;
 		start_number--;
+		/* interdiff in cover-letter; omit from patches */
+		rev.idiff_oid1 = NULL;
 	}
 	rev.add_signoff = do_signoff;
 
diff --git a/log-tree.c b/log-tree.c
index 9d38f1cf79..56513fa83d 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -14,6 +14,7 @@
 #include "sequencer.h"
 #include "line-log.h"
 #include "help.h"
+#include "interdiff.h"
 
 static struct decoration name_decoration = { "object names" };
 static int decoration_loaded;
@@ -736,6 +737,19 @@ void show_log(struct rev_info *opt)
 
 	strbuf_release(&msgbuf);
 	free(ctx.notes_message);
+
+	if (cmit_fmt_is_mail(ctx.fmt) && opt->idiff_oid1) {
+		struct diff_queue_struct dq;
+
+		memcpy(&dq, &diff_queued_diff, sizeof(diff_queued_diff));
+		DIFF_QUEUE_CLEAR(&diff_queued_diff);
+
+		next_commentary_block(opt, NULL);
+		fprintf_ln(opt->diffopt.file, "%s", opt->idiff_title);
+		show_interdiff(opt, 2);
+
+		memcpy(&diff_queued_diff, &dq, sizeof(diff_queued_diff));
+	}
 }
 
 int log_tree_diff_flush(struct rev_info *opt)
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 5950890d30..909c743c13 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1730,6 +1730,7 @@ test_expect_success 'interdiff: cover-letter' '
 	EOF
 	git format-patch --cover-letter --interdiff=boop~2 -1 boop &&
 	test_i18ngrep "^Interdiff:$" 0000-cover-letter.patch &&
+	test_i18ngrep ! "^Interdiff:$" 0001-fleep.patch &&
 	sed "1,/^@@ /d; /^-- $/q" <0000-cover-letter.patch >actual &&
 	test_cmp expect actual
 '
@@ -1739,4 +1740,15 @@ test_expect_success 'interdiff: reroll-count' '
 	test_i18ngrep "^Interdiff ..* v1:$" v2-0000-cover-letter.patch
 '
 
+test_expect_success 'interdiff: solo-patch' '
+	cat >expect <<-\EOF &&
+	  +fleep
+
+	EOF
+	git format-patch --interdiff=boop~2 -1 boop &&
+	test_i18ngrep "^Interdiff:$" 0001-fleep.patch &&
+	sed "1,/^  @@ /d; /^$/q" <0001-fleep.patch >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.18.0.345.g5c9ce644c3

[PATCH 07/14] range-diff: respect diff_option.file rather than assuming 'stdout'

[Eric Sunshine]

The actual diffs output by range-diff respect diff_option.file, which
range-diff passes down the call-chain, thus are destination-agnostic.
However, output_pair_header() is hard-coded to emit to 'stdout'. Fix
this by making output_pair_header() respect diff_option.file, as well.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 range-diff.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/range-diff.c b/range-diff.c
index 347b4a79f2..76e053c2b2 100644
--- a/range-diff.c
+++ b/range-diff.c
@@ -328,7 +328,7 @@ static void output_pair_header(struct diff_options *diffopt,
 	}
 	strbuf_addf(buf, "%s\n", color_reset);
 
-	fwrite(buf->buf, buf->len, 1, stdout);
+	fwrite(buf->buf, buf->len, 1, diffopt->file);
 }
 
 static struct userdiff_driver no_func_name = {
-- 
2.18.0.345.g5c9ce644c3

[PATCH 08/14] range-diff: publish default creation factor

[Eric Sunshine]

The range-diff back-end allows its heuristic to be tweaked via the
"creation factor". git-range-diff, the only client of the back-end,
defaults the factor to 60% (hard-coded in builtin/range-diff.c), but
allows the user to override it with the --creation-factor option.

Publish the default range factor to allow new callers of the range-diff
back-end to default to the same value without duplicating the hard-coded
constant, and to avoid worrying about various callers becoming
out-of-sync if the default ever needs to change.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 builtin/range-diff.c | 2 +-
 range-diff.h         | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/builtin/range-diff.c b/builtin/range-diff.c
index 77ac3bff7b..bbe6b05ae9 100644
--- a/builtin/range-diff.c
+++ b/builtin/range-diff.c
@@ -18,7 +18,7 @@ static struct strbuf *output_prefix_cb(struct diff_options *opt, void *data)
 
 int cmd_range_diff(int argc, const char **argv, const char *prefix)
 {
-	int creation_factor = 60;
+	int creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;
 	struct diff_options diffopt = { NULL };
 	int simple_color = -1;
 	struct option options[] = {
diff --git a/range-diff.h b/range-diff.h
index aea9d43f34..73d6e232fe 100644
--- a/range-diff.h
+++ b/range-diff.h
@@ -3,6 +3,8 @@
 
 #include "diff.h"
 
+#define RANGE_DIFF_CREATION_FACTOR_DEFAULT 60
+
 int show_range_diff(const char *range1, const char *range2,
 		    int creation_factor, struct diff_options *diffopt);
 
-- 
2.18.0.345.g5c9ce644c3

[PATCH 09/14] range-diff: relieve callers of low-level configuration burden

[Eric Sunshine]

There are a number of very low-level configuration details which need to
be managed precisely to generate a proper range-diff. In particular,
'diff_options' output format, header suppression, indentation, and
dual-color mode must all be set appropriately to ensure proper behavior.

Handle these details locally in the libified range-diff back-end rather
than forcing each caller to have specialized knowledge of these
implementation details, and to avoid duplication as new callers are
added.

While at it, localize these tweaks to be active only while generating
the range-diff, so they don't clobber the caller-provided
'diff_options', which might be used beyond range-diff generation.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 builtin/range-diff.c | 23 ++++-------------------
 range-diff.c         | 24 ++++++++++++++++++++++--
 range-diff.h         |  3 ++-
 3 files changed, 28 insertions(+), 22 deletions(-)

diff --git a/builtin/range-diff.c b/builtin/range-diff.c
index bbe6b05ae9..91463a1df6 100644
--- a/builtin/range-diff.c
+++ b/builtin/range-diff.c
@@ -11,11 +11,6 @@ N_("git range-diff [<options>] <base> <old-tip> <new-tip>"),
 NULL
 };
 
-static struct strbuf *output_prefix_cb(struct diff_options *opt, void *data)
-{
-	return data;
-}
-
 int cmd_range_diff(int argc, const char **argv, const char *prefix)
 {
 	int creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;
@@ -29,17 +24,11 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)
 		OPT_END()
 	};
 	int i, j, res = 0;
-	struct strbuf four_spaces = STRBUF_INIT;
 	struct strbuf range1 = STRBUF_INIT, range2 = STRBUF_INIT;
 
 	git_config(git_diff_ui_config, NULL);
 
 	diff_setup(&diffopt);
-	diffopt.output_format = DIFF_FORMAT_PATCH;
-	diffopt.flags.suppress_diff_headers = 1;
-	diffopt.output_prefix = output_prefix_cb;
-	strbuf_addstr(&four_spaces, "    ");
-	diffopt.output_prefix_data = &four_spaces;
 
 	argc = parse_options(argc, argv, NULL, options,
 			     builtin_range_diff_usage, PARSE_OPT_KEEP_UNKNOWN);
@@ -55,12 +44,9 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)
 	argc = j;
 	diff_setup_done(&diffopt);
 
-	if (simple_color < 1) {
-		if (!simple_color)
-			/* force color when --dual-color was used */
-			diffopt.use_color = 1;
-		diffopt.flags.dual_color_diffed_diffs = 1;
-	}
+	/* force color when --dual-color was used */
+	if (!simple_color)
+		diffopt.use_color = 1;
 
 	if (argc == 2) {
 		if (!strstr(argv[0], ".."))
@@ -96,11 +82,10 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)
 	}
 
 	res = show_range_diff(range1.buf, range2.buf, creation_factor,
-			      &diffopt);
+			      simple_color < 1, &diffopt);
 
 	strbuf_release(&range1);
 	strbuf_release(&range2);
-	strbuf_release(&four_spaces);
 
 	return res;
 }
diff --git a/range-diff.c b/range-diff.c
index 76e053c2b2..091fc344cd 100644
--- a/range-diff.c
+++ b/range-diff.c
@@ -414,8 +414,14 @@ static void output(struct string_list *a, struct string_list *b,
 	strbuf_release(&dashes);
 }
 
+static struct strbuf *output_prefix_cb(struct diff_options *opt, void *data)
+{
+	return data;
+}
+
 int show_range_diff(const char *range1, const char *range2,
-		    int creation_factor, struct diff_options *diffopt)
+		    int creation_factor, int dual_color,
+		    struct diff_options *diffopt)
 {
 	int res = 0;
 
@@ -428,9 +434,23 @@ 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;
+
+		memcpy(&opts, diffopt, sizeof(opts));
+		opts.output_format = DIFF_FORMAT_PATCH;
+		opts.flags.suppress_diff_headers = 1;
+		opts.flags.dual_color_diffed_diffs = dual_color;
+		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, creation_factor);
-		output(&branch1, &branch2, diffopt);
+		output(&branch1, &branch2, &opts);
+
+		strbuf_release(&indent);
 	}
 
 	string_list_clear(&branch1, 1);
diff --git a/range-diff.h b/range-diff.h
index 73d6e232fe..bf265594f3 100644
--- a/range-diff.h
+++ b/range-diff.h
@@ -6,6 +6,7 @@
 #define RANGE_DIFF_CREATION_FACTOR_DEFAULT 60
 
 int show_range_diff(const char *range1, const char *range2,
-		    int creation_factor, struct diff_options *diffopt);
+		    int creation_factor, int dual_color,
+		    struct diff_options *diffopt);
 
 #endif
-- 
2.18.0.345.g5c9ce644c3

[PATCH 10/14] format-patch: add --range-diff option to embed diff in cover letter

[Eric Sunshine]

When submitting a revised version of a patch series, it can be helpful
(to reviewers) to include a summary of changes since the previous
attempt in the form of a range-diff, however, doing so involves manually
copy/pasting the diff into the cover letter.

Add a --range-diff option to automate this process. The argument to
--range-diff specifies the tip of the previous attempt against which to
generate the range-diff. For example:

    git format-patch --cover-letter --range-diff=v1 -3 v2

(At this stage, the previous attempt and the patch series being
formatted must share a common base, however, a subsequent enhancement
will make it possible to specify an explicit revision range for the
previous attempt.)

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 Documentation/git-format-patch.txt | 10 +++++++++
 builtin/log.c                      | 35 ++++++++++++++++++++++++++++++
 revision.h                         |  5 +++++
 t/t3206-range-diff.sh              | 12 ++++++++++
 4 files changed, 62 insertions(+)

diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index f8a061794d..e7f404be3d 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -24,6 +24,7 @@ SYNOPSIS
 		   [--to=<email>] [--cc=<email>]
 		   [--[no-]cover-letter] [--quiet] [--notes[=<ref>]]
 		   [--interdiff=<previous>]
+		   [--range-diff=<previous>]
 		   [--progress]
 		   [<common diff options>]
 		   [ <since> | <revision range> ]
@@ -238,6 +239,15 @@ feeding the result to `git send-email`.
 	the series being formatted (for example `git format-patch
 	--cover-letter --interdiff=feature/v1 -3 feature/v2`).
 
+--range-diff=<previous>::
+	As a reviewer aid, insert a range-diff (see linkgit:git-range-diff[1])
+	into the cover letter showing the differences between the previous
+	version of the patch series and the series currently being formatted.
+	`previous` is a single revision naming the tip of the previous
+	series which shares a common base with the series being formatted (for
+	example `git format-patch --cover-letter --range-diff=feature/v1 -3
+	feature/v2`).
+
 --notes[=<ref>]::
 	Append the notes (see linkgit:git-notes[1]) for the commit
 	after the three-dash line.
diff --git a/builtin/log.c b/builtin/log.c
index e990027c28..d6e57e8b04 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -31,6 +31,7 @@
 #include "progress.h"
 #include "commit-slab.h"
 #include "interdiff.h"
+#include "range-diff.h"
 
 #define MAIL_DEFAULT_WRAP 72
 
@@ -1088,6 +1089,12 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 		fprintf_ln(rev->diffopt.file, "%s", rev->idiff_title);
 		show_interdiff(rev, 0);
 	}
+
+	if (rev->rdiff1) {
+		fprintf_ln(rev->diffopt.file, "%s", _("Range-diff:"));
+		show_range_diff(rev->rdiff1, rev->rdiff2,
+				rev->creation_factor, 1, &rev->diffopt);
+	}
 }
 
 static const char *clean_message_id(const char *msg_id)
@@ -1437,6 +1444,17 @@ static const char *diff_title(struct strbuf *sb, int reroll_count,
 	return sb->buf;
 }
 
+static void infer_range_diff_ranges(struct strbuf *r1,
+				    struct strbuf *r2,
+				    const char *prev,
+				    struct commit *head)
+{
+	const char *head_oid = oid_to_hex(&head->object.oid);
+
+	strbuf_addf(r1, "%s..%s", head_oid, prev);
+	strbuf_addf(r2, "%s..%s", prev, head_oid);
+}
+
 int cmd_format_patch(int argc, const char **argv, const char *prefix)
 {
 	struct commit *commit;
@@ -1466,6 +1484,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	struct progress *progress = NULL;
 	struct oid_array idiff_prev = OID_ARRAY_INIT;
 	struct strbuf idiff_title = STRBUF_INIT;
+	const char *rdiff_prev = NULL;
+	struct strbuf rdiff1 = STRBUF_INIT;
+	struct strbuf rdiff2 = STRBUF_INIT;
 
 	const struct option builtin_format_patch_options[] = {
 		{ OPTION_CALLBACK, 'n', "numbered", &numbered, NULL,
@@ -1542,6 +1563,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		OPT_CALLBACK(0, "interdiff", &idiff_prev, N_("rev"),
 			     N_("show changes against <rev> in cover letter or single patch"),
 			     parse_opt_object_name),
+		OPT_STRING(0, "range-diff", &rdiff_prev, N_("refspec"),
+			   N_("show changes against <refspec> in cover letter")),
 		OPT_END()
 	};
 
@@ -1774,6 +1797,16 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 					     _("Interdiff against v%d:"));
 	}
 
+	if (rdiff_prev) {
+		if (!cover_letter)
+			die(_("--range-diff requires --cover-letter"));
+
+		infer_range_diff_ranges(&rdiff1, &rdiff2, rdiff_prev, list[0]);
+		rev.rdiff1 = rdiff1.buf;
+		rev.rdiff2 = rdiff2.buf;
+		rev.creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;
+	}
+
 	if (!signature) {
 		; /* --no-signature inhibits all signatures */
 	} else if (signature && signature != git_version_string) {
@@ -1897,6 +1930,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 done:
 	oid_array_clear(&idiff_prev);
 	strbuf_release(&idiff_title);
+	strbuf_release(&rdiff1);
+	strbuf_release(&rdiff2);
 	return 0;
 }
 
diff --git a/revision.h b/revision.h
index ffeadc261a..11159416dc 100644
--- a/revision.h
+++ b/revision.h
@@ -217,6 +217,11 @@ struct rev_info {
 	const struct object_id *idiff_oid2;
 	const char *idiff_title;
 
+	/* range-diff */
+	const char *rdiff1;
+	const char *rdiff2;
+	int creation_factor;
+
 	/* commit counts */
 	int count_left;
 	int count_right;
diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
index 2237c7f4af..dd854b6ebc 100755
--- a/t/t3206-range-diff.sh
+++ b/t/t3206-range-diff.sh
@@ -142,4 +142,16 @@ test_expect_success 'changed message' '
 	test_cmp expected actual
 '
 
+for prev in topic
+do
+	test_expect_success "format-patch --range-diff=$prev" '
+		git format-patch --stdout --cover-letter --range-diff=$prev \
+			master..unmodified >actual &&
+		grep "= 1: .* s/5/A" actual &&
+		grep "= 2: .* s/4/A" actual &&
+		grep "= 3: .* s/11/B" actual &&
+		grep "= 4: .* s/12/B" actual
+	'
+done
+
 test_done
-- 
2.18.0.345.g5c9ce644c3

[PATCH 11/14] format-patch: extend --range-diff to accept revision range

[Eric Sunshine]

When submitting a revised a patch series, the --range-diff option embeds
a range-diff in the cover letter showing changes since the previous
version of the patch series. The argument to --range-diff is a simple
revision naming the tip of the previous series, which works fine if the
previous and current versions of the patch series share a common base.

However, it fails if the revision ranges of the old and new versions of
the series are disjoint. To address this shortcoming, extend
--range-diff to also accept an explicit revision range for the previous
series. For example:

    git format-patch --cover-letter --range-diff=v1~3..v1 -3 v2

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 Documentation/git-format-patch.txt |  8 +++++---
 builtin/log.c                      | 16 +++++++++++++---
 t/t3206-range-diff.sh              |  2 +-
 3 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index e7f404be3d..425145f536 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -243,10 +243,12 @@ feeding the result to `git send-email`.
 	As a reviewer aid, insert a range-diff (see linkgit:git-range-diff[1])
 	into the cover letter showing the differences between the previous
 	version of the patch series and the series currently being formatted.
-	`previous` is a single revision naming the tip of the previous
-	series which shares a common base with the series being formatted (for
+	`previous` can be a single revision naming the tip of the previous
+	series if it shares a common base with the series being formatted (for
 	example `git format-patch --cover-letter --range-diff=feature/v1 -3
-	feature/v2`).
+	feature/v2`), or a revision range if the two versions of the series are
+	disjoint (for example `git format-patch --cover-letter
+	--range-diff=feature/v1~3..feature/v1 -3 feature/v2`).
 
 --notes[=<ref>]::
 	Append the notes (see linkgit:git-notes[1]) for the commit
diff --git a/builtin/log.c b/builtin/log.c
index d6e57e8b04..4f937aad15 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1447,12 +1447,21 @@ static const char *diff_title(struct strbuf *sb, int reroll_count,
 static void infer_range_diff_ranges(struct strbuf *r1,
 				    struct strbuf *r2,
 				    const char *prev,
+				    struct commit *origin,
 				    struct commit *head)
 {
 	const char *head_oid = oid_to_hex(&head->object.oid);
 
-	strbuf_addf(r1, "%s..%s", head_oid, prev);
-	strbuf_addf(r2, "%s..%s", prev, head_oid);
+	if (!strstr(prev, "..")) {
+		strbuf_addf(r1, "%s..%s", head_oid, prev);
+		strbuf_addf(r2, "%s..%s", prev, head_oid);
+	} else if (!origin) {
+		die(_("failed to infer range-diff ranges"));
+	} else {
+		strbuf_addstr(r1, prev);
+		strbuf_addf(r2, "%s..%s",
+			    oid_to_hex(&origin->object.oid), head_oid);
+	}
 }
 
 int cmd_format_patch(int argc, const char **argv, const char *prefix)
@@ -1801,7 +1810,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		if (!cover_letter)
 			die(_("--range-diff requires --cover-letter"));
 
-		infer_range_diff_ranges(&rdiff1, &rdiff2, rdiff_prev, list[0]);
+		infer_range_diff_ranges(&rdiff1, &rdiff2, rdiff_prev,
+					origin, list[0]);
 		rev.rdiff1 = rdiff1.buf;
 		rev.rdiff2 = rdiff2.buf;
 		rev.creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;
diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
index dd854b6ebc..3d7a2d8a4d 100755
--- a/t/t3206-range-diff.sh
+++ b/t/t3206-range-diff.sh
@@ -142,7 +142,7 @@ test_expect_success 'changed message' '
 	test_cmp expected actual
 '
 
-for prev in topic
+for prev in topic master..topic
 do
 	test_expect_success "format-patch --range-diff=$prev" '
 		git format-patch --stdout --cover-letter --range-diff=$prev \
-- 
2.18.0.345.g5c9ce644c3

[PATCH 12/14] format-patch: teach --range-diff to respect -v/--reroll-count

[Eric Sunshine]

The --range-diff option announces the embedded range-diff generically
as "Range-diff:", however, we can do better when --reroll-count is
specified by emitting "Range-diff against v{n}:" instead.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 builtin/log.c | 7 ++++++-
 revision.h    | 1 +
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/builtin/log.c b/builtin/log.c
index 4f937aad15..fdb2180d7e 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1091,7 +1091,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 	}
 
 	if (rev->rdiff1) {
-		fprintf_ln(rev->diffopt.file, "%s", _("Range-diff:"));
+		fprintf_ln(rev->diffopt.file, "%s", rev->rdiff_title);
 		show_range_diff(rev->rdiff1, rev->rdiff2,
 				rev->creation_factor, 1, &rev->diffopt);
 	}
@@ -1496,6 +1496,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	const char *rdiff_prev = NULL;
 	struct strbuf rdiff1 = STRBUF_INIT;
 	struct strbuf rdiff2 = STRBUF_INIT;
+	struct strbuf rdiff_title = STRBUF_INIT;
 
 	const struct option builtin_format_patch_options[] = {
 		{ OPTION_CALLBACK, 'n', "numbered", &numbered, NULL,
@@ -1815,6 +1816,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		rev.rdiff1 = rdiff1.buf;
 		rev.rdiff2 = rdiff2.buf;
 		rev.creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;
+		rev.rdiff_title = diff_title(&rdiff_title, reroll_count,
+					     _("Range-diff:"),
+					     _("Range-diff against v%d:"));
 	}
 
 	if (!signature) {
@@ -1942,6 +1946,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	strbuf_release(&idiff_title);
 	strbuf_release(&rdiff1);
 	strbuf_release(&rdiff2);
+	strbuf_release(&rdiff_title);
 	return 0;
 }
 
diff --git a/revision.h b/revision.h
index 11159416dc..cd0873b575 100644
--- a/revision.h
+++ b/revision.h
@@ -221,6 +221,7 @@ struct rev_info {
 	const char *rdiff1;
 	const char *rdiff2;
 	int creation_factor;
+	const char *rdiff_title;
 
 	/* commit counts */
 	int count_left;
-- 
2.18.0.345.g5c9ce644c3

[PATCH 13/14] format-patch: add --creation-factor tweak for --range-diff

[Eric Sunshine]

When generating a range-diff, matching up commits between two version of
a patch series involves heuristics, thus may give unexpected results.
git-range-diff allows tweaking the heuristic via --creation-factor.
Follow suit by accepting --creation-factor in combination with
--range-diff when generating a range-diff for a cover-letter.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 Documentation/git-format-patch.txt |  8 +++++++-
 builtin/log.c                      | 10 +++++++++-
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 425145f536..9b2e172159 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -24,7 +24,7 @@ SYNOPSIS
 		   [--to=<email>] [--cc=<email>]
 		   [--[no-]cover-letter] [--quiet] [--notes[=<ref>]]
 		   [--interdiff=<previous>]
-		   [--range-diff=<previous>]
+		   [--range-diff=<previous> [--creation-factor=<percent>]]
 		   [--progress]
 		   [<common diff options>]
 		   [ <since> | <revision range> ]
@@ -250,6 +250,12 @@ feeding the result to `git send-email`.
 	disjoint (for example `git format-patch --cover-letter
 	--range-diff=feature/v1~3..feature/v1 -3 feature/v2`).
 
+--creation-factor=<percent>::
+	Used with `--range-diff`, tweak the heuristic which matches up commits
+	between the previous and current series of patches by adjusting the
+	creation/deletion cost fudge factor. See linkgit:git-range-diff[1])
+	for details.
+
 --notes[=<ref>]::
 	Append the notes (see linkgit:git-notes[1]) for the commit
 	after the three-dash line.
diff --git a/builtin/log.c b/builtin/log.c
index fdb2180d7e..10c3801ceb 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1497,6 +1497,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	struct strbuf rdiff1 = STRBUF_INIT;
 	struct strbuf rdiff2 = STRBUF_INIT;
 	struct strbuf rdiff_title = STRBUF_INIT;
+	int creation_factor = -1;
 
 	const struct option builtin_format_patch_options[] = {
 		{ OPTION_CALLBACK, 'n', "numbered", &numbered, NULL,
@@ -1575,6 +1576,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			     parse_opt_object_name),
 		OPT_STRING(0, "range-diff", &rdiff_prev, N_("refspec"),
 			   N_("show changes against <refspec> in cover letter")),
+		OPT_INTEGER(0, "creation-factor", &creation_factor,
+			    N_("percentage by which creation is weighted")),
 		OPT_END()
 	};
 
@@ -1807,6 +1810,11 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 					     _("Interdiff against v%d:"));
 	}
 
+	if (creation_factor < 0)
+		creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;
+	else if (!rdiff_prev)
+		die(_("--creation-factor requires --range-diff"));
+
 	if (rdiff_prev) {
 		if (!cover_letter)
 			die(_("--range-diff requires --cover-letter"));
@@ -1815,7 +1823,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 					origin, list[0]);
 		rev.rdiff1 = rdiff1.buf;
 		rev.rdiff2 = rdiff2.buf;
-		rev.creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;
+		rev.creation_factor = creation_factor;
 		rev.rdiff_title = diff_title(&rdiff_title, reroll_count,
 					     _("Range-diff:"),
 					     _("Range-diff against v%d:"));
-- 
2.18.0.345.g5c9ce644c3

[PATCH 14/14] format-patch: allow --range-diff to apply to a lone-patch

[Eric Sunshine]

When submitting a revised version of a patch or series, it can be
helpful (to reviewers) to include a summary of changes since the
previous attempt in the form of a range-diff, typically in the cover
letter. However, it is occasionally useful, despite making for a noisy
read, to insert a range-diff into the commentary section of the lone
patch of a 1-patch series.

Therefore, extend "git format-patch --range-diff=<refspec>" to insert a
range-diff into the commentary section of a lone patch rather than
requiring a cover letter.

Implementation note: Generating a range-diff for insertion into the
commentary section of a patch which itself is currently being generated
requires invoking the diffing machinery recursively. However, the
machinery does not (presently) support this since it uses global state.
Consequently, we need to take care to stash away the state of the
in-progress operation while generating the range-diff, and restore it
after.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 Documentation/git-format-patch.txt |  3 ++-
 builtin/log.c                      |  9 +++++----
 log-tree.c                         | 15 +++++++++++++++
 3 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 9b2e172159..aba4c5febe 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -241,7 +241,8 @@ feeding the result to `git send-email`.
 
 --range-diff=<previous>::
 	As a reviewer aid, insert a range-diff (see linkgit:git-range-diff[1])
-	into the cover letter showing the differences between the previous
+	into the cover letter, or as commentary of the lone patch of a
+	1-patch series, showing the differences between the previous
 	version of the patch series and the series currently being formatted.
 	`previous` can be a single revision naming the tip of the previous
 	series if it shares a common base with the series being formatted (for
diff --git a/builtin/log.c b/builtin/log.c
index 10c3801ceb..f0e99256a0 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1575,7 +1575,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			     N_("show changes against <rev> in cover letter or single patch"),
 			     parse_opt_object_name),
 		OPT_STRING(0, "range-diff", &rdiff_prev, N_("refspec"),
-			   N_("show changes against <refspec> in cover letter")),
+			   N_("show changes against <refspec> in cover letter or single patch")),
 		OPT_INTEGER(0, "creation-factor", &creation_factor,
 			    N_("percentage by which creation is weighted")),
 		OPT_END()
@@ -1816,8 +1816,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		die(_("--creation-factor requires --range-diff"));
 
 	if (rdiff_prev) {
-		if (!cover_letter)
-			die(_("--range-diff requires --cover-letter"));
+		if (!cover_letter && total != 1)
+			die(_("--range-diff requires --cover-letter or single patch"));
 
 		infer_range_diff_ranges(&rdiff1, &rdiff2, rdiff_prev,
 					origin, list[0]);
@@ -1866,8 +1866,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		print_signature(rev.diffopt.file);
 		total++;
 		start_number--;
-		/* interdiff in cover-letter; omit from patches */
+		/* interdiff/range-diff in cover-letter; omit from patches */
 		rev.idiff_oid1 = NULL;
+		rev.rdiff1 = NULL;
 	}
 	rev.add_signoff = do_signoff;
 
diff --git a/log-tree.c b/log-tree.c
index 56513fa83d..37afc585dc 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -15,6 +15,7 @@
 #include "line-log.h"
 #include "help.h"
 #include "interdiff.h"
+#include "range-diff.h"
 
 static struct decoration name_decoration = { "object names" };
 static int decoration_loaded;
@@ -750,6 +751,20 @@ void show_log(struct rev_info *opt)
 
 		memcpy(&diff_queued_diff, &dq, sizeof(diff_queued_diff));
 	}
+
+	if (cmit_fmt_is_mail(ctx.fmt) && opt->rdiff1) {
+		struct diff_queue_struct dq;
+
+		memcpy(&dq, &diff_queued_diff, sizeof(diff_queued_diff));
+		DIFF_QUEUE_CLEAR(&diff_queued_diff);
+
+		next_commentary_block(opt, NULL);
+		fprintf_ln(opt->diffopt.file, "%s", opt->rdiff_title);
+		show_range_diff(opt->rdiff1, opt->rdiff2,
+				opt->creation_factor, 1, &opt->diffopt);
+
+		memcpy(&diff_queued_diff, &dq, sizeof(diff_queued_diff));
+	}
 }
 
 int log_tree_diff_flush(struct rev_info *opt)
-- 
2.18.0.345.g5c9ce644c3

Re: [PATCH 02/14] format-patch: add --interdiff option to embed diff in cover letter

[Eric Sunshine]

On Sun, Jul 22, 2018 at 5:57 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
> Add an --interdiff option to automate this process. The argument to
> --interdiff specifies the tip of the previous attempt against which to
> generate the interdiff.
>
> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
> ---
> diff --git a/interdiff.c b/interdiff.c
> @@ -0,0 +1,17 @@
> +void show_interdiff(struct rev_info *rev)
> +{
> +       struct diff_options opts;
> +
> +       memcpy(&opts, &rev->diffopt, sizeof(opts));
> +       opts.output_format = DIFF_FORMAT_PATCH;
> +       diff_setup_done(&opts);
> +
> +       diff_tree_oid(rev->idiff_oid1, rev->idiff_oid2, "", &opts);

Passing a 'rev_info' to this function is overkill. It actually only
needs the two OID's and a 'diff_options'. I'll make the change if I
need to reroll for some other reason, otherwise I'll do it as a
follow-up patch.

Re: [PATCH 02/14] format-patch: add --interdiff option to embed diff in cover letter

[Duy Nguyen]

On Sun, Jul 22, 2018 at 11:57 AM Eric Sunshine <sunshine@sunshineco.com> wrote:

> diff --git a/interdiff.c b/interdiff.c
> new file mode 100644
> index 0000000000..d0fac10c7c
> --- /dev/null
> +++ b/interdiff.c
> @@ -0,0 +1,17 @@
> +#include "cache.h"
> +#include "commit.h"
> +#include "revision.h"
> +#include "interdiff.h"
> +
> +void show_interdiff(struct rev_info *rev)
> +{
> +       struct diff_options opts;
> +
> +       memcpy(&opts, &rev->diffopt, sizeof(opts));
> +       opts.output_format = DIFF_FORMAT_PATCH;
> +       diff_setup_done(&opts);
> +
> +       diff_tree_oid(rev->idiff_oid1, rev->idiff_oid2, "", &opts);
> +       diffcore_std(&opts);
> +       diff_flush(&opts);
> +}

Is it worth adding a new file just for a single function? I haven't
read the rest of the series, but the cover letter's diffstat suggests
this is it. Is interdiff intended to become a lot more complicated in
the future? If not maybe just add this function in diff-lib.c
-- 
Duy

Re: [PATCH 03/14] format-patch: teach --interdiff to respect -v/--reroll-count

[Duy Nguyen]

On Sun, Jul 22, 2018 at 11:57 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
>
> The --interdiff option introduces the embedded interdiff generically as
> "Interdiff:", however, we can do better when --reroll-count is specified

Oh boy. --reroll-count was added in 2012 and here I am typing
--subject-prefix='PATCH vX' everyday, thinking that somebody should
really do something about it. I've learned --reroll-count today!

> diff --git a/revision.h b/revision.h
> index 61931fbac5..ffeadc261a 100644
> --- a/revision.h
> +++ b/revision.h
> @@ -215,6 +215,7 @@ struct rev_info {
>         /* interdiff */
>         const struct object_id *idiff_oid1;
>         const struct object_id *idiff_oid2;
> +       const char *idiff_title;

I feel we're abusing struct rev_info a bit for this since this
interdiff thing is very builtin/log.c's business and not at all
related to rev walk. Is it possible (and easy) to just pass
idfff_title from cmd_format_patch to make_cover_letter()? If it's a
lot of code, then I guess we can just leave it here.
-- 
Duy

Re: [PATCH 06/14] format-patch: allow --interdiff to apply to a lone-patch

[Duy Nguyen]

On Sun, Jul 22, 2018 at 11:58 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
> diff --git a/log-tree.c b/log-tree.c
> index 9d38f1cf79..56513fa83d 100644
> --- a/log-tree.c
> +++ b/log-tree.c
> @@ -14,6 +14,7 @@
>  #include "sequencer.h"
>  #include "line-log.h"
>  #include "help.h"
> +#include "interdiff.h"
>
>  static struct decoration name_decoration = { "object names" };
>  static int decoration_loaded;
> @@ -736,6 +737,19 @@ void show_log(struct rev_info *opt)
>
>         strbuf_release(&msgbuf);
>         free(ctx.notes_message);
> +
> +       if (cmit_fmt_is_mail(ctx.fmt) && opt->idiff_oid1) {

OK putting idiff stuff in rev_info is probably the right choice. But
we all three fields prefixed with idiff_, maybe you could just add a
new struct "idiff_options" to contain them and add a pointer to that
struct in rev_info. Then "opt->idiff" is enough to know if idiff is
requested instead of relying on idiff_oid1 (seems too random).

> +               struct diff_queue_struct dq;
> +
> +               memcpy(&dq, &diff_queued_diff, sizeof(diff_queued_diff));
> +               DIFF_QUEUE_CLEAR(&diff_queued_diff);
> +
> +               next_commentary_block(opt, NULL);
> +               fprintf_ln(opt->diffopt.file, "%s", opt->idiff_title);
> +               show_interdiff(opt, 2);
> +
> +               memcpy(&diff_queued_diff, &dq, sizeof(diff_queued_diff));
> +       }
>  }
-- 
Duy

Re: [PATCH 10/14] format-patch: add --range-diff option to embed diff in cover letter

[Duy Nguyen]

On Sun, Jul 22, 2018 at 11:58 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
> index f8a061794d..e7f404be3d 100644
> --- a/Documentation/git-format-patch.txt
> +++ b/Documentation/git-format-patch.txt
> @@ -24,6 +24,7 @@ SYNOPSIS
>                    [--to=<email>] [--cc=<email>]
>                    [--[no-]cover-letter] [--quiet] [--notes[=<ref>]]
>                    [--interdiff=<previous>]
> +                  [--range-diff=<previous>]

I wonder if people will use both --interdiff=<rev> and
--range-diff=<rev> often enough to justify a shortcut
"--all-kinds-of-diff=<rev>" so that we don't have to type <previous>
twice. But I guess we don't have to worry about this right now.
-- 
Duy

Re: [PATCH 00/14] format-patch: add --interdiff and --range-diff options

[Duy Nguyen]

On Sun, Jul 22, 2018 at 11:57 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
>
> When re-submitting a patch series, it is often helpful (for reviewers)
> to include an interdiff or range-diff against the previous version.
> Doing so requires manually running git-diff or git-range-diff and
> copy/pasting the result into the cover letter of the new version.
>
> This series automates the process by introducing git-format-patch
> options --interdiff and --range-diff which insert such a diff into the
> cover-letter or into the commentary section of the lone patch of a
> 1-patch series. In the latter case, the interdiff or range-diff is
> indented to avoid confusing git-am and human readers.

I gave up after 10/14. But what I've seen is nice (yes I have a couple
comments here and there but you probably won't need to update
anything).
-- 
Duy

Re: [PATCH 02/14] format-patch: add --interdiff option to embed diff in cover letter

[Eric Sunshine]

On Mon, Jul 23, 2018 at 12:03 PM Duy Nguyen <pclouds@gmail.com> wrote:
> On Sun, Jul 22, 2018 at 11:57 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
> > @@ -0,0 +1,17 @@
> > +void show_interdiff(struct rev_info *rev)
> > +{
> > +       struct diff_options opts;
> > +
> > +       memcpy(&opts, &rev->diffopt, sizeof(opts));
> > +       opts.output_format = DIFF_FORMAT_PATCH;
> > +       diff_setup_done(&opts);
> > +
> > +       diff_tree_oid(rev->idiff_oid1, rev->idiff_oid2, "", &opts);
> > +       diffcore_std(&opts);
> > +       diff_flush(&opts);
> > +}
>
> Is it worth adding a new file just for a single function? I haven't
> read the rest of the series, but the cover letter's diffstat suggests
> this is it. Is interdiff intended to become a lot more complicated in
> the future? If not maybe just add this function in diff-lib.c

Good question. The functionality originally lived in builtin/log.c but
moved to log-tree.c when I added the ability to embed an interdiff in
a single patch. However, it didn't "feel" right in log-tree.c, so I
moved it to its own file to mirror how the range-diff engine resides
in its own file.

And, the function actually did several more things as originally
implemented. For instance, it took care of not clobbering global
diff-queue state, and consulting 'reroll_count' and printing the
"Interdiff:" header, but those bits eventually moved to live at more
"correct" locations, leaving this relatively minimal function behind.
It does get a bit more complex in a later patch, but not significantly
so.

I wasn't aware of diff-lib.c, but it does seem like show_interdiff()
could be at home there.

Re: [PATCH 03/14] format-patch: teach --interdiff to respect -v/--reroll-count

[Eric Sunshine]

On Mon, Jul 23, 2018 at 12:12 PM Duy Nguyen <pclouds@gmail.com> wrote:
> On Sun, Jul 22, 2018 at 11:57 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
> > @@ -215,6 +215,7 @@ struct rev_info {
> >         /* interdiff */
> >         const struct object_id *idiff_oid1;
> >         const struct object_id *idiff_oid2;
> > +       const char *idiff_title;
>
> I feel we're abusing struct rev_info a bit for this since this
> interdiff thing is very builtin/log.c's business and not at all
> related to rev walk. Is it possible (and easy) to just pass
> idfff_title from cmd_format_patch to make_cover_letter()? If it's a
> lot of code, then I guess we can just leave it here.

As originally implemented, this information was passed directly to
make_cover_letter(), however, as you discovered in your review of
patch 6/14[1], which makes it possible to embed an interdiff in the
commentary section of a lone patch, 'struct rev_info' is the only way
to pass this information down that deeply in the patch-generation
process. So, yes, this pretty much needs to use 'struct rev_info',
even if that need doesn't exist at this early step in the series.

[1]: https://public-inbox.org/git/CACsJy8Aw6R8-3kDfhCqunXziajCg9O_1WrEYc4rfKa+-=m1D5g@mail.gmail.com/

Re: [PATCH 06/14] format-patch: allow --interdiff to apply to a lone-patch

[Eric Sunshine]

On Mon, Jul 23, 2018 at 12:22 PM Duy Nguyen <pclouds@gmail.com> wrote:
> On Sun, Jul 22, 2018 at 11:58 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
> > +       if (cmit_fmt_is_mail(ctx.fmt) && opt->idiff_oid1) {
>
> OK putting idiff stuff in rev_info is probably the right choice. But
> we all three fields prefixed with idiff_, maybe you could just add a
> new struct "idiff_options" to contain them and add a pointer to that
> struct in rev_info. Then "opt->idiff" is enough to know if idiff is
> requested instead of relying on idiff_oid1 (seems too random).

I have mixed feelings about this suggestion for the following reasons:

* 'struct rev_info' already contains a number of specialized fields
which apply in only certain use cases but not others, and those fields
often are grouped textually to show relationship rather than being
bundled in a struct.

* These new fields are very specific to this particular operation and
are unlikely to ever have wider use, so adding the extra level of
indirection/abstraction (whatever you'd call it) feels overkill and,
while nice theoretically, adds complexity without much practical
value.

* Bundling these fields in a struct might incorrectly imply to readers
that these items, collectively, can be used in some general-purpose
fashion, which isn't at all the case; they are very specific to this
operation and that struct would never be used elsewhere or for any
other purpose.

The upside of bundling them in a struct, as you mention, is that
"opt->idiff" would be slightly more obvious than "opt->idiff_oid1" as
a "should we print an interdiff?" conditional. On the other hand, this
case is so specific and narrow that I'm not sure it warrants such
generality.

Re: [PATCH 10/14] format-patch: add --range-diff option to embed diff in cover letter

[Eric Sunshine]

On Mon, Jul 23, 2018 at 12:28 PM Duy Nguyen <pclouds@gmail.com> wrote:
> On Sun, Jul 22, 2018 at 11:58 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
> > diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
> > index f8a061794d..e7f404be3d 100644
> > --- a/Documentation/git-format-patch.txt
> > +++ b/Documentation/git-format-patch.txt
> > @@ -24,6 +24,7 @@ SYNOPSIS
> >                    [--to=<email>] [--cc=<email>]
> >                    [--[no-]cover-letter] [--quiet] [--notes[=<ref>]]
> >                    [--interdiff=<previous>]
> > +                  [--range-diff=<previous>]
>
> I wonder if people will use both --interdiff=<rev> and
> --range-diff=<rev> often enough to justify a shortcut
> "--all-kinds-of-diff=<rev>" so that we don't have to type <previous>
> twice. But I guess we don't have to worry about this right now.

My original thought was that --interdiff and --range-diff would be
mutually exclusive, however, I quickly realized that some people might
like to use both options together since each format has its strengths
and weaknesses. (I've used both types of diffs together when preparing
rerolls of my own series and found that, together, they provided a
better picture of the reroll than either would have alone.)

Based upon experience on this mailing list, I'd guess that most people
would use only one or the other, though that doesn't speak for other
projects. And, as you note, it's something that can be added later if
warranted (plus, this series is already long and I'd like to avoid
making it longer for something like this whose value is only
speculative).

Re: [PATCH 00/14] format-patch: add --interdiff and --range-diff options

[Eric Sunshine]

On Mon, Jul 23, 2018 at 12:32 PM Duy Nguyen <pclouds@gmail.com> wrote:
> On Sun, Jul 22, 2018 at 11:57 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
> > When re-submitting a patch series, it is often helpful (for reviewers)
> > to include an interdiff or range-diff against the previous version.
> > Doing so requires manually running git-diff or git-range-diff and
> > copy/pasting the result into the cover letter of the new version.
> >
> > This series automates the process by introducing git-format-patch
> > options --interdiff and --range-diff which insert such a diff into the
> > cover-letter or into the commentary section of the lone patch of a
> > 1-patch series. In the latter case, the interdiff or range-diff is
> > indented to avoid confusing git-am and human readers.
>
> I gave up after 10/14. But what I've seen is nice (yes I have a couple
> comments here and there but you probably won't need to update
> anything).

Thanks for the review comments.

Re: [PATCH 02/14] format-patch: add --interdiff option to embed diff in cover letter

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Mon, Jul 23, 2018 at 12:03 PM Duy Nguyen <pclouds@gmail.com> wrote:
>> On Sun, Jul 22, 2018 at 11:57 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
>> > @@ -0,0 +1,17 @@
>> > +void show_interdiff(struct rev_info *rev)
>> > +{
>> > +       struct diff_options opts;
>> > +
>> > +       memcpy(&opts, &rev->diffopt, sizeof(opts));
>> > +       opts.output_format = DIFF_FORMAT_PATCH;
>> > +       diff_setup_done(&opts);
>> > +
>> > +       diff_tree_oid(rev->idiff_oid1, rev->idiff_oid2, "", &opts);
>> > +       diffcore_std(&opts);
>> > +       diff_flush(&opts);
>> > +}
>>
>> Is it worth adding a new file just for a single function? I haven't
>> read the rest of the series, but the cover letter's diffstat suggests
>> this is it. Is interdiff intended to become a lot more complicated in
>> the future? If not maybe just add this function in diff-lib.c
>
> Good question. The functionality originally lived in builtin/log.c but
> moved to log-tree.c when I added the ability to embed an interdiff in
> a single patch. However, it didn't "feel" right in log-tree.c, so I
> moved it to its own file to mirror how the range-diff engine resides
> in its own file.

> And, the function actually did several more things as originally
> implemented. For instance, it took care of not clobbering global
> diff-queue state, and consulting 'reroll_count' and printing the
> "Interdiff:" header, but those bits eventually moved to live at more
> "correct" locations, leaving this relatively minimal function behind.
> It does get a bit more complex in a later patch, but not significantly
> so.
>
> I wasn't aware of diff-lib.c, but it does seem like show_interdiff()
> could be at home there.

Yeah, diff-lib.c is meant to be a home for the implementation of low
level "diff-{files,tree,index}" plumbing that can be called from
other routines; if you are adding "take two things, compare them"
that can be reused from places, then it is a good match.

Re: [PATCH 07/14] range-diff: respect diff_option.file rather than assuming 'stdout'

[Stefan Beller]

On Sun, Jul 22, 2018 at 2:58 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
>
> The actual diffs output by range-diff respect diff_option.file, which
> range-diff passes down the call-chain, thus are destination-agnostic.
> However, output_pair_header() is hard-coded to emit to 'stdout'. Fix
> this by making output_pair_header() respect diff_option.file, as well.
>
> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>

Depending how much the range diff series has progressed
already, it might make sense to squash it there?

Thanks,
Stefan

> ---
>  range-diff.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/range-diff.c b/range-diff.c
> index 347b4a79f2..76e053c2b2 100644
> --- a/range-diff.c
> +++ b/range-diff.c
> @@ -328,7 +328,7 @@ static void output_pair_header(struct diff_options *diffopt,
>         }
>         strbuf_addf(buf, "%s\n", color_reset);
>
> -       fwrite(buf->buf, buf->len, 1, stdout);
> +       fwrite(buf->buf, buf->len, 1, diffopt->file);
>  }
>
>  static struct userdiff_driver no_func_name = {
> --
> 2.18.0.345.g5c9ce644c3
>

Re: [PATCH 07/14] range-diff: respect diff_option.file rather than assuming 'stdout'

[Eric Sunshine]

On Mon, Jul 23, 2018 at 6:59 PM Stefan Beller <sbeller@google.com> wrote:
> On Sun, Jul 22, 2018 at 2:58 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
> > The actual diffs output by range-diff respect diff_option.file, which
> > range-diff passes down the call-chain, thus are destination-agnostic.
> > However, output_pair_header() is hard-coded to emit to 'stdout'. Fix
> > this by making output_pair_header() respect diff_option.file, as well.
> >
> > Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
>
> Depending how much the range diff series has progressed
> already, it might make sense to squash it there?

It could be done that way, though it would be nice for Dscho's
range-diff series to land without another re-roll, and it looks like
I'll probably be re-rolling this series, anyhow, to move
show_interdiff() to diff-lib.c and to fix its signature. Junio could
manage it as a "squash", but that's probably more work for him than
for me to retain it in this series. *And*, this change _is_ required
for this series, whereas it doesn't really matter for Dscho's series,
even though it's an obvious "fix".

Anyhow, whichever way Junio and Dscho would like to play it is fine with me.

Re: [PATCH 10/14] format-patch: add --range-diff option to embed diff in cover letter

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Mon, Jul 23, 2018 at 12:28 PM Duy Nguyen <pclouds@gmail.com> wrote:
>> On Sun, Jul 22, 2018 at 11:58 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
>> > diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
>> > index f8a061794d..e7f404be3d 100644
>> > --- a/Documentation/git-format-patch.txt
>> > +++ b/Documentation/git-format-patch.txt
>> > @@ -24,6 +24,7 @@ SYNOPSIS
>> >                    [--to=<email>] [--cc=<email>]
>> >                    [--[no-]cover-letter] [--quiet] [--notes[=<ref>]]
>> >                    [--interdiff=<previous>]
>> > +                  [--range-diff=<previous>]
>>
>> I wonder if people will use both --interdiff=<rev> and
>> --range-diff=<rev> often enough to justify a shortcut
>> "--all-kinds-of-diff=<rev>" so that we don't have to type <previous>
>> twice. But I guess we don't have to worry about this right now.
>
> My original thought was that --interdiff and --range-diff would be
> mutually exclusive, however, I quickly realized that some people might
> like to use both options together since each format has its strengths
> and weaknesses. (I've used both types of diffs together when preparing
> rerolls of my own series and found that, together, they provided a
> better picture of the reroll than either would have alone.)
>
> Based upon experience on this mailing list, I'd guess that most people
> would use only one or the other, though that doesn't speak for other
> projects. And, as you note, it's something that can be added later if
> warranted (plus, this series is already long and I'd like to avoid
> making it longer for something like this whose value is only
> speculative).

A few random thoughts.

 * I find it somewhat disturbing that use of dash is inconsistent
   between "--interdiff=<arg>" and "--range-diff=<arg>".

 * Perhaps "--interdiff=<previous> --range-diff" may be a possible
   way to say "use the same <previous> and show both"?  Do we want
   "--range-diff=<previous> --interdiff" to mean the same two
   meta-diff but shown in different order?

 * Do we expect that we may find a third-kind of "meta-diff" that
   sits next to interdiff and range-diff in the future?  I *think*
   two separate options "--interdiff=..." and "--range-diff=..." is
   still a good way forward, and we'd add "--frotzdiff=..." when
   such a third-kind of "meta-diff" turns out to be useful, without
   fearing proliferation of options, and that would be OK, but I am
   just thinking aloud to see if other people have better ideas.

Re: [PATCH 10/14] format-patch: add --range-diff option to embed diff in cover letter

[Duy Nguyen]

On Mon, Jul 23, 2018 at 9:59 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
>
> On Mon, Jul 23, 2018 at 12:28 PM Duy Nguyen <pclouds@gmail.com> wrote:
> > On Sun, Jul 22, 2018 at 11:58 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
> > > diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
> > > index f8a061794d..e7f404be3d 100644
> > > --- a/Documentation/git-format-patch.txt
> > > +++ b/Documentation/git-format-patch.txt
> > > @@ -24,6 +24,7 @@ SYNOPSIS
> > >                    [--to=<email>] [--cc=<email>]
> > >                    [--[no-]cover-letter] [--quiet] [--notes[=<ref>]]
> > >                    [--interdiff=<previous>]
> > > +                  [--range-diff=<previous>]
> >
> > I wonder if people will use both --interdiff=<rev> and
> > --range-diff=<rev> often enough to justify a shortcut
> > "--all-kinds-of-diff=<rev>" so that we don't have to type <previous>
> > twice. But I guess we don't have to worry about this right now.
>
> My original thought was that --interdiff and --range-diff would be
> mutually exclusive, however, I quickly realized that some people might
> like to use both options together since each format has its strengths
> and weaknesses. (I've used both types of diffs together when preparing
> rerolls of my own series and found that, together, they provided a
> better picture of the reroll than either would have alone.)

I actually had another question that I answered myself: how do I know
which one to choose? There's no preview option (and I'm lazy, I don't
want to do separate diff commands myself). So my answer was "choose
both, then delete the one that does not look good (and explain it in
the cover too when I delete it)"

> And, as you note, it's something that can be added later if
> warranted (plus, this series is already long and I'd like to avoid
> making it longer for something like this whose value is only
> speculative).

Yes of course. We can revisit this later.
-- 
Duy

Re: [PATCH 07/14] range-diff: respect diff_option.file rather than assuming 'stdout'

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Mon, Jul 23, 2018 at 6:59 PM Stefan Beller <sbeller@google.com> wrote:
>> On Sun, Jul 22, 2018 at 2:58 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
>> > The actual diffs output by range-diff respect diff_option.file, which
>> > range-diff passes down the call-chain, thus are destination-agnostic.
>> > However, output_pair_header() is hard-coded to emit to 'stdout'. Fix
>> > this by making output_pair_header() respect diff_option.file, as well.
>> >
>> > Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
>>
>> Depending how much the range diff series has progressed
>> already, it might make sense to squash it there?
>
> It could be done that way, though it would be nice for Dscho's
> range-diff series to land without another re-roll, and it looks like
> I'll probably be re-rolling this series, anyhow, to move
> show_interdiff() to diff-lib.c and to fix its signature. Junio could
> manage it as a "squash", but that's probably more work for him than
> for me to retain it in this series. *And*, this change _is_ required
> for this series, whereas it doesn't really matter for Dscho's series,
> even though it's an obvious "fix".
>
> Anyhow, whichever way Junio and Dscho would like to play it is fine with me.

Having it at the beginning of your series (I consider 1-6/14 to be a
separate series, on top of which another series whose first change
is this 7/14 is builds upon) like you do here is probably fine.

Re: [PATCH 10/14] format-patch: add --range-diff option to embed diff in cover letter

[Eric Sunshine]

On Wed, Jul 25, 2018 at 1:29 PM Junio C Hamano <gitster@pobox.com> wrote:
> A few random thoughts.
>
>  * I find it somewhat disturbing that use of dash is inconsistent
>    between "--interdiff=<arg>" and "--range-diff=<arg>".

I went with the common spellings we've seen on the mailing lists.
"Interdiff", in particular, seems well established. "Range-diff" is
new, so we don't have much data other than what we saw during the
discussion when renaming git-branch-diff, and, of course,
git-branch-diff itself is hyphenated. Another consideration: "inter"
is a prefix, whereas "range" stands on its own.

I don't have a super strong opinion, as I'm used to both the
hyphenated name (from discussion and the command name itself), and the
unhyphenated name ("rangediff" was my local branch name). I'm open to
opinions.

We probably wouldn't want to do so, but another possibility is to
recognize both --range-diff and --rangediff.

>  * Perhaps "--interdiff=<previous> --range-diff" may be a possible
>    way to say "use the same <previous> and show both"?  Do we want
>    "--range-diff=<previous> --interdiff" to mean the same two
>    meta-diff but shown in different order?

That's not at all a bad shorthand. I like it better than the
"--all-kinds-of-diff=<rev>" mentioned earlier. We don't need to make a
decision for this series since such functionality can be added later.

The order of specification of the two options affecting the order of
output is also an interesting idea. We may want to decide that before
graduating this topic.

>  * Do we expect that we may find a third-kind of "meta-diff" that
>    sits next to interdiff and range-diff in the future?  I *think*
>    two separate options "--interdiff=..." and "--range-diff=..." is
>    still a good way forward, and we'd add "--frotzdiff=..." when
>    such a third-kind of "meta-diff" turns out to be useful, without
>    fearing proliferation of options, and that would be OK, but I am
>    just thinking aloud to see if other people have better ideas.

I did consider other approaches, such as a more generic option. For
example, --embed=range-diff:<prev>. I also considered supporting more
complex use-cases. For instance, git-range-diff itself accepts the two
ranges in several formats ("a..b c..d" or "x...z" or "i j k"), plus
the creation-factor can be tweaked, so I weighed ways of incorporating
all that detail into the single argument to --range-diff.

However, those are all difficult to use (onerous to type and remember)
and difficult to document. Thus, I opted for simplicity of individual,
straight-forward options (--interdiff, --range-diff,
--creation-factor), which are easy to type, remember, and document.

We may want to revisit this later if git-format-patch grows additional
similar options.

Re: [PATCH 10/14] format-patch: add --range-diff option to embed diff in cover letter

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> I did consider other approaches, such as a more generic option. For
> example, --embed=range-diff:<prev>. I also considered supporting more
> complex use-cases. For instance, git-range-diff itself accepts the two
> ranges in several formats ("a..b c..d" or "x...z" or "i j k"), plus
> the creation-factor can be tweaked, so I weighed ways of incorporating
> all that detail into the single argument to --range-diff.
>
> However, those are all difficult to use (onerous to type and remember)
> and difficult to document. Thus, I opted for simplicity of individual,
> straight-forward options (--interdiff, --range-diff,
> --creation-factor), which are easy to type, remember, and document.

Yes, that matches my conclusion that it is OK to have these on
separate options that can grow as many meta-diff formats we would
support.

Re: [PATCH 11/14] format-patch: extend --range-diff to accept revision range

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> When submitting a revised a patch series, the --range-diff option embeds
> a range-diff in the cover letter showing changes since the previous
> version of the patch series. The argument to --range-diff is a simple
> revision naming the tip of the previous series, which works fine if the
> previous and current versions of the patch series share a common base.
>
> However, it fails if the revision ranges of the old and new versions of
> the series are disjoint. To address this shortcoming, extend
> --range-diff to also accept an explicit revision range for the previous
> series. For example:
>
>     git format-patch --cover-letter --range-diff=v1~3..v1 -3 v2
>
> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
> ---

Makes sense.  Even though a single "common rev" would serve as a
feature to discourage rebasing done "just to catch up" without a
good reason, it is a good idea to give an escape hatch like this
to support a case where rebasing is the right thing to do.

>  static void infer_range_diff_ranges(struct strbuf *r1,
>  				    struct strbuf *r2,
>  				    const char *prev,
> +				    struct commit *origin,
>  				    struct commit *head)
>  {
>  	const char *head_oid = oid_to_hex(&head->object.oid);
>  
> -	strbuf_addf(r1, "%s..%s", head_oid, prev);
> -	strbuf_addf(r2, "%s..%s", prev, head_oid);

I thought "git range-diff" also took the three-dot notation as a
short-hand but there is no point using that in this application,
which wants the RHS and the LHS range in separate output variables.

> +	if (!strstr(prev, "..")) {
> +		strbuf_addf(r1, "%s..%s", head_oid, prev);
> +		strbuf_addf(r2, "%s..%s", prev, head_oid);
> +	} else if (!origin) {
> +		die(_("failed to infer range-diff ranges"));
> +	} else {
> +		strbuf_addstr(r1, prev);
> +		strbuf_addf(r2, "%s..%s",
> +			    oid_to_hex(&origin->object.oid), head_oid);

Interesting.

I actually would feel better to see the second range for the normal
case to be computed exactly the same way, i.e.

	int prev_is_range = strstr(prev, "..");

	if (prev_is_range)
		strbuf_addstr(r1, prev);
	else
		strbuf_addf(r1, "%s..%s", head_oid, prev);

	if (origin)
		strbuf_addf(r2, "%s..%s", oid_to_hex(&origin->object.oid, head_oid);
	else if (prev_is_range)
		die("cannot figure out the origin of new series");
	else {
		warning("falling back to use '%s' as the origin of new series",	prev);
		strbuf_addf(r2, "%s..%s", prev, head_oid);
	}

because origin..HEAD is always the set of the "new" series, no
matter what "prev" the user chooses to compare that series against,
when there is a single "origin".

Re: [PATCH 14/14] format-patch: allow --range-diff to apply to a lone-patch

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> @@ -750,6 +751,20 @@ void show_log(struct rev_info *opt)
>  
>  		memcpy(&diff_queued_diff, &dq, sizeof(diff_queued_diff));
>  	}
> +
> +	if (cmit_fmt_is_mail(ctx.fmt) && opt->rdiff1) {
> +		struct diff_queue_struct dq;
> +
> +		memcpy(&dq, &diff_queued_diff, sizeof(diff_queued_diff));
> +		DIFF_QUEUE_CLEAR(&diff_queued_diff);
> +
> +		next_commentary_block(opt, NULL);
> +		fprintf_ln(opt->diffopt.file, "%s", opt->rdiff_title);
> +		show_range_diff(opt->rdiff1, opt->rdiff2,
> +				opt->creation_factor, 1, &opt->diffopt);
> +
> +		memcpy(&diff_queued_diff, &dq, sizeof(diff_queued_diff));
> +	}
>  }
>  
>  int log_tree_diff_flush(struct rev_info *opt)

This essentially repeats what is already done for "interdiff".

Does the global diff_queued_diff gets cleaned up when
show_interdiff() and show_range_diff() return, like diff_flush()
does?  Otherwise we'd be leaking the filepairs accumulated in the
diff_queued_diff.

Re: [PATCH 14/14] format-patch: allow --range-diff to apply to a lone-patch

[Eric Sunshine]

On Wed, Jul 25, 2018 at 5:07 PM Junio C Hamano <gitster@pobox.com> wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
> > +     if (cmit_fmt_is_mail(ctx.fmt) && opt->rdiff1) {
> > +             struct diff_queue_struct dq;
> > +
> > +             memcpy(&dq, &diff_queued_diff, sizeof(diff_queued_diff));
> > +             DIFF_QUEUE_CLEAR(&diff_queued_diff);
> > +
> > +             next_commentary_block(opt, NULL);
> > +             fprintf_ln(opt->diffopt.file, "%s", opt->rdiff_title);
> > +             show_range_diff(opt->rdiff1, opt->rdiff2,
> > +                             opt->creation_factor, 1, &opt->diffopt);
> > +
> > +             memcpy(&diff_queued_diff, &dq, sizeof(diff_queued_diff));
> > +     }
> >  }
>
> This essentially repeats what is already done for "interdiff".

Yes, the two blocks are very similar, although they access different
members of 'rev_info' and call different functions to perform the
actual diff. I explored ways of avoiding the repeated boilerplate
(using macros or passing a function pointer to a driver function
containing the boilerplate), but the end result was uglier and harder
to understand due to the added abstraction. Introducing
next_commentary_block()[1] reduced the repetition a bit.

> Does the global diff_queued_diff gets cleaned up when
> show_interdiff() and show_range_diff() return, like diff_flush()
> does?  Otherwise we'd be leaking the filepairs accumulated in the
> diff_queued_diff.

Both show_interdiff() and show_range_diff() call diff_flush(). So, the
"temporary" diff_queued_diff set up here does get cleaned up by
diff_flush(), as far as I understand (though this is my first foray
into the diff-generation code, so I may be missing something). And,
the diff_queued_diff which gets "interrupted" by this excursion into
interdiff/range-diff gets cleaned up normally when the interrupted
diff operation completes.

[1]: https://public-inbox.org/git/20180722095717.17912-6-sunshine@sunshineco.com/

Re: [PATCH 11/14] format-patch: extend --range-diff to accept revision range

[Eric Sunshine]

On Wed, Jul 25, 2018 at 4:53 PM Junio C Hamano <gitster@pobox.com> wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
> > When submitting a revised a patch series, the --range-diff option embeds
> > a range-diff in the cover letter showing changes since the previous
> > version of the patch series. The argument to --range-diff is a simple
> > revision naming the tip of the previous series [...]
> >
> > However, it fails if the revision ranges of the old and new versions of
> > the series are disjoint. To address this shortcoming, extend
> > --range-diff to also accept an explicit revision range for the previous
> > series. [...]
>
> >  static void infer_range_diff_ranges(struct strbuf *r1,
> >  {
> > -     strbuf_addf(r1, "%s..%s", head_oid, prev);
> > -     strbuf_addf(r2, "%s..%s", prev, head_oid);
>
> I thought "git range-diff" also took the three-dot notation as a
> short-hand but there is no point using that in this application,
> which wants the RHS and the LHS range in separate output variables.

Correct.

> I actually would feel better to see the second range for the normal
> case to be computed exactly the same way, i.e.
>
>         int prev_is_range = strstr(prev, "..");
>
>         if (prev_is_range)
>                 strbuf_addstr(r1, prev);
>         else
>                 strbuf_addf(r1, "%s..%s", head_oid, prev);
>
>         if (origin)
>                 strbuf_addf(r2, "%s..%s", oid_to_hex(&origin->object.oid, head_oid);
>         else if (prev_is_range)
>                 die("cannot figure out the origin of new series");
>         else {
>                 warning("falling back to use '%s' as the origin of new series", prev);
>                 strbuf_addf(r2, "%s..%s", prev, head_oid);
>         }
>
> because origin..HEAD is always the set of the "new" series, no
> matter what "prev" the user chooses to compare that series against,
> when there is a single "origin".

I plan to submit a short series as incremental changes atop
es/format-patch-{interdiff,rangediff} to address the few review
comments[1] (including my own[2]), so I can make this change, as well.

[1]: https://public-inbox.org/git/CAPig+cSuYUYSPTuKx08wcmQM-G12_-W2T4BS07fA=6grM1b8Gw@mail.gmail.com/T/#rfaf5a563e81b862bd8ed69232040056ab9a86dd8
[2]: https://public-inbox.org/git/CAPig+cSuYUYSPTuKx08wcmQM-G12_-W2T4BS07fA=6grM1b8Gw@mail.gmail.com/