[PATCH 1/3] format-patch: Make implementation and documentation agree

[PATCH 1/3] format-patch: Make implementation and documentation agree

[Björn Gustavsson]

The documentation for the -p option for format-patch says:
"Generate patches without diffstat."

Starting with commit 68daa64d, however, -p no longer suppresses the
diffstat and is a no-op *most* of the time. It is still needed when
--stat (or one of the other 'stat' options) is given in order to
produce a patch at all.

Since no-one seems to have noticed (or cared) that -p no longer
suppresses the diffstat (after more than a year), it makes most
sense to fix the documentation (rather than the implementation).
The trouble is that -p is still sometimes needed, so to simplify
the documentation, it would be best to change the implementation
as well.

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. They don't
  make sense for format-patch anyway.

* Remove the description of -p from 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).

* Remove the description of the options that are no longer
  allowed (--name-only, --name-status, and --check).
---
 Documentation/diff-options.txt |   19 ++++++++++++-------
 builtin-log.c                  |   12 +++++++++++-
 t/t4014-format-patch.sh        |   18 ++++++++++++++++++
 3 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 9276fae..673fbb0 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -12,11 +12,6 @@ endif::git-log[]
 endif::git-diff[]
 endif::git-format-patch[]
 
-ifdef::git-format-patch[]
--p::
-	Generate patches without diffstat.
-endif::git-format-patch[]
-
 ifndef::git-format-patch[]
 -p::
 -u::
@@ -27,14 +22,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 +71,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 +118,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 25e21ed..7b2cde2 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -1027,9 +1027,19 @@ 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 & 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 (!rev.diffopt.output_format
 		|| rev.diffopt.output_format == DIFF_FORMAT_PATCH)
-		rev.diffopt.output_format = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_SUMMARY | 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 531f5b7..f826348 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -515,4 +515,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