git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[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)

From
Junio C Hamano <gitster@pobox.com>
Date
Nov 30, 2018, 04:27 UTC
Message-ID
<xmqqwoovkx5s.fsf_-_@gitster-ct.c.googlers.com>
In-Reply-To
<xmqq7egvmh54.fsf@gitster-ct.c.googlers.com>
Junio C Hamano <gitster@pobox.com> writes:
Show 16 quoted lines
> 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 <gitster@pobox.com>
---
 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);
Previous: Junio C HamanoNext: Junio C Hamano
Message 21 of 53 in “[ANNOUNCE] Git v2.20.0-rc1”
  1. Junio C HamanoNov 21, 2018
  2. Ævar Arnfjörð BjarmasonNov 22, 2018
  3. Eric SunshineNov 22, 2018
  4. 0/2 format-patch: pre-2.20 range-diff regression fixÆvar Arnfjörð Bjarmason, Nov 22, 2018
  5. 1/2 format-patch: add a more exhaustive --range-diff testÆvar Arnfjörð Bjarmason, Nov 22, 2018
  6. Junio C HamanoNov 24, 2018
  7. Ævar Arnfjörð BjarmasonNov 24, 2018
  8. 2/2 format-patch: don't include --stat with --range-diff outputÆvar Arnfjörð Bjarmason, Nov 22, 2018
  9. Junio C HamanoNov 24, 2018
  10. Junio C HamanoNov 24, 2018
  11. 2/2 format-patch: allow for independent diff & range-diff optionsÆvar Arnfjörð Bjarmason, Nov 28, 2018
  12. Junio C HamanoNov 29, 2018
  13. Johannes SchindelinNov 29, 2018
  14. Ævar Arnfjörð BjarmasonNov 29, 2018
  15. Johannes SchindelinNov 29, 2018
  16. Ævar Arnfjörð BjarmasonNov 29, 2018
  17. Johannes SchindelinNov 29, 2018
  18. Ævar Arnfjörð BjarmasonNov 29, 2018
  19. Johannes SchindelinNov 29, 2018
  20. Junio C HamanoNov 30, 2018
  21. 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, Nov 30, 2018
  22. Junio C HamanoNov 30, 2018
  23. Ævar Arnfjörð BjarmasonNov 30, 2018
  24. Johannes SchindelinNov 30, 2018
  25. Eric SunshineNov 30, 2018
  26. Martin ÅgrenDec 3, 2018
  27. range-diff: always pass at least minimal diff optionsMartin Ågren, Dec 3, 2018
  28. range-diff: always pass at least minimal diff optionsEric Sunshine, Dec 3, 2018
  29. Junio C HamanoDec 4, 2018
  30. Martin ÅgrenDec 4, 2018
  31. Eric SunshineNov 30, 2018
  32. 0/2 format-patch: fix root cause of recent regressionÆvar Arnfjörð Bjarmason, Nov 28, 2018
  33. 1/2 format-patch: add test for --range-diff diff outputÆvar Arnfjörð Bjarmason, Nov 28, 2018
  34. Ævar Arnfjörð BjarmasonNov 24, 2018
  35. Junio C HamanoNov 25, 2018
  36. rebase: mark the C reimplementation as an experimental opt-in feature (was Re: [ANNOUNCE] Git v2.20.0-rc1)Junio C Hamano, Nov 26, 2018
  37. Jonathan NiederNov 28, 2018
  38. Johannes SchindelinNov 28, 2018
  39. Ævar Arnfjörð BjarmasonNov 28, 2018
  40. Junio C HamanoNov 29, 2018
  41. Johannes SchindelinNov 29, 2018
  42. Ian JacksonNov 29, 2018
  43. Johannes SchindelinNov 29, 2018
  44. Ian JacksonNov 29, 2018
  45. Johannes SchindelinNov 29, 2018
  46. Ian JacksonNov 29, 2018
  47. Johannes SchindelinNov 26, 2018
  48. Johannes SchindelinNov 26, 2018
  49. Junio C HamanoNov 28, 2018
  50. Johannes SchindelinNov 28, 2018
  51. Junio C HamanoNov 26, 2018
  52. Elijah NewrenNov 26, 2018
  53. Junio C HamanoNov 27, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.