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

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

From
Jeff King <peff@peff.net>
Date
Nov 4, 2009, 07:10 UTC
Message-ID
<20091104071053.GB24263@coredump.intra.peff.net>
In-Reply-To
<20091104063612.GA24263@coredump.intra.peff.net>
On Wed, Nov 04, 2009 at 01:36:13AM -0500, Jeff King wrote:
Show 10 quoted lines
> 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@peff.net>
---
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
Previous: Jeff KingNext: Jeff King
Message 4 of 12 in “log-tree: always add --- marker when options are patch and a stat”
  1. log-tree: always add --- marker when options are patch and a statStephen Boyd, Nov 3, 2009
  2. Junio C HamanoNov 4, 2009
  3. Jeff KingNov 4, 2009
  4. Jeff KingNov 4, 2009
  5. Jeff KingNov 4, 2009
  6. Stephen BoydNov 4, 2009
  7. Jeff KingNov 4, 2009
  8. Junio C HamanoNov 4, 2009
  9. Björn GustavssonNov 4, 2009
  10. Junio C HamanoNov 4, 2009
  11. Björn GustavssonNov 4, 2009
  12. Junio C HamanoNov 4, 2009

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.