{"thread":{"id":"43918","subject":"[PATCH v2] format-patch: show 0/1 and 1/1 for singleton patch with cover letter","startedAt":"2016-08-23T21:46:43Z","lastAt":"2016-08-23T22:45:05Z","messageCount":3,"participants":["Jacob Keller","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"299995","messageId":"20160823214620.28624-1-jacob.e.keller@intel.com","threadId":"43918","inReplyTo":null,"subject":"[PATCH v2] format-patch: show 0/1 and 1/1 for singleton patch with cover letter","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2016-08-23T21:46:20Z","receivedAt":"2016-08-23T21:46:43Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller <jacob.keller@gmail.com>\n\nChange the default behavior of git-format-patch to generate numbered\nsequence of 0/1 and 1/1 when generating both a cover-letter and a single\npatch. This standardizes the cover letter to have 0/N which helps\ndistinguish the cover letter from the patch itself. Since the behavior\nis easily changed via configuration as well as the use of -n and -N this\nshould be acceptable default behavior.\n\nAdd tests for the new default behavior.\n\nSigned-off-by: Jacob Keller <jacob.keller@gmail.com>\n---\n builtin/log.c                    |  8 ++++----\n t/t4021-format-patch-numbered.sh | 15 +++++++++++++++\n 2 files changed, 19 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 92dc34dcb0cc..49aa534f4a01 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1676,16 +1676,16 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t/* nothing to do */\n \t\treturn 0;\n \ttotal = nr;\n-\tif (!keep_subject && auto_number && total > 1)\n-\t\tnumbered = 1;\n-\tif (numbered)\n-\t\trev.total = total + start_number - 1;\n \tif (cover_letter == -1) {\n \t\tif (config_cover_letter == COVER_AUTO)\n \t\t\tcover_letter = (total > 1);\n \t\telse\n \t\t\tcover_letter = (config_cover_letter == COVER_ON);\n \t}\n+\tif (!keep_subject && auto_number && (total > 1 || cover_letter))\n+\t\tnumbered = 1;\n+\tif (numbered)\n+\t\trev.total = total + start_number - 1;\n \n \tif (!signature) {\n \t\t; /* --no-signature inhibits all signatures */\ndiff --git a/t/t4021-format-patch-numbered.sh b/t/t4021-format-patch-numbered.sh\nindex 886494b58f67..ea0a388f0191 100755\n--- a/t/t4021-format-patch-numbered.sh\n+++ b/t/t4021-format-patch-numbered.sh\n@@ -36,6 +36,11 @@ test_no_numbered() {\n \ttest_num_no_numbered $1 2\n }\n \n+test_single_cover_letter_numbered() {\n+\tgrep \"^Subject: \\[PATCH 0/1\\]\" $1 &&\n+\tgrep \"^Subject: \\[PATCH 1/1\\]\" $1\n+}\n+\n test_single_numbered() {\n \tgrep \"^Subject: \\[PATCH 1/1\\]\" $1\n }\n@@ -50,6 +55,11 @@ test_expect_success 'single patch defaults to no numbers' '\n \ttest_single_no_numbered patch0.single\n '\n \n+test_expect_success 'single patch with cover-letter defaults to numbers' '\n+\tgit format-patch --cover-letter --stdout HEAD~1 >patch0.single &&\n+\ttest_single_cover_letter_numbered patch0.single\n+'\n+\n test_expect_success 'multiple patch defaults to numbered' '\n \n \tgit format-patch --stdout HEAD~2 >patch0.multiple &&\n@@ -64,6 +74,11 @@ test_expect_success 'Use --numbered' '\n \n '\n \n+test_expect_success 'Use --no-numbered and --cover-letter single patch' '\n+\tgit format-patch --no-numbered --stdout --cover-letter HEAD~1 >patch1 &&\n+\ttest_no_numbered patch1\n+'\n+\n test_expect_success 'format.numbered = true' '\n \n \tgit config format.numbered true &&\n-- \n2.10.0.rc0.259.g83512d9\n\n"},{"id":"300007","messageId":"xmqqk2f7t8ra.fsf@gitster.mtv.corp.google.com","threadId":"43918","inReplyTo":"20160823214620.28624-1-jacob.e.keller@intel.com","subject":"Re: [PATCH v2] format-patch: show 0/1 and 1/1 for singleton patch with cover letter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-23T22:34:17Z","receivedAt":"2016-08-23T22:34:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Keller <jacob.e.keller@intel.com> writes:\n\n> diff --git a/builtin/log.c b/builtin/log.c\n> index 92dc34dcb0cc..49aa534f4a01 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -1676,16 +1676,16 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n>  \t\t/* nothing to do */\n>  \t\treturn 0;\n>  \ttotal = nr;\n> -\tif (!keep_subject && auto_number && total > 1)\n> -\t\tnumbered = 1;\n> -\tif (numbered)\n> -\t\trev.total = total + start_number - 1;\n>  \tif (cover_letter == -1) {\n>  \t\tif (config_cover_letter == COVER_AUTO)\n>  \t\t\tcover_letter = (total > 1);\n>  \t\telse\n>  \t\t\tcover_letter = (config_cover_letter == COVER_ON);\n>  \t}\n> +\tif (!keep_subject && auto_number && (total > 1 || cover_letter))\n> +\t\tnumbered = 1;\n> +\tif (numbered)\n> +\t\trev.total = total + start_number - 1;\n>  \n>  \tif (!signature) {\n>  \t\t; /* --no-signature inhibits all signatures */\n\nThat sounds sensible.\n\n> diff --git a/t/t4021-format-patch-numbered.sh b/t/t4021-format-patch-numbered.sh\n> index 886494b58f67..ea0a388f0191 100755\n> --- a/t/t4021-format-patch-numbered.sh\n> +++ b/t/t4021-format-patch-numbered.sh\n> @@ -36,6 +36,11 @@ test_no_numbered() {\n>  \ttest_num_no_numbered $1 2\n>  }\n>  \n> +test_single_cover_letter_numbered() {\n> +\tgrep \"^Subject: \\[PATCH 0/1\\]\" $1 &&\n> +\tgrep \"^Subject: \\[PATCH 1/1\\]\" $1\n> +}\n> +\n>  test_single_numbered() {\n>  \tgrep \"^Subject: \\[PATCH 1/1\\]\" $1\n>  }\n> @@ -50,6 +55,11 @@ test_expect_success 'single patch defaults to no numbers' '\n>  \ttest_single_no_numbered patch0.single\n>  '\n>  \n> +test_expect_success 'single patch with cover-letter defaults to numbers' '\n> +\tgit format-patch --cover-letter --stdout HEAD~1 >patch0.single &&\n> +\ttest_single_cover_letter_numbered patch0.single\n> +'\n\nThe remaining and existing tests seems to expect that the result of\neach test is stored in a distinct file so that the output can be\ninspected after seeing a test failure without running the script\nwith the \"-i\" option.  Perhaps rename this to patch0.single-cover or\nsomething?  The same for the overwriting of patch1 we can see below.\n\n>  test_expect_success 'multiple patch defaults to numbered' '\n>  \n>  \tgit format-patch --stdout HEAD~2 >patch0.multiple &&\n> @@ -64,6 +74,11 @@ test_expect_success 'Use --numbered' '\n>  \n>  '\n>  \n> +test_expect_success 'Use --no-numbered and --cover-letter single patch' '\n> +\tgit format-patch --no-numbered --stdout --cover-letter HEAD~1 >patch1 &&\n> +\ttest_no_numbered patch1\n> +'\n> +\n>  test_expect_success 'format.numbered = true' '\n>  \n>  \tgit config format.numbered true &&\n"},{"id":"300011","messageId":"CA+P7+xrUnwxa9GqutKGcirrOLKysCn=RLaeMUpgtKF173VxHpA@mail.gmail.com","threadId":"43918","inReplyTo":"xmqqk2f7t8ra.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2] format-patch: show 0/1 and 1/1 for singleton patch with cover letter","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-08-23T22:44:12Z","receivedAt":"2016-08-23T22:45:05Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Tue, Aug 23, 2016 at 3:34 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Jacob Keller <jacob.e.keller@intel.com> writes:\n>\n>> diff --git a/builtin/log.c b/builtin/log.c\n>> index 92dc34dcb0cc..49aa534f4a01 100644\n>> --- a/builtin/log.c\n>> +++ b/builtin/log.c\n>> @@ -1676,16 +1676,16 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n>>               /* nothing to do */\n>>               return 0;\n>>       total = nr;\n>> -     if (!keep_subject && auto_number && total > 1)\n>> -             numbered = 1;\n>> -     if (numbered)\n>> -             rev.total = total + start_number - 1;\n>>       if (cover_letter == -1) {\n>>               if (config_cover_letter == COVER_AUTO)\n>>                       cover_letter = (total > 1);\n>>               else\n>>                       cover_letter = (config_cover_letter == COVER_ON);\n>>       }\n>> +     if (!keep_subject && auto_number && (total > 1 || cover_letter))\n>> +             numbered = 1;\n>> +     if (numbered)\n>> +             rev.total = total + start_number - 1;\n>>\n>>       if (!signature) {\n>>               ; /* --no-signature inhibits all signatures */\n>\n> That sounds sensible.\n>\n>> diff --git a/t/t4021-format-patch-numbered.sh b/t/t4021-format-patch-numbered.sh\n>> index 886494b58f67..ea0a388f0191 100755\n>> --- a/t/t4021-format-patch-numbered.sh\n>> +++ b/t/t4021-format-patch-numbered.sh\n>> @@ -36,6 +36,11 @@ test_no_numbered() {\n>>       test_num_no_numbered $1 2\n>>  }\n>>\n>> +test_single_cover_letter_numbered() {\n>> +     grep \"^Subject: \\[PATCH 0/1\\]\" $1 &&\n>> +     grep \"^Subject: \\[PATCH 1/1\\]\" $1\n>> +}\n>> +\n>>  test_single_numbered() {\n>>       grep \"^Subject: \\[PATCH 1/1\\]\" $1\n>>  }\n>> @@ -50,6 +55,11 @@ test_expect_success 'single patch defaults to no numbers' '\n>>       test_single_no_numbered patch0.single\n>>  '\n>>\n>> +test_expect_success 'single patch with cover-letter defaults to numbers' '\n>> +     git format-patch --cover-letter --stdout HEAD~1 >patch0.single &&\n>> +     test_single_cover_letter_numbered patch0.single\n>> +'\n>\n> The remaining and existing tests seems to expect that the result of\n> each test is stored in a distinct file so that the output can be\n> inspected after seeing a test failure without running the script\n> with the \"-i\" option.  Perhaps rename this to patch0.single-cover or\n> something?  The same for the overwriting of patch1 we can see below.\n>\n\nHmm, ya. I'll rework that.\n\nThanks,\nJake\n"}]}