[PATCH 1/4] format-patch: Always generate a patch

["Björn Gustavsson" <bgustavsson@gmail.com>]

Jeff King recently reinstated -p to suppress the default diffstat
(as -p used to work before 68daa64, about 14 months ago).

However, -p is also needed in combination with certain options
(e.g. --stat or --numstat) in order to produce any patch at all.
The documentation does not mention this.

Since the purpose of format-patch is to produce a patch that
can be emailed, it does not make sense that certain combination
of options will suppress the generation of the patch itself.

Therefore:

* Update 'git format-patch' to always generate a patch.

* Since the --name-only, --name-status, and --check suppresses
  the generation of the patch, disallow those options,
  and remove the description of them in the documentation.

* Remove the reference to -p in the description of -U.

* Remove the descriptions of the options that are synonyms for -p
  plus another option (--patch-with-raw and --patch-with-stat).

* While at it, slightly tweak the description of -p itself
  to say that it generates "plain patches", so that you can
  think of -p as "plain patch" as an mnemonic aid.

Signed-off-by: Björn Gustavsson <bgustavsson@gmail.com>
---
Compared to my previous patch, most of the changes are in
the commit message. It was straightforward to combine my
previous changes in the code with Peff's version. In the
documentation, -p is now included.

 Documentation/diff-options.txt |   16 +++++++++++++---
 builtin-log.c                  |   21 ++++++++++++++++-----
 t/t4014-format-patch.sh        |   18 ++++++++++++++++++
 3 files changed, 47 insertions(+), 8 deletions(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 9276fae..c58d085 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -14,7 +14,7 @@ endif::git-format-patch[]
 
 ifdef::git-format-patch[]
 -p::
-	Generate patches without diffstat.
+	Generate plain patches without any diffstats.
 endif::git-format-patch[]
 
 ifndef::git-format-patch[]
@@ -27,14 +27,19 @@ endif::git-format-patch[]
 -U<n>::
 --unified=<n>::
 	Generate diffs with <n> lines of context instead of
-	the usual three. Implies "-p".
+	the usual three.
+ifndef::git-format-patch[]
+	Implies "-p".
+endif::git-format-patch[]
 
 --raw::
 	Generate the raw format.
 	{git-diff-core? This is the default.}
 
+ifndef::git-format-patch[]
 --patch-with-raw::
 	Synonym for "-p --raw".
+endif::git-format-patch[]
 
 --patience::
 	Generate a diff using the "patience diff" algorithm.
@@ -71,21 +76,24 @@ endif::git-format-patch[]
 	Output a condensed summary of extended header information
 	such as creations, renames and mode changes.
 
+ifndef::git-format-patch[]
 --patch-with-stat::
 	Synonym for "-p --stat".
-	{git-format-patch? This is the default.}
+endif::git-format-patch[]
 
 -z::
 	NUL-line termination on output.  This affects the --raw
 	output field terminator.  Also output from commands such
 	as "git-log" will be delimited with NUL between commits.
 
+ifndef::git-format-patch[]
 --name-only::
 	Show only names of changed files.
 
 --name-status::
 	Show only names and status of changed files. See the description
 	of the `--diff-filter` option on what the status letters mean.
+endif::git-format-patch[]
 
 --color::
 	Show colored diff.
@@ -115,11 +123,13 @@ override configuration settings.
 	Turn off rename detection, even when the configuration
 	file gives the default to do so.
 
+ifndef::git-format-patch[]
 --check::
 	Warn if changes introduce trailing whitespace
 	or an indent that uses a space before a tab. Exits with
 	non-zero status if problems are found. Not compatible with
 	--exit-code.
+endif::git-format-patch[]
 
 --full-index::
 	Instead of the first handful of characters, show the full
diff --git a/builtin-log.c b/builtin-log.c
index 7b91c91..9df8dac 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -925,6 +925,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			"show patch format instead of default (patch + stat)"),
 		OPT_BOOLEAN(0, "ignore-if-in-upstream", &ignore_if_in_upstream,
 			    "don't include a patch matching a commit upstream"),
+		OPT_BOOLEAN('p', NULL, &use_patch_format,
+			"show patch format instead of default (patch + stat)"),
 		OPT_GROUP("Messaging"),
 		{ OPTION_CALLBACK, 0, "add-header", NULL, "header",
 			    "add email header", PARSE_OPT_NONEG,
@@ -1030,11 +1032,20 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	if (argc > 1)
 		die ("unrecognized argument: %s", argv[1]);
 
-	if (use_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 (rev.diffopt.output_format & DIFF_FORMAT_NAME)
+		die("--name-only does not make sense");
+	if (rev.diffopt.output_format & DIFF_FORMAT_NAME_STATUS)
+		die("--name-status does not make sense");
+	if (rev.diffopt.output_format & DIFF_FORMAT_CHECKDIFF)
+		die("--check does not make sense");
+
+	if (!use_patch_format &&
+		(!rev.diffopt.output_format ||
+		 rev.diffopt.output_format == DIFF_FORMAT_PATCH))
+		rev.diffopt.output_format = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_SUMMARY;
+
+	/* Always generate a patch */
+	rev.diffopt.output_format |= DIFF_FORMAT_PATCH;
 
 	if (!DIFF_OPT_TST(&rev.diffopt, TEXT) && !no_binary_diff)
 		DIFF_OPT_SET(&rev.diffopt, BINARY);
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index cab6ce2..5689d59 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -536,4 +536,22 @@ test_expect_success 'format-patch --signoff' '
 	grep "^Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
 '
 
+echo "fatal: --name-only does not make sense" > expect.name-only
+echo "fatal: --name-status does not make sense" > expect.name-status
+echo "fatal: --check does not make sense" > expect.check
+
+test_expect_success 'options no longer allowed for format-patch' '
+	test_must_fail git format-patch --name-only 2> output &&
+	test_cmp expect.name-only output &&
+	test_must_fail git format-patch --name-status 2> output &&
+	test_cmp expect.name-status output &&
+	test_must_fail git format-patch --check 2> output &&
+	test_cmp expect.check output'
+
+test_expect_success 'format-patch --numstat should produce a patch' '
+	git format-patch --numstat --stdout master..side |
+	grep "^diff --git a/" |
+	wc -l |
+	xargs test 6 = '
+
 test_done
-- 
1.6.5.1.69.g36942