[PATCH v2] format-patch: show 0/1 and 1/1 for singleton patch with cover letter

[PATCH v2] format-patch: show 0/1 and 1/1 for singleton patch with cover letter

[Jacob Keller]

From: Jacob Keller <jacob.keller@gmail.com>

Change the default behavior of git-format-patch to generate numbered
sequence of 0/1 and 1/1 when generating both a cover-letter and a single
patch. This standardizes the cover letter to have 0/N which helps
distinguish the cover letter from the patch itself. Since the behavior
is easily changed via configuration as well as the use of -n and -N this
should be acceptable default behavior.

Add tests for the new default behavior.

Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
 builtin/log.c                    |  8 ++++----
 t/t4021-format-patch-numbered.sh | 15 +++++++++++++++
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 92dc34dcb0cc..49aa534f4a01 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1676,16 +1676,16 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		/* nothing to do */
 		return 0;
 	total = nr;
-	if (!keep_subject && auto_number && total > 1)
-		numbered = 1;
-	if (numbered)
-		rev.total = total + start_number - 1;
 	if (cover_letter == -1) {
 		if (config_cover_letter == COVER_AUTO)
 			cover_letter = (total > 1);
 		else
 			cover_letter = (config_cover_letter == COVER_ON);
 	}
+	if (!keep_subject && auto_number && (total > 1 || cover_letter))
+		numbered = 1;
+	if (numbered)
+		rev.total = total + start_number - 1;
 
 	if (!signature) {
 		; /* --no-signature inhibits all signatures */
diff --git a/t/t4021-format-patch-numbered.sh b/t/t4021-format-patch-numbered.sh
index 886494b58f67..ea0a388f0191 100755
--- a/t/t4021-format-patch-numbered.sh
+++ b/t/t4021-format-patch-numbered.sh
@@ -36,6 +36,11 @@ test_no_numbered() {
 	test_num_no_numbered $1 2
 }
 
+test_single_cover_letter_numbered() {
+	grep "^Subject: \[PATCH 0/1\]" $1 &&
+	grep "^Subject: \[PATCH 1/1\]" $1
+}
+
 test_single_numbered() {
 	grep "^Subject: \[PATCH 1/1\]" $1
 }
@@ -50,6 +55,11 @@ test_expect_success 'single patch defaults to no numbers' '
 	test_single_no_numbered patch0.single
 '
 
+test_expect_success 'single patch with cover-letter defaults to numbers' '
+	git format-patch --cover-letter --stdout HEAD~1 >patch0.single &&
+	test_single_cover_letter_numbered patch0.single
+'
+
 test_expect_success 'multiple patch defaults to numbered' '
 
 	git format-patch --stdout HEAD~2 >patch0.multiple &&
@@ -64,6 +74,11 @@ test_expect_success 'Use --numbered' '
 
 '
 
+test_expect_success 'Use --no-numbered and --cover-letter single patch' '
+	git format-patch --no-numbered --stdout --cover-letter HEAD~1 >patch1 &&
+	test_no_numbered patch1
+'
+
 test_expect_success 'format.numbered = true' '
 
 	git config format.numbered true &&
-- 
2.10.0.rc0.259.g83512d9

Re: [PATCH v2] format-patch: show 0/1 and 1/1 for singleton patch with cover letter

[Junio C Hamano]

Jacob Keller <jacob.e.keller@intel.com> writes:

> diff --git a/builtin/log.c b/builtin/log.c
> index 92dc34dcb0cc..49aa534f4a01 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -1676,16 +1676,16 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
>  		/* nothing to do */
>  		return 0;
>  	total = nr;
> -	if (!keep_subject && auto_number && total > 1)
> -		numbered = 1;
> -	if (numbered)
> -		rev.total = total + start_number - 1;
>  	if (cover_letter == -1) {
>  		if (config_cover_letter == COVER_AUTO)
>  			cover_letter = (total > 1);
>  		else
>  			cover_letter = (config_cover_letter == COVER_ON);
>  	}
> +	if (!keep_subject && auto_number && (total > 1 || cover_letter))
> +		numbered = 1;
> +	if (numbered)
> +		rev.total = total + start_number - 1;
>  
>  	if (!signature) {
>  		; /* --no-signature inhibits all signatures */

That sounds sensible.

> diff --git a/t/t4021-format-patch-numbered.sh b/t/t4021-format-patch-numbered.sh
> index 886494b58f67..ea0a388f0191 100755
> --- a/t/t4021-format-patch-numbered.sh
> +++ b/t/t4021-format-patch-numbered.sh
> @@ -36,6 +36,11 @@ test_no_numbered() {
>  	test_num_no_numbered $1 2
>  }
>  
> +test_single_cover_letter_numbered() {
> +	grep "^Subject: \[PATCH 0/1\]" $1 &&
> +	grep "^Subject: \[PATCH 1/1\]" $1
> +}
> +
>  test_single_numbered() {
>  	grep "^Subject: \[PATCH 1/1\]" $1
>  }
> @@ -50,6 +55,11 @@ test_expect_success 'single patch defaults to no numbers' '
>  	test_single_no_numbered patch0.single
>  '
>  
> +test_expect_success 'single patch with cover-letter defaults to numbers' '
> +	git format-patch --cover-letter --stdout HEAD~1 >patch0.single &&
> +	test_single_cover_letter_numbered patch0.single
> +'

The remaining and existing tests seems to expect that the result of
each test is stored in a distinct file so that the output can be
inspected after seeing a test failure without running the script
with the "-i" option.  Perhaps rename this to patch0.single-cover or
something?  The same for the overwriting of patch1 we can see below.

>  test_expect_success 'multiple patch defaults to numbered' '
>  
>  	git format-patch --stdout HEAD~2 >patch0.multiple &&
> @@ -64,6 +74,11 @@ test_expect_success 'Use --numbered' '
>  
>  '
>  
> +test_expect_success 'Use --no-numbered and --cover-letter single patch' '
> +	git format-patch --no-numbered --stdout --cover-letter HEAD~1 >patch1 &&
> +	test_no_numbered patch1
> +'
> +
>  test_expect_success 'format.numbered = true' '
>  
>  	git config format.numbered true &&

Re: [PATCH v2] format-patch: show 0/1 and 1/1 for singleton patch with cover letter

[Jacob Keller]

On Tue, Aug 23, 2016 at 3:34 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jacob Keller <jacob.e.keller@intel.com> writes:
>
>> diff --git a/builtin/log.c b/builtin/log.c
>> index 92dc34dcb0cc..49aa534f4a01 100644
>> --- a/builtin/log.c
>> +++ b/builtin/log.c
>> @@ -1676,16 +1676,16 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
>>               /* nothing to do */
>>               return 0;
>>       total = nr;
>> -     if (!keep_subject && auto_number && total > 1)
>> -             numbered = 1;
>> -     if (numbered)
>> -             rev.total = total + start_number - 1;
>>       if (cover_letter == -1) {
>>               if (config_cover_letter == COVER_AUTO)
>>                       cover_letter = (total > 1);
>>               else
>>                       cover_letter = (config_cover_letter == COVER_ON);
>>       }
>> +     if (!keep_subject && auto_number && (total > 1 || cover_letter))
>> +             numbered = 1;
>> +     if (numbered)
>> +             rev.total = total + start_number - 1;
>>
>>       if (!signature) {
>>               ; /* --no-signature inhibits all signatures */
>
> That sounds sensible.
>
>> diff --git a/t/t4021-format-patch-numbered.sh b/t/t4021-format-patch-numbered.sh
>> index 886494b58f67..ea0a388f0191 100755
>> --- a/t/t4021-format-patch-numbered.sh
>> +++ b/t/t4021-format-patch-numbered.sh
>> @@ -36,6 +36,11 @@ test_no_numbered() {
>>       test_num_no_numbered $1 2
>>  }
>>
>> +test_single_cover_letter_numbered() {
>> +     grep "^Subject: \[PATCH 0/1\]" $1 &&
>> +     grep "^Subject: \[PATCH 1/1\]" $1
>> +}
>> +
>>  test_single_numbered() {
>>       grep "^Subject: \[PATCH 1/1\]" $1
>>  }
>> @@ -50,6 +55,11 @@ test_expect_success 'single patch defaults to no numbers' '
>>       test_single_no_numbered patch0.single
>>  '
>>
>> +test_expect_success 'single patch with cover-letter defaults to numbers' '
>> +     git format-patch --cover-letter --stdout HEAD~1 >patch0.single &&
>> +     test_single_cover_letter_numbered patch0.single
>> +'
>
> The remaining and existing tests seems to expect that the result of
> each test is stored in a distinct file so that the output can be
> inspected after seeing a test failure without running the script
> with the "-i" option.  Perhaps rename this to patch0.single-cover or
> something?  The same for the overwriting of patch1 we can see below.
>

Hmm, ya. I'll rework that.

Thanks,
Jake