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

[PATCH 2/2] format-patch: allow for independent diff & range-diff options

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Nov 28, 2018, 20:18 UTC
Message-ID
<20181128201852.9782-3-avarab@gmail.com>
In-Reply-To
<xmqqk1l32jo2.fsf@gitster-ct.c.googlers.com>

Change the semantics of the "--range-diff" option so that the regular diff options can be provided separately for the range-diff and the patch. This allows for supplying e.g. --range-diff-U0 and -U1 to "format-patch" to provide different context for the range-diff and the patch. This wasn't possible before.

Ever since the "--range-diff" option was added in 31e2617a5f ("format-patch: add --range-diff option to embed diff in cover letter", 2018-07-22) the "rev->diffopt" we pass down to the diff machinery has been the one we get from "format-patch"'s own setup_revisions().

This sort of thing is unique among the log-like commands in builtin/log.c, no command than format-patch will embed the output of another log-like command. Since the "rev->diffopt" is reused we need to munge it before we pass it to show_range_diff(). See 43dafc4172 ("format-patch: don't include --stat with --range-diff output", 2018-11-22) for a related regression fix which is being mostly reverted here.

Implementation notes: 1) We're not bothering with the full teardown around die() and will leak memory, but it's too much boilerplate to do all the frees with/without the die() and not worth it. 2) We call repo_init_revisions() for "rd_rev" even though we could get away with a shallow copy like the code we're replacing (and which show_range_diff() itself does). This is to make this code more easily understood.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/git-format-patch.txt | 10 ++++++-
 builtin/log.c                      | 42 +++++++++++++++++++++++-------
 t/t3206-range-diff.sh              | 41 +++++++++++++++++++++++++++++
 3 files changed, 82 insertions(+), 11 deletions(-)
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index aba4c5febe..6c048f415f 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -24,7 +24,8 @@ SYNOPSIS
 		   [--to=<email>] [--cc=<email>]
 		   [--[no-]cover-letter] [--quiet] [--notes[=<ref>]]
 		   [--interdiff=<previous>]
-		   [--range-diff=<previous> [--creation-factor=<percent>]]
+		   [--range-diff=<previous> [--creation-factor=<percent>]
+		      [--range-diff<common diff option>]]
 		   [--progress]
 		   [<common diff options>]
 		   [ <since> | <revision range> ]
@@ -257,6 +258,13 @@ feeding the result to `git send-email`.
 	creation/deletion cost fudge factor. See linkgit:git-range-diff[1])
 	for details.
 
+--range-diff<common diff option>::
+	Other options prefixed with `--range-diff` are stripped of
+	that prefix and passed as-is to the diff machinery used to
+	generate the range-diff, e.g. `--range-diff-U0` and
+	`--range-diff--no-color`. This allows for adjusting the format
+	of the range-diff independently from the patch itself.
+
 --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 02d88fa233..7658e56ecc 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1023,7 +1023,8 @@ static void show_diffstat(struct rev_info *rev,
 	fprintf(rev->diffopt.file, "\n");
 }
 
-static void make_cover_letter(struct rev_info *rev, int use_stdout,
+static void make_cover_letter(struct rev_info *rev, struct rev_info *rd_rev,
+			      int use_stdout,
 			      struct commit *origin,
 			      int nr, struct commit **list,
 			      const char *branch_name,
@@ -1095,13 +1096,9 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 	}
 
 	if (rev->rdiff1) {
-		struct diff_options opts;
-		memcpy(&opts, &rev->diffopt, sizeof(opts));
-		opts.output_format &= ~(DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_SUMMARY);
-
 		fprintf_ln(rev->diffopt.file, "%s", rev->rdiff_title);
 		show_range_diff(rev->rdiff1, rev->rdiff2,
-				rev->creation_factor, 1, &opts);
+				rev->creation_factor, 1, &rd_rev->diffopt);
 	}
 }
 
@@ -1485,6 +1482,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	struct commit *commit;
 	struct commit **list = NULL;
 	struct rev_info rev;
+	struct rev_info rd_rev;
 	struct setup_revision_opt s_r_opt;
 	int nr = 0, total, i;
 	int use_stdout = 0;
@@ -1603,6 +1601,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	init_log_defaults();
 	git_config(git_format_config, NULL);
 	repo_init_revisions(the_repository, &rev, prefix);
+	repo_init_revisions(the_repository, &rd_rev, prefix);
 	rev.commit_format = CMIT_FMT_EMAIL;
 	rev.expand_tabs_in_log_default = 0;
 	rev.verbose_header = 1;
@@ -1689,8 +1688,32 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	rev.preserve_subject = keep_subject;
 
 	argc = setup_revisions(argc, argv, &rev, &s_r_opt);
-	if (argc > 1)
-		die(_("unrecognized argument: %s"), argv[1]);
+	if (argc > 1) {
+		struct argv_array args = ARGV_ARRAY_INIT;
+		const char *prefix = "--range-diff";
+		int have_prefix = 0;
+
+		for (i = 0; i < argc; i++) {
+			struct strbuf sb = STRBUF_INIT;
+			char *str;
+
+			strbuf_addstr(&sb, argv[i]);
+			if (starts_with(argv[i], prefix)) {
+				have_prefix = 1;
+				strbuf_remove(&sb, 0, strlen(prefix));
+			}
+			str = strbuf_detach(&sb, NULL);
+			strbuf_release(&sb);
+
+			argv_array_push(&args, str);
+		}
+
+		if (!have_prefix)
+			die(_("unrecognized argument: %s"), argv[1]);
+		argc = setup_revisions(args.argc, args.argv, &rd_rev, NULL);
+		if (argc > 1)
+			die(_("unrecognized argument: %s"), argv[1]);
+	}
 
 	if (rev.diffopt.output_format & DIFF_FORMAT_NAME)
 		die(_("--name-only does not make sense"));
@@ -1702,7 +1725,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	if (!use_patch_format &&
 		(!rev.diffopt.output_format ||
 		 rev.diffopt.output_format == DIFF_FORMAT_PATCH))
-		/* Needs to be mirrored in show_range_diff() invocation */
 		rev.diffopt.output_format = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_SUMMARY;
 	if (!rev.diffopt.stat_width)
 		rev.diffopt.stat_width = MAIL_DEFAULT_WRAP;
@@ -1877,7 +1899,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	if (cover_letter) {
 		if (thread)
 			gen_message_id(&rev, "cover");
-		make_cover_letter(&rev, use_stdout,
+		make_cover_letter(&rev, &rd_rev, use_stdout,
 				  origin, nr, list, branch_name, quiet);
 		print_bases(&bases, rev.diffopt.file);
 		print_signature(rev.diffopt.file);
diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
index bc5facc1cd..6916103888 100755
--- a/t/t3206-range-diff.sh
+++ b/t/t3206-range-diff.sh
@@ -308,6 +308,35 @@ test_expect_success 'format-patch with <common diff option>' '
 		--range-diff=topic~..topic changed~..changed >actual.raw &&
 	sed -ne "/^1:/,/^--/p" <actual.raw >actual.range-diff &&
 	sed -e "s|:$||" >expect <<-\EOF &&
+	1:  a63e992 ! 1:  d966c5c s/12/B/
+	    @@ -8,7 +8,7 @@
+	     @@
+	      9
+	      10
+	    - B
+	    + BB
+	     -12
+	     +B
+	      13
+	-- :
+	EOF
+	test_cmp expect actual.range-diff &&
+	sed -ne "/^--- /,/^--/p" <actual.raw >actual.diff &&
+	sed -e "s|:$||" >expect <<-\EOF &&
+	--- a/file
+	+++ b/file
+	@@ -12 +12 @@ BB
+	-12
+	+B
+	-- :
+	EOF
+	test_cmp expect actual.diff &&
+
+	# -U0 & --range-diff-U0
+	git format-patch --cover-letter --stdout -U0 --range-diff-U0 \
+		--range-diff=topic~..topic changed~..changed >actual.raw &&
+	sed -ne "/^1:/,/^--/p" <actual.raw >actual.range-diff &&
+	sed -e "s|:$||" >expect <<-\EOF &&
 	1:  a63e992 ! 1:  d966c5c s/12/B/
 	    @@ -11 +11 @@
 	    - B
@@ -327,4 +356,16 @@ test_expect_success 'format-patch with <common diff option>' '
 	test_cmp expect actual.diff
 '
 
+test_expect_success 'format-patch option parsing with --range-diff-*' '
+	test_must_fail git format-patch --stdout --unknown \
+		master..unmodified 2>stderr &&
+	test_i18ngrep "unrecognized argument: --unknown" stderr &&
+	test_must_fail git format-patch --stdout --range-diff-unknown \
+		master..unmodified 2>stderr &&
+	test_i18ngrep "unrecognized argument: --range-diff-unknown" stderr &&
+	test_must_fail git format-patch --stdout --unknown --range-diff-unknown \
+		master..unmodified 2>stderr &&
+	test_i18ngrep "unrecognized argument: --unknown" stderr
+'
+
 test_done

-- 
2.20.0.rc1.387.gf8505762e3
Previous: Junio C HamanoNext: Junio C Hamano
Message 11 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.