format-patch let -k override a config-specified format.numbered

PATCH In review Jim Meyering 4 participants 4 messages Jun 15, 2016

Share & tools

Jim Meyering

author

2016-06-15 22:46 UTC #0 lore raw

PATCH 1/1 format-patch let -k override a config-specified format.numbered

Let a command-line --keep-subject (-k) override a config-specified format.numbered (--numbered (-n)), rather than provoking the "-n and -k are mutually exclusive" failure.
* t4021-format-patch-numbered.sh: Test for the above
---

Here's a quick demo of the problem:

  $ git config format.numbered true
  $ git format-patch --stdout -1 -k > /dev/null
  fatal: -n and -k are mutually exclusive.

This started when a colleague reported that "git rebase master" was failing with a "fatal: -n and -k are mutually exclusive". Stephen Tweedie discovered that it was due to the format.numbered=true setting in ~/.gitconfig conflicting with the -k of the format-patch command run as part of the rebase operation. Here's a fix.

Stephen suggested an alternate patch in which rather than adding a new variable, you'd use "numbered = -1" to indicate that the setting originated in config. Let me know if you'd prefer that.

 builtin-log.c                    |   14 +++++++++++++-
 t/t4021-format-patch-numbered.sh |    7 +++++++
 2 files changed, 20 insertions(+), 1 deletions(-)

builtin-log.c +13 -1

index 5eaec5d..f10cfeb 100644
@@ -755,6 +755,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	int cover_letter = 0;
 	int boundary_count = 0;
 	int no_binary_diff = 0;
+	int numbered_cmdline_opt = 0;
 	struct commit *origin = NULL, *head = NULL;
 	const char *in_reply_to = NULL;
 	struct patch_ids ids;

@@ -786,8 +787,10 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		if (!strcmp(argv[i], "--stdout"))
 			use_stdout = 1;
 		else if (!strcmp(argv[i], "-n") ||
-				!strcmp(argv[i], "--numbered"))
+				!strcmp(argv[i], "--numbered")) {
 			numbered = 1;
+			numbered_cmdline_opt = 1;
+		}
 		else if (!strcmp(argv[i], "-N") ||
 				!strcmp(argv[i], "--no-numbered")) {
 			numbered = 0;

@@ -918,6 +921,15 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 
 	if (start_number < 0)
 		start_number = 1;
+
+	/*
+	 * If numbered is set solely due to format.numbered in config,
+	 * and it would conflict with --keep-subject (-k) from the
+	 * command line, reset "numbered".
+	 */
+	if (numbered && keep_subject && !numbered_cmdline_opt)
+		numbered = 0;
+
 	if (numbered && keep_subject)
 		die ("-n and -k are mutually exclusive.");
 	if (keep_subject && subject_prefix)

t/t4021-format-patch-numbered.sh +7 -0

index 390af23..9b6e1be 100755
@@ -86,6 +86,13 @@ test_expect_success 'format.numbered && --no-numbered' '
 
 '
 
+test_expect_success 'format.numbered && --keep-subject' '
+
+	git format-patch --keep-subject --stdout HEAD^ >patch4a &&
+	grep "^Subject: Third" patch4a
+
+'
+
 test_expect_success 'format.numbered = auto' '
 
 	git config format.numbered auto

--
1.6.3.195.gad81

Stephen Boyd

2016-06-15 22:46 UTC #1 lore raw

Quoted 5 lines · Jim Meyering

This started when a colleague reported that "git rebase master" was failing with a "fatal: -n and -k are mutually exclusive". Stephen Tweedie discovered that it was due to the format.numbered=true setting in ~/.gitconfig conflicting with the -k of the format-patch command run as part of the rebase operation. Here's a fix.

I think the real fix here should be in rebase. We should be passing --no-numbered to format-patch. I was expecting 'format.subjectprefix' to trigger the same bug, but it seems we don't set subject_prefix=1 in this case. I imagine fixing that can be done in a later patch.

Junio C Hamano

2016-06-15 22:46 UTC #2 lore raw

In reply to Jim Meyering

Quoted 5 lines · Jim Meyering

Let a command-line --keep-subject (-k) override a config-specified format.numbered (--numbered (-n)), rather than provoking the "-n and -k are mutually exclusive" failure.
* t4021-format-patch-numbered.sh: Test for the above
---

Sign-off?

Quoted 12 lines

Here's a quick demo of the problem:
  $ git config format.numbered true
  $ git format-patch --stdout -1 -k > /dev/null
  fatal: -n and -k are mutually exclusive.
This started when a colleague reported that "git rebase master" was failing with a "fatal: -n and -k are mutually exclusive". Stephen Tweedie discovered that it was due to the format.numbered=true setting in ~/.gitconfig conflicting with the -k of the format-patch command run as part of the rebase operation. Here's a fix.

Quoted 3 lines

Stephen suggested an alternate patch in which rather than adding a new variable, you'd use "numbered = -1" to indicate that the setting originated in config. Let me know if you'd prefer that.

Some people might find an extra variable redundant or verbose, but I think your approach is easier to follow and to the point, especially with the in-code comment.

Thanks.

Jim Meyering

2016-06-15 22:46 UTC #3 lore raw

Signed-off-by Jim Meyering <[email protected]>

---

Quoted 8 lines · Junio C Hamano

Quoted 5 lines · Jim Meyering
Let a command-line --keep-subject (-k) override a config-specified format.numbered (--numbered (-n)), rather than provoking the "-n and -k are mutually exclusive" failure.
* t4021-format-patch-numbered.sh: Test for the above
---
Sign-off?

Whoops. Here you go.

 builtin-log.c                    |   14 +++++++++++++-
 t/t4021-format-patch-numbered.sh |    7 +++++++
 2 files changed, 20 insertions(+), 1 deletions(-)

builtin-log.c +13 -1

index 5eaec5d..f10cfeb 100644
@@ -755,6 +755,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	int cover_letter = 0;
 	int boundary_count = 0;
 	int no_binary_diff = 0;
+	int numbered_cmdline_opt = 0;
 	struct commit *origin = NULL, *head = NULL;
 	const char *in_reply_to = NULL;
 	struct patch_ids ids;

@@ -786,8 +787,10 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		if (!strcmp(argv[i], "--stdout"))
 			use_stdout = 1;
 		else if (!strcmp(argv[i], "-n") ||
-				!strcmp(argv[i], "--numbered"))
+				!strcmp(argv[i], "--numbered")) {
 			numbered = 1;
+			numbered_cmdline_opt = 1;
+		}
 		else if (!strcmp(argv[i], "-N") ||
 				!strcmp(argv[i], "--no-numbered")) {
 			numbered = 0;

@@ -918,6 +921,15 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 
 	if (start_number < 0)
 		start_number = 1;
+
+	/*
+	 * If numbered is set solely due to format.numbered in config,
+	 * and it would conflict with --keep-subject (-k) from the
+	 * command line, reset "numbered".
+	 */
+	if (numbered && keep_subject && !numbered_cmdline_opt)
+		numbered = 0;
+
 	if (numbered && keep_subject)
 		die ("-n and -k are mutually exclusive.");
 	if (keep_subject && subject_prefix)

t/t4021-format-patch-numbered.sh +7 -0

index 390af23..9b6e1be 100755
@@ -86,6 +86,13 @@ test_expect_success 'format.numbered && --no-numbered' '
 
 '
 
+test_expect_success 'format.numbered && --keep-subject' '
+
+	git format-patch --keep-subject --stdout HEAD^ >patch4a &&
+	grep "^Subject: Third" patch4a
+
+'
+
 test_expect_success 'format.numbered = auto' '
 
 	git config format.numbered auto

--
1.6.3.195.gad81