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