All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Eric Sunshine" <sunshine@sunshineco.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH 2/2] format-patch: allow for independent diff & range-diff options
Date: Wed, 28 Nov 2018 21:18:52 +0100	[thread overview]
Message-ID: <20181128201852.9782-3-avarab@gmail.com> (raw)
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


  parent reply	other threads:[~2018-11-28 20:24 UTC|newest]

Thread overview: 97+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-21 15:20 [ANNOUNCE] Git v2.20.0-rc1 Junio C Hamano
2018-11-22 15:58 ` Ævar Arnfjörð Bjarmason
2018-11-22 19:27   ` Eric Sunshine
2018-11-22 21:12     ` [PATCH 0/2] format-patch: pre-2.20 range-diff regression fix Ævar Arnfjörð Bjarmason
2018-11-22 21:12     ` [PATCH 1/2] format-patch: add a more exhaustive --range-diff test Ævar Arnfjörð Bjarmason
2018-11-24  4:14       ` Junio C Hamano
2018-11-24 11:45         ` Ævar Arnfjörð Bjarmason
2018-11-22 21:12     ` [PATCH 2/2] format-patch: don't include --stat with --range-diff output Ævar Arnfjörð Bjarmason
2018-11-24  2:26       ` Junio C Hamano
2018-11-24  4:17         ` Junio C Hamano
2018-11-28 20:18           ` [PATCH 0/2] format-patch: fix root cause of recent regression Ævar Arnfjörð Bjarmason
2018-11-28 20:18           ` [PATCH 1/2] format-patch: add test for --range-diff diff output Ævar Arnfjörð Bjarmason
2018-11-28 20:18           ` Ævar Arnfjörð Bjarmason [this message]
2018-11-29  2:59             ` [PATCH 2/2] format-patch: allow for independent diff & range-diff options Junio C Hamano
2018-11-29 10:07             ` Johannes Schindelin
2018-11-29 10:30               ` Ævar Arnfjörð Bjarmason
2018-11-29 12:12                 ` Johannes Schindelin
2018-11-29 14:35                   ` Ævar Arnfjörð Bjarmason
2018-11-29 15:41                     ` Johannes Schindelin
2018-11-29 16:03                       ` Ævar Arnfjörð Bjarmason
2018-11-29 19:03                         ` Johannes Schindelin
2018-11-30  2:30                         ` Junio C Hamano
2018-11-30  4:27                           ` [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
2018-11-30  8:57                             ` Junio C Hamano
2018-11-30  9:24                               ` Ævar Arnfjörð Bjarmason
2018-11-30 12:32                               ` Johannes Schindelin
2018-11-30  9:31                             ` Eric Sunshine
2018-12-03 13:27                               ` Martin Ågren
2018-12-03 20:07                                 ` [PATCH v2] range-diff: always pass at least minimal diff options Martin Ågren
2018-12-03 21:21                                   ` [PATCH v3] " Eric Sunshine
2018-12-04  1:35                                     ` Junio C Hamano
2018-12-04  5:40                                     ` Martin Ågren
2018-11-30  9:58                         ` [PATCH 2/2] format-patch: allow for independent diff & range-diff options Eric Sunshine
2018-11-26  7:35 ` [ANNOUNCE] Git v2.20.0-rc1 Junio C Hamano
2018-11-26 15:41   ` Elijah Newren
2018-11-27  0:40     ` Junio C Hamano
  -- strict thread matches above, loose matches on Subject: below --
2018-11-19  2:54 Git Test Coverage Report (v2.20.0-rc0) Derrick Stolee
2018-11-19 15:40 ` Derrick Stolee
2018-11-19 16:21   ` Jeff King
2018-11-19 18:44     ` Jeff King
2018-11-19 19:00   ` Ben Peart
2018-11-19 21:06     ` Derrick Stolee
2018-11-20 11:34   ` Jeff King
2018-11-20 12:17     ` Derrick Stolee
2018-11-20 12:40       ` Jeff King
2018-11-19 18:33 ` Ævar Arnfjörð Bjarmason
2018-11-19 18:51   ` [PATCH] tests: add a special setup where rebase.useBuiltin is off (Re: Git Test Coverage Report (v2.20.0-rc0)) Jonathan Nieder
2018-11-19 21:03     ` Ævar Arnfjörð Bjarmason
2018-11-19 19:10   ` Git Test Coverage Report (v2.20.0-rc0) Derrick Stolee
2018-11-19 19:39     ` Ævar Arnfjörð Bjarmason
2018-11-19 19:44       ` Derrick Stolee
2018-11-19 21:31   ` Derrick Stolee
2018-11-20 20:43     ` Johannes Schindelin
2018-11-13 12:38 [PATCH 0/1] rebase: understand -C again, refactor Johannes Schindelin via GitGitGadget
2018-11-13 12:38 ` [PATCH 1/1] rebase: really just passthru the `git am` options Johannes Schindelin via GitGitGadget
2018-11-13 13:05   ` Junio C Hamano
2018-11-13 15:05   ` Phillip Wood
2018-11-13 19:21     ` Johannes Schindelin
2018-11-13 19:58       ` Phillip Wood
2018-11-13 21:50         ` rebase-in-C stability for 2.20 Ævar Arnfjörð Bjarmason
2018-11-14  0:07           ` Stefan Beller
2018-11-14  9:01             ` [PATCH 0/2] rebase.useBuiltin doc & test mode Ævar Arnfjörð Bjarmason
2018-11-14 14:07               ` Johannes Schindelin
2018-11-14  9:01             ` [PATCH 1/2] rebase doc: document rebase.useBuiltin Ævar Arnfjörð Bjarmason
2018-11-14  9:01             ` [PATCH 2/2] tests: add a special setup where rebase.useBuiltin is off Ævar Arnfjörð Bjarmason
2018-11-14  0:36           ` rebase-in-C stability for 2.20 Elijah Newren
2018-11-14  3:39           ` Junio C Hamano
2018-11-24 20:54           ` [ANNOUNCE] Git v2.20.0-rc1 Ævar Arnfjörð Bjarmason
2018-11-25  1:00             ` Junio C Hamano
2018-11-26  6:10               ` [PATCH] rebase: mark the C reimplementation as an experimental opt-in feature (was Re: [ANNOUNCE] Git v2.20.0-rc1) Junio C Hamano
2018-11-28  4:31                 ` Jonathan Nieder
2018-11-28  9:23                   ` Johannes Schindelin
2018-11-28 12:21                     ` Ævar Arnfjörð Bjarmason
2018-11-29  4:58                       ` Junio C Hamano
2018-11-29 14:17                     ` Johannes Schindelin
2018-11-29 14:30                       ` Ian Jackson
2018-11-29 15:39                         ` Johannes Schindelin
2018-11-29 15:50                           ` Ian Jackson
2018-11-29 16:14                             ` Johannes Schindelin
2018-11-29 16:26                               ` Ian Jackson
2018-11-26 22:52             ` [ANNOUNCE] Git v2.20.0-rc1 Johannes Schindelin
2018-11-26 23:47               ` Johannes Schindelin
2018-11-28  4:07                 ` Junio C Hamano
2018-11-28  9:30                   ` Johannes Schindelin
2018-11-14 14:22         ` [PATCH 1/1] rebase: really just passthru the `git am` options Johannes Schindelin
2018-11-14  7:29 ` [PATCH 0/1] rebase: understand -C again, refactor Jeff King
2018-11-14 14:28   ` Johannes Schindelin
2018-11-14 16:25 ` [PATCH v2 0/2] " Johannes Schindelin via GitGitGadget
2018-11-14 16:25   ` [PATCH v2 1/2] rebase: really just passthru the `git am` options Johannes Schindelin via GitGitGadget
2018-11-14 16:25   ` [PATCH v2 2/2] rebase: validate -C<n> and --whitespace=<mode> parameters early Johannes Schindelin via GitGitGadget
2018-11-14 16:37     ` Phillip Wood
2018-11-14 21:24       ` Johannes Schindelin
2018-11-19 12:38     ` Ævar Arnfjörð Bjarmason
2018-11-19 21:37       ` Git Test Coverage Report (v2.20.0-rc0) Ævar Arnfjörð Bjarmason
2018-11-20 10:58       ` [PATCH v2 2/2] rebase: validate -C<n> and --whitespace=<mode> parameters early Johannes Schindelin
2018-11-20 11:42         ` [PATCH] rebase: mark a test as failing with rebase.useBuiltin=false Ævar Arnfjörð Bjarmason
2018-11-20 19:55           ` Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181128201852.9782-3-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.