# [PATCH] format-patch: show 0/1 and 1/1 for singleton patch with cover letter 5 messages from 2016-08-19 to 2016-08-23. Participants: Jacob Keller, Junio C Hamano. Thread: https://gitlist.dev/t/43890 ## Jacob Keller, 2016-08-19 23:49 Subject: [PATCH] format-patch: show 0/1 and 1/1 for singleton patch with cover letter Message-ID: <20160819234959.26308-1-jacob.e.keller@intel.com> URL: https://gitlist.dev/e/20160819234959.26308-1-jacob.e.keller%40intel.com ``` From: Jacob Keller 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. Signed-off-by: Jacob Keller --- builtin/log.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/log.c b/builtin/log.c index 92dc34dcb0cc..8e6100fb0c5b 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1676,7 +1676,7 @@ 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) + if (!keep_subject && auto_number && (total > 1 || cover_letter)) numbered = 1; if (numbered) rev.total = total + start_number - 1; -- 2.10.0.rc0.259.g83512d9 ``` ## Junio C Hamano, 2016-08-23 16:33 Subject: Re: [PATCH] format-patch: show 0/1 and 1/1 for singleton patch with cover letter Message-ID: URL: https://gitlist.dev/e/xmqq37lvxx5o.fsf%40gitster.mtv.corp.google.com In-Reply-To: <20160819234959.26308-1-jacob.e.keller@intel.com> ``` Jacob Keller writes: > From: Jacob Keller > > 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. > > Signed-off-by: Jacob Keller > --- This obviously changes the behaviour, but I do not think of a reason why this change is a bad idea. > builtin/log.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/builtin/log.c b/builtin/log.c > index 92dc34dcb0cc..8e6100fb0c5b 100644 > --- a/builtin/log.c > +++ b/builtin/log.c > @@ -1676,7 +1676,7 @@ 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) > + if (!keep_subject && auto_number && (total > 1 || cover_letter)) > numbered = 1; > if (numbered) > rev.total = total + start_number - 1; ``` ## Jacob Keller, 2016-08-23 21:06 Subject: Re: [PATCH] format-patch: show 0/1 and 1/1 for singleton patch with cover letter Message-ID: URL: https://gitlist.dev/e/CA%2BP7%2Bxp1ipo1dDT%2Bf4GF76%2BEY%3DvGx2kEqOsy0jdgCmzA%3DUd1eQ%40mail.gmail.com In-Reply-To: ``` On Tue, Aug 23, 2016 at 9:33 AM, Junio C Hamano wrote: > Jacob Keller writes: > >> From: Jacob Keller >> >> 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. >> >> Signed-off-by: Jacob Keller >> --- > > This obviously changes the behaviour, but I do not think of a reason > why this change is a bad idea. > Yes. The basic idea is "number all files outputted if we generate more than 1 file" Since a cover letter is a separate file, we number 0/1 and 1/1. Thanks, Jake ``` ## Jacob Keller, 2016-08-23 21:28 Subject: Re: [PATCH] format-patch: show 0/1 and 1/1 for singleton patch with cover letter Message-ID: URL: https://gitlist.dev/e/CA%2BP7%2BxqSurMJG7%2Bz7yZGxpfuYWAuoZggQ_09KLiPBAs5E6yxZA%40mail.gmail.com In-Reply-To: ``` On Tue, Aug 23, 2016 at 2:21 PM, Junio C Hamano wrote: > Junio C Hamano writes: >>> total = nr; >>> - if (!keep_subject && auto_number && total > 1) >>> + if (!keep_subject && auto_number && (total > 1 || cover_letter)) >>> numbered = 1; >>> if (numbered) >>> rev.total = total + start_number - 1; > > Actually there is a very good reason why this patch is not good > (yet). When the --cover option is not specified on the command > line, cover_letter is -1 (use configuration or turn it on only when > it is a multi-patch series) at this point. > > I think you would have noticed it if you ran any format-patch tests. > t/t4021-format-patch-numbered.sh fails at the very beginning. > Oops! Sorry for not running tests. > With the attached SQUASH, existing tests pass, which is a strong > sign that this new feature needs to be protected by a new test in > the t4021 script to make sure other people would not break it in the > future. > I'll add a new test and squash your fix in. Thanks, Jake > builtin/log.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/builtin/log.c b/builtin/log.c > index e50d361..b7bfeb9 100644 > --- a/builtin/log.c > +++ b/builtin/log.c > @@ -1650,16 +1650,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 || cover_letter)) > - 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 */ ``` ## Junio C Hamano, 2016-08-23 21:21 Subject: Re: [PATCH] format-patch: show 0/1 and 1/1 for singleton patch with cover letter Message-ID: URL: https://gitlist.dev/e/xmqqinuruqpk.fsf%40gitster.mtv.corp.google.com In-Reply-To: ``` Junio C Hamano writes: > This obviously changes the behaviour, but I do not think of a reason > why this change is a bad idea. >> diff --git a/builtin/log.c b/builtin/log.c >> index 92dc34dcb0cc..8e6100fb0c5b 100644 >> --- a/builtin/log.c >> +++ b/builtin/log.c >> @@ -1676,7 +1676,7 @@ 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) >> + if (!keep_subject && auto_number && (total > 1 || cover_letter)) >> numbered = 1; >> if (numbered) >> rev.total = total + start_number - 1; Actually there is a very good reason why this patch is not good (yet). When the --cover option is not specified on the command line, cover_letter is -1 (use configuration or turn it on only when it is a multi-patch series) at this point. I think you would have noticed it if you ran any format-patch tests. t/t4021-format-patch-numbered.sh fails at the very beginning. With the attached SQUASH, existing tests pass, which is a strong sign that this new feature needs to be protected by a new test in the t4021 script to make sure other people would not break it in the future. builtin/log.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/builtin/log.c b/builtin/log.c index e50d361..b7bfeb9 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1650,16 +1650,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 || cover_letter)) - 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 */ ```