Re: [PATCH bg/format-patch-p-noop] log-tree: always add --- marker when options are patch and a stat

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Nov 04, 2009 at 01:36:13AM -0500, Jeff King wrote:

> Here's a patch which fixes the regression, but it feels awfully hack-ish
> to me, as it treats "-p" specially in diff_opt (but in a way that no
> other existing code should care about). It would be "cleaner" to me to
> have some infrastructure for keeping an implicit and an explicit format,
> and then merging them at the end. But I don't think we ever care about
> this explicitness for any other formats, so this is at least simple.
> 
> Another option might be for format-patch to simply parse "-p" itself,
> setting the format and marking an "explicit" flag. I'll look into that
> as an alternative.

OK, doing that turned out to be much nicer. Less code, localized to
format-patch, and less confusing to read.

This patch goes on top of master, and terribly conflicts with Björn's
changes in the area. But I had the impression you wanted to revert those
changes for now anyway, so probably this should go in as a bug fix and
everything else should be built on top. It actually would be an even
smaller change on top of his "always show patch, even when other formats
are given" change, but I didn't want to depend on it.

-- >8 --
Subject: [PATCH] format-patch: make "-p" suppress diffstat

Once upon a time, format-patch would use its default stat
plus patch format only when no diff format was given on the
command line. This meant that "format-patch -p" would
suppress the stat and show just the patch.

Commit 68daa64 changed this to keep the stat format when we
had an "implicit" patch format, like "-U5". As a side
effect, this meant that an explicit patch format was now
ignored (because cmd_format_patch didn't know the reason
that the format was set way down in diff_opt_parse).

This patch unbreaks what 68daa64 did (while still preserving
what 68daa64 was trying to do), reinstating "-p" to suppress
the default behavior. We do this by parsing "-p" ourselves
in format-patch, and noting whether it was used explicitly.

Signed-off-by: Jeff King <peff@xxxxxxxx>
---
The help text for "-p" is up for debate. It can also of course be used
as "show patch in addition to other things you already specified on the
command line". But I wanted to point out that its main use is the side
effect that it suppresses the default output. I am open to suggestions.

 builtin-log.c           |    9 +++++++--
 t/t4014-format-patch.sh |   21 +++++++++++++++++++++
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/builtin-log.c b/builtin-log.c
index 207a361..da79047 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -891,6 +891,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	struct patch_ids ids;
 	char *add_signoff = NULL;
 	struct strbuf buf = STRBUF_INIT;
+	int use_patch_format = 0;
 	const struct option builtin_format_patch_options[] = {
 		{ OPTION_CALLBACK, 'n', "numbered", &numbered, NULL,
 			    "use [PATCH n/m] even with a single patch",
@@ -940,6 +941,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		{ OPTION_CALLBACK, 0, "thread", &thread, "style",
 			    "enable message threading, styles: shallow, deep",
 			    PARSE_OPT_OPTARG, thread_callback },
+		OPT_BOOLEAN('p', NULL, &use_patch_format,
+			"show patch format instead of default (patch + stat)"),
 		OPT_END()
 	};
 
@@ -1027,8 +1030,10 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	if (argc > 1)
 		die ("unrecognized argument: %s", argv[1]);
 
-	if (!rev.diffopt.output_format
-		|| rev.diffopt.output_format == DIFF_FORMAT_PATCH)
+	if (patch_format)
+		rev.diffopt.output_format |= DIFF_FORMAT_PATCH;
+	else if (!rev.diffopt.output_format ||
+		  rev.diffopt.output_format == DIFF_FORMAT_PATCH)
 		rev.diffopt.output_format = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_SUMMARY | DIFF_FORMAT_PATCH;
 
 	if (!DIFF_OPT_TST(&rev.diffopt, TEXT) && !no_binary_diff)
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 531f5b7..cab6ce2 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -455,6 +455,27 @@ test_expect_success 'format-patch respects -U' '
 
 '
 
+cat > expect << EOF
+
+diff --git a/file b/file
+index 40f36c6..2dc5c23 100644
+--- a/file
++++ b/file
+@@ -14,3 +14,19 @@ C
+ D
+ E
+ F
++5
+EOF
+
+test_expect_success 'format-patch -p suppresses stat' '
+
+	git format-patch -p -2 &&
+	sed -e "1,/^$/d" -e "/^+5/q" < 0001-This-is-an-excessively-long-subject-line-for-a-messa.patch > output &&
+	test_cmp expect output
+
+'
+
 test_expect_success 'format-patch from a subdirectory (1)' '
 	filename=$(
 		rm -rf sub &&
-- 
1.6.5.2.308.g2bb5

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]