{"thread":{"id":"43890","subject":"[PATCH] format-patch: show 0/1 and 1/1 for singleton patch with cover letter","startedAt":"2016-08-19T23:50:06Z","lastAt":"2016-08-23T21:35:56Z","messageCount":5,"participants":["Jacob Keller","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"299782","messageId":"20160819234959.26308-1-jacob.e.keller@intel.com","threadId":"43890","inReplyTo":null,"subject":"[PATCH] 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-19T23:49:59Z","receivedAt":"2016-08-19T23:50:06Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller \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\nSigned-off-by: Jacob Keller \n---\n builtin/log.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 92dc34dcb0cc..8e6100fb0c5b 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1676,7 +1676,7 @@ 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+\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-- \n2.10.0.rc0.259.g83512d9\n\n"},{"id":"299962","messageId":"xmqq37lvxx5o.fsf@gitster.mtv.corp.google.com","threadId":"43890","inReplyTo":"20160819234959.26308-1-jacob.e.keller@intel.com","subject":"Re: [PATCH] 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-23T16:33:39Z","receivedAt":"2016-08-23T16:50:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Keller writes:\n\n> From: Jacob Keller \n>\n> Change the default behavior of git-format-patch to generate numbered\n> sequence of 0/1 and 1/1 when generating both a cover-letter and a single\n> patch. This standardizes the cover letter to have 0/N which helps\n> distinguish the cover letter from the patch itself. Since the behavior\n> is easily changed via configuration as well as the use of -n and -N this\n> should be acceptable default behavior.\n>\n> Signed-off-by: Jacob Keller \n> ---\n\nThis obviously changes the behaviour, but I do not think of a reason\nwhy this change is a bad idea. \n\n> builtin/log.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/builtin/log.c b/builtin/log.c\n> index 92dc34dcb0cc..8e6100fb0c5b 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -1676,7 +1676,7 @@ 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> +\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"},{"id":"299985","messageId":"CA+P7+xp1ipo1dDT+f4GF76+EY=vGx2kEqOsy0jdgCmzA=Ud1eQ@mail.gmail.com","threadId":"43890","inReplyTo":"xmqq37lvxx5o.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] 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-23T21:06:17Z","receivedAt":"2016-08-23T21:12:35Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Tue, Aug 23, 2016 at 9:33 AM, Junio C Hamano wrote:\n> Jacob Keller writes:\n>\n>> From: Jacob Keller \n>>\n>> Change the default behavior of git-format-patch to generate numbered\n>> sequence of 0/1 and 1/1 when generating both a cover-letter and a single\n>> patch. This standardizes the cover letter to have 0/N which helps\n>> distinguish the cover letter from the patch itself. Since the behavior\n>> is easily changed via configuration as well as the use of -n and -N this\n>> should be acceptable default behavior.\n>>\n>> Signed-off-by: Jacob Keller \n>> ---\n>\n> This obviously changes the behaviour, but I do not think of a reason\n> why this change is a bad idea.\n>\n\nYes. The basic idea is \"number all files outputted if we generate more\nthan 1 file\" Since a cover letter is a separate file, we number 0/1\nand 1/1.\n\nThanks,\nJake\n"},{"id":"299987","messageId":"CA+P7+xqSurMJG7+z7yZGxpfuYWAuoZggQ_09KLiPBAs5E6yxZA@mail.gmail.com","threadId":"43890","inReplyTo":"xmqqinuruqpk.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] 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-23T21:28:02Z","receivedAt":"2016-08-23T21:29:14Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Tue, Aug 23, 2016 at 2:21 PM, Junio C Hamano wrote:\n> Junio C Hamano writes:\n>>> total = nr;\n>>> - if (!keep_subject && auto_number && total > 1)\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> Actually there is a very good reason why this patch is not good\n> (yet). When the --cover option is not specified on the command\n> line, cover_letter is -1 (use configuration or turn it on only when\n> it is a multi-patch series) at this point.\n>\n> I think you would have noticed it if you ran any format-patch tests.\n> t/t4021-format-patch-numbered.sh fails at the very beginning.\n>\n\nOops! Sorry for not running tests.\n\n> With the attached SQUASH, existing tests pass, which is a strong\n> sign that this new feature needs to be protected by a new test in\n> the t4021 script to make sure other people would not break it in the\n> future.\n>\n\nI'll add a new test and squash your fix in.\n\nThanks,\nJake\n\n> builtin/log.c | 8 ++++----\n> 1 file changed, 4 insertions(+), 4 deletions(-)\n>\n> diff --git a/builtin/log.c b/builtin/log.c\n> index e50d361..b7bfeb9 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -1650,16 +1650,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 || cover_letter))\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"},{"id":"299988","messageId":"xmqqinuruqpk.fsf@gitster.mtv.corp.google.com","threadId":"43890","inReplyTo":"xmqq37lvxx5o.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] 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-23T21:21:11Z","receivedAt":"2016-08-23T21:35:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> This obviously changes the behaviour, but I do not think of a reason\n> why this change is a bad idea. \n\n>> diff --git a/builtin/log.c b/builtin/log.c\n>> index 92dc34dcb0cc..8e6100fb0c5b 100644\n>> --- a/builtin/log.c\n>> +++ b/builtin/log.c\n>> @@ -1676,7 +1676,7 @@ 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>> +\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\nActually there is a very good reason why this patch is not good\n(yet). When the --cover option is not specified on the command\nline, cover_letter is -1 (use configuration or turn it on only when\nit is a multi-patch series) at this point.\n\nI think you would have noticed it if you ran any format-patch tests.\nt/t4021-format-patch-numbered.sh fails at the very beginning.\n\nWith the attached SQUASH, existing tests pass, which is a strong\nsign that this new feature needs to be protected by a new test in\nthe t4021 script to make sure other people would not break it in the\nfuture.\n\n builtin/log.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex e50d361..b7bfeb9 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1650,16 +1650,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 || cover_letter))\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"}]}