[PATCH] format-patch: do not let its diff-options affect --range-diff (was Re: [PATCH 2/2] format-patch: allow for independent diff & range-diff options)

Junio C Hamano [off-list ref] writes:

In any case, I tend to agree with the conclusion in the downthread
by Dscho that we should just clearly mark that invocations of the
"format-patch --range-diff" command with additional diff options is
an experimental feature that may not do anything sensible in the
upcoming release, and declare that the UI to pass diff options to
affect only the range-diff part may later be invented.  IOW, I am
coming a bit stronger than Dscho's suggestion in that we should not
even pretend that we aimed to make the options used for range-diff
customizable when driven from format-patch in the upcoming release,
or aimed to make --range-diff option compatible with other diff
options given to the format-patch command.

I had to delay -rc2 to see these last minute tweaks come to some
reasonable place to stop at, and I do not think we want to delay the
final any longer or destablizing it further by piling last minute
undercooked changes on top.

So how about doing this on top of 'master' instead?  As this leaks
*no* information wrt how range-diff machinery should behave from the
format-patch side by not passing any diffopt, as long as the new
code I added to show_range_diff() comes up with a reasonable default
diffopts (for which I really would appreciate extra sets of eyes to
make sure), this change by definition cannot be wrong (famous last
words).

-- >8 --
Subject: format-patch: do not let its diff-options affect --range-diff

Stop leaking how the primary output of format-patch is customized to
the range-diff machinery and instead let the latter use its own
"reasonable default", in order to correct the breakage introduced by
a5170794 ("Merge branch 'ab/range-diff-no-patch'", 2018-11-18) on
the 'master' front.  "git format-patch --range-diff..." without any
weird diff option started to include the "range-diff --stat" output,
which is rather useless right now, that made the whole thing
unusable and this is probably the least disruptive way to whip the
codebase into a shippable shape.

We may want to later make the range-diff driven by format-patch more
configurable, but that would have to wait until we have a good
design.

Signed-off-by: Junio C Hamano <redacted>
---
 Documentation/git-format-patch.txt | 5 +++++
 builtin/log.c                      | 2 +-
 log-tree.c                         | 2 +-
 range-diff.c                       | 6 +++++-
 range-diff.h                       | 5 +++++
 5 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index aba4c5febe..27304428a1 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt

@@ -250,6 +250,11 @@ feeding the result to `git send-email`.
 	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`).
++
+Note that diff options passed to the command affect how the primary
+product of `format-patch` is generated, and they are not passed to
+the underlying `range-diff` machinery used to generate the cover-letter
+material (this may change in the future).
 
 --creation-factor=<percent>::
 	Used with `--range-diff`, tweak the heuristic which matches up commits

diff --git a/builtin/log.c b/builtin/log.c
index 0fe6f9ba1e..5ac18e2848 100644
--- a/builtin/log.c
+++ b/builtin/log.c

@@ -1096,7 +1096,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 	if (rev->rdiff1) {
 		fprintf_ln(rev->diffopt.file, "%s", rev->rdiff_title);
 		show_range_diff(rev->rdiff1, rev->rdiff2,
-				rev->creation_factor, 1, &rev->diffopt);
+				rev->creation_factor, 1, NULL);
 	}
 }
 

diff --git a/log-tree.c b/log-tree.c
index 7a83e99250..b243779a0b 100644
--- a/log-tree.c
+++ b/log-tree.c

@@ -762,7 +762,7 @@ void show_log(struct rev_info *opt)
 		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);
+				opt->creation_factor, 1, NULL);
 
 		memcpy(&diff_queued_diff, &dq, sizeof(diff_queued_diff));
 	}

diff --git a/range-diff.c b/range-diff.c
index 767af8c5bb..8e52a85c19 100644
--- a/range-diff.c
+++ b/range-diff.c

@@ -460,7 +460,11 @@ int show_range_diff(const char *range1, const char *range2,
 		struct diff_options opts;
 		struct strbuf indent = STRBUF_INIT;
 
-		memcpy(&opts, diffopt, sizeof(opts));
+		if (diffopt)
+			memcpy(&opts, diffopt, sizeof(opts));
+		else
+			repo_diff_setup(the_repository, &opts);
+
 		if (!opts.output_format)
 			opts.output_format = DIFF_FORMAT_PATCH;
 		opts.flags.suppress_diff_headers = 1;

diff --git a/range-diff.h b/range-diff.h
index 190593f0c7..08a50b6e98 100644
--- a/range-diff.h
+++ b/range-diff.h

@@ -5,6 +5,11 @@
 
 #define RANGE_DIFF_CREATION_FACTOR_DEFAULT 60
 
+/*
+ * Compare series of commmits 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,
 		    struct diff_options *diffopt);