| Message ID | pull.885.v2.git.1614588030233.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] format-patch: allow a non-integral version numbers | expand |
"ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com> writes: > diff --git a/builtin/log.c b/builtin/log.c > index f67b67d80ed1..95c95eab5393 100644 > --- a/builtin/log.c > +++ b/builtin/log.c > @@ -1662,13 +1662,15 @@ static void print_bases(struct base_tree_info *bases, FILE *file) > oidclr(&bases->base_commit); > } > > -static const char *diff_title(struct strbuf *sb, int reroll_count, > - const char *generic, const char *rerolled) > +static const char *diff_title(struct strbuf *sb, const char *reroll_count, int reroll_count_is_integer, > + const char*previous_count, const char *generic, const char *rerolled) Avoid overly long lines here, but quite honestly, I find that this interface is way too ugly to live. Can we do all the computation around previous count in the caller, so that this function only takes reroll_count and previous_count that are both "const char *", and then the body will just be: if (!reroll_count) strbuf_addstr(sb, generic); else if (previous_count) strbuf_addf(sb, rerolled, previous_count); return sb->buf; That way, the callers do not have to prepare two different rerolled template and switch between them based on "is_integer". In other words, they need to care "is_integer" already, so making them responsible for preparing "previous_count" always usable by this function would be a reasonable way to partition the tasks between this callee and the caller. That way, this function do not even need to know about "is_integer" bit. > + if (previous_count && !reroll_count) > + usage(_("previous-count can only used when reroll-count is used")); > + if (reroll_count) { > struct strbuf sprefix = STRBUF_INIT; > - strbuf_addf(&sprefix, "%s v%d", > + char ch; > + size_t i = 0 , reroll_count_len = strlen(reroll_count); > + > + for (; i != reroll_count_len; i++) { > + ch = reroll_count[i]; > + if(!isdigit(ch)) > + break; > + } > + reroll_count_is_integer = i == reroll_count_len ? 1 : 0; Do not reinvent integer parsing. In our codebase, it is far more common (and it is less error prone) to do something like this: char *endp; count = strtoul(reroll_count_string, &endp, 10); if (*endp) { /* followed by non-digit: not an integer */ is_integer = 0; } else { is_integer = 1; if (0 < count) previous_count_string = xstrfmt("%d", count - 1); } And then, you can move the "if previous is there and count is not specified" check after this block, to make sure that a non-integer reroll count is always accompanied by a previous count, for example. > + strbuf_addf(&sprefix, "%s v%s", > rev.subject_prefix, reroll_count); > rev.reroll_count = reroll_count; > rev.subject_prefix = strbuf_detach(&sprefix, NULL); > @@ -2079,8 +2095,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) > rev.idiff_oid1 = &idiff_prev.oid[idiff_prev.nr - 1]; > rev.idiff_oid2 = get_commit_tree_oid(list[0]); > rev.idiff_title = diff_title(&idiff_title, reroll_count, > - _("Interdiff:"), > - _("Interdiff against v%d:")); > + reroll_count_is_integer, previous_count, _("Interdiff:"), > + reroll_count_is_integer ? _("Interdiff against v%d:") : > + _("Interdiff against v%s:")); I've touched the calling convention into diff_title() function earlier. > @@ -2098,8 +2115,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) > rev.rdiff2 = rdiff2.buf; > rev.creation_factor = creation_factor; > rev.rdiff_title = diff_title(&rdiff_title, reroll_count, > - _("Range-diff:"), > - _("Range-diff against v%d:")); > + reroll_count_is_integer, previous_count, _("Range-diff:"), > + reroll_count_is_integer ? _("Range-diff against v%d:") : > + _("Range-diff against v%s:")); Ditto.
Hi ZheNing, Thanks for picking up the issue. It's good to see that the GGG issue tracker is helpful. On Mon, Mar 01, 2021 at 08:40:29AM +0000, ZheNing Hu via GitGitGadget wrote: > From: ZheNing Hu <adlternative@gmail.com> > > Usually we can only use `format-patch -v<n>` to generate integral > version numbers patches, but sometimes a same fixup should be > laveled as a non-integral versions like `v1.1`,so teach format-patch Some typos: s/laveled/labeled/; s/1.1`,/& /; s/format-patch/& to/ > allow a non-integral versions may be helpful to send those patches. > > Since the original `format-patch` logic, if we specify a version `-v<n>` > and commbine with `--interdiff` or `--rangediff`, the patch will output > "Interdiff again v<n-1>:" or "Range-diff again v<n-1>:`, but this does > not meet the requirements of our fractional version numbers, so provide > `format patch` a new option `--previous-count=<n>`, the patch can output > user-specified previous version number. If the user use a integral version > number `-v<n>`, ensure that the output in the patch is still `v<n-1>`. > (let `--previous-count` become invalid.) Hmm, others may disagree but I don't really like the idea of `--previous-count`. It may be useful for populating "Range-diff vs <n>" instead of just "Range-diff" but I don't think it's worth the cost of maintaining this option. > Signed-off-by: ZheNing Hu <adlternative@gmail.com> > --- [...] > diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt > index 3e49bf221087..f10fa6527f5f 100644 > --- a/Documentation/git-format-patch.txt > +++ b/Documentation/git-format-patch.txt > @@ -22,6 +22,7 @@ SYNOPSIS > [--cover-from-description=<mode>] > [--rfc] [--subject-prefix=<subject prefix>] > [(--reroll-count|-v) <n>] > + [--previous-count=<n>] > [--to=<email>] [--cc=<email>] > [--[no-]cover-letter] [--quiet] > [--[no-]encode-email-headers] > @@ -221,6 +222,15 @@ populated with placeholder text. > `--subject-prefix` option) has ` v<n>` appended to it. E.g. > `--reroll-count=4` may produce `v4-0001-add-makefile.patch` > file that has "Subject: [PATCH v4 1/20] Add makefile" in it. > + now can support non-integrated version number like `-v1.1`. > + > +--previous-count=<n>:: > + Under the premise that we have used `--reroll-count=<n>`, > + we can use `--previous-count=<n>` to specify the previous > + version number. E.g. When we use the `--range-diff` or > + `--interdiff` option and combine with `-v2.3 --previous-count=2.2`, > + "Interdiff against v2.2:" or "Range-diff against v2.2:" > + will be output in the patch. > > --to=<email>:: > Add a `To:` header to the email headers. This is in addition > diff --git a/builtin/log.c b/builtin/log.c > index f67b67d80ed1..95c95eab5393 100644 > --- a/builtin/log.c > +++ b/builtin/log.c > @@ -1662,13 +1662,15 @@ static void print_bases(struct base_tree_info *bases, FILE *file) > oidclr(&bases->base_commit); > } > > -static const char *diff_title(struct strbuf *sb, int reroll_count, > - const char *generic, const char *rerolled) > +static const char *diff_title(struct strbuf *sb, const char *reroll_count, int reroll_count_is_integer, > + const char*previous_count, const char *generic, const char *rerolled) > { > - if (reroll_count <= 0) > + if (!reroll_count || (!reroll_count_is_integer && !previous_count)) > strbuf_addstr(sb, generic); > - else /* RFC may be v0, so allow -v1 to diff against v0 */ > - strbuf_addf(sb, rerolled, reroll_count - 1); > + else if (reroll_count_is_integer)/* RFC may be v0, so allow -v1 to diff against v0 */ > + strbuf_addf(sb, rerolled, atoi(reroll_count) - 1); > + else if (previous_count) > + strbuf_addf(sb, rerolled, previous_count); > return sb->buf; > } I would just remove this hunk entirely and keep the existing logic... > @@ -1717,7 +1719,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) > struct strbuf buf = STRBUF_INIT; > int use_patch_format = 0; > int quiet = 0; > - int reroll_count = -1; > + int reroll_count_is_integer = 0; > + const char *reroll_count = NULL; > + const char *previous_count = NULL; ...then over here, we can do something like const char *reroll_count = NULL; int reroll_count_int = -1; and then... > char *cover_from_description_arg = NULL; > char *branch_name = NULL; > char *base_commit = NULL; > @@ -1751,8 +1755,10 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) > N_("use <sfx> instead of '.patch'")), > OPT_INTEGER(0, "start-number", &start_number, > N_("start numbering patches at <n> instead of 1")), > - OPT_INTEGER('v', "reroll-count", &reroll_count, > - N_("mark the series as Nth re-roll")), > + OPT_STRING('v', "reroll-count", &reroll_count, N_("reroll-count"), > + N_("mark the series as specified version re-roll")), > + OPT_STRING(0, "previous-count", &previous_count, N_("previous-count"), > + N_("specified as the last version while we use --reroll-count")), > OPT_INTEGER(0, "filename-max-length", &fmt_patch_name_max, > N_("max length of output filename")), > OPT_CALLBACK_F(0, "rfc", &rev, NULL, > @@ -1861,10 +1867,20 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) > > if (cover_from_description_arg) > cover_from_description_mode = parse_cover_from_description(cover_from_description_arg); > - > - if (0 < reroll_count) { > + if (previous_count && !reroll_count) > + usage(_("previous-count can only used when reroll-count is used")); > + if (reroll_count) { > struct strbuf sprefix = STRBUF_INIT; > - strbuf_addf(&sprefix, "%s v%d", > + char ch; > + size_t i = 0 , reroll_count_len = strlen(reroll_count); > + > + for (; i != reroll_count_len; i++) { > + ch = reroll_count[i]; > + if(!isdigit(ch)) > + break; > + } > + reroll_count_is_integer = i == reroll_count_len ? 1 : 0; > + strbuf_addf(&sprefix, "%s v%s", > rev.subject_prefix, reroll_count); > rev.reroll_count = reroll_count; > rev.subject_prefix = strbuf_detach(&sprefix, NULL); ...over here we can use Junio's integer parsing example and assign reroll_count_int only if reroll_count can be parsed into an integer. Thanks, Denton
Denton Liu <liu.denton@gmail.com> writes: > Hmm, others may disagree but I don't really like the idea of > `--previous-count`. It may be useful for populating "Range-diff vs <n>" > instead of just "Range-diff" but I don't think it's worth the cost of > maintaining this option. It really depends on the target audience. As a reviewer who may be too busy to read every iteration of a series, I would probably find it useless if it gives just "range-diff" or "range-diff with last" without saying which exact round. Obviously, if you are not doing range-diff, it will not be an issue. If the patch requires (I didn't read the latest one) the previous-count to be given when range-diff or interdiff is not requested, it should probably be fixed. I am also OK with any design decision, as long as it will not close the door for the occasionally requested feature to carry over cover letter material from the previous round to the current one. Thanks.
Junio C Hamano <gitster@pobox.com> 于2021年3月4日周四 上午8:54写道： > > Denton Liu <liu.denton@gmail.com> writes: > > > Hmm, others may disagree but I don't really like the idea of > > `--previous-count`. It may be useful for populating "Range-diff vs <n>" > > instead of just "Range-diff" but I don't think it's worth the cost of > > maintaining this option. > > It really depends on the target audience. As a reviewer who may be > too busy to read every iteration of a series, I would probably find > it useless if it gives just "range-diff" or "range-diff with last" > without saying which exact round. Obviously, if you are not doing > range-diff, it will not be an issue. If the patch requires (I > didn't read the latest one) the previous-count to be given when > range-diff or interdiff is not requested, it should probably be > fixed. > > I am also OK with any design decision, as long as it will not close > the door for the occasionally requested feature to carry over cover > letter material from the previous round to the current one. > > Thanks. What we are arguing now is whether it is necessary to add "aginst v<previous_count>" to the patch when the non-integer version number + rangediff/interdiff is required. Denton's point of view may be similar to that of Eric before. Here are my personal thoughts: Personally, I may use GGG more. When I see a title like "Range-diff vs v1:", I can know that this is a change from the previous v1, and it may be better than "Range-diff again v1" To be more specific, but if it is a small patch such as "v1.2", we use previous_count to tell the reviewer that this is a range-diff change from "v1.1" or other versions. Of course this `previous count` can be used in a very small range, but I think it doesn't hurt to keep it, because even if you don't use it, `format patch` will still output "Range-diff", which will not break any known functions. It can only be said that `previous count` provides an option for submitters to know the previous version for reviewers. In this regard, I agree with Junio's point of view. Thanks.
On Wed, Mar 3, 2021 at 9:08 PM ZheNing Hu <adlternative@gmail.com> wrote: > What we are arguing now is whether it is necessary to add > "aginst v<previous_count>" to the patch when the non-integer version > number + rangediff/interdiff is required. Denton's point of view may be > similar to that of Eric before. Yes, it sounds as if Denton and I share the same point of view. > Here are my personal thoughts: > > Of course this `previous count` can be used in a very small range, but > I think it > doesn't hurt to keep it, because even if you don't use it, `format > patch` will still > output "Range-diff", which will not break any known functions. It can > only be said > that `previous count` provides an option for submitters to know the > previous version > for reviewers. In this regard, I agree with Junio's point of view. I'm not outright opposed to supporting non-numeric, non-integer reroll-counts, but I also don't see a big need for it. As mentioned earlier, Denton is the only person I recall who sends fractional re-rolls, so it's not obvious that there is a big advantage to adding such support and complicating the code just for one person. Also, when Denton does send fractional re-rolls, he typically does so for just a single patch out of a longer series, and he doesn't (I think) provide a range-diff or interdiff for the patch. So, for Denton's intended use-case, this entire discussion about "Range-diff against v$V" and "Interdiff against v$V" seems superfluous. That is, the simple logic: if reroll_count specified and is integer: s = "Range-diff against v${reroll_count -1}:" else s = "Range-diff:" satisfies Denton's case without the complication of adding a --previous-count switch. This probably explains why Denton doesn't see a need for the extra complexity of --previous-count. So, some ways forward are: (1) drop this topic altogether since it so far seems of interest to only a single person (Denton) -- nobody else has asked for it (2) support non-integer reroll-count but just use the simple logic shown above for constructing the "Range-diff/Interdiff against" header; this leaves the door open for Junio's idea(s) of allowing the user to specify --previous-count and automatically determining the previous reroll-count by scanning a directory (3) continue refining the changes made by this patch until reviewers are happy with it (which might take a few more re-rolls) I lean toward #1, but wouldn't be opposed to #2 or #3 either.
Hi Eric, On Wed, Mar 03, 2021 at 10:27:25PM -0500, Eric Sunshine wrote: > I'm not outright opposed to supporting non-numeric, non-integer > reroll-counts, but I also don't see a big need for it. As mentioned > earlier, Denton is the only person I recall who sends fractional > re-rolls, so it's not obvious that there is a big advantage to adding > such support and complicating the code just for one person. Although it is quite rare, I'm definitely not the only one who's done it. A cursory search on the list reveals a few examples from people other than me: * https://lore.kernel.org/git/20180308224458.5730-1-szeder.dev@gmail.com/ * https://lore.kernel.org/git/20190706162114.21169-1-szeder.dev@gmail.com/ * https://lore.kernel.org/git/20180225134015.26964-1-szeder.dev@gmail.com/ * https://lore.kernel.org/git/1435350166-7122-1-git-send-email-Matthieu.Moy@imag.fr/ * https://lore.kernel.org/git/574DED66.6050008@web.de/
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt index 3e49bf221087..f10fa6527f5f 100644 --- a/Documentation/git-format-patch.txt +++ b/Documentation/git-format-patch.txt @@ -22,6 +22,7 @@ SYNOPSIS [--cover-from-description=<mode>] [--rfc] [--subject-prefix=<subject prefix>] [(--reroll-count|-v) <n>] + [--previous-count=<n>] [--to=<email>] [--cc=<email>] [--[no-]cover-letter] [--quiet] [--[no-]encode-email-headers] @@ -221,6 +222,15 @@ populated with placeholder text. `--subject-prefix` option) has ` v<n>` appended to it. E.g. `--reroll-count=4` may produce `v4-0001-add-makefile.patch` file that has "Subject: [PATCH v4 1/20] Add makefile" in it. + now can support non-integrated version number like `-v1.1`. + +--previous-count=<n>:: + Under the premise that we have used `--reroll-count=<n>`, + we can use `--previous-count=<n>` to specify the previous + version number. E.g. When we use the `--range-diff` or + `--interdiff` option and combine with `-v2.3 --previous-count=2.2`, + "Interdiff against v2.2:" or "Range-diff against v2.2:" + will be output in the patch. --to=<email>:: Add a `To:` header to the email headers. This is in addition diff --git a/builtin/log.c b/builtin/log.c index f67b67d80ed1..95c95eab5393 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1662,13 +1662,15 @@ static void print_bases(struct base_tree_info *bases, FILE *file) oidclr(&bases->base_commit); } -static const char *diff_title(struct strbuf *sb, int reroll_count, - const char *generic, const char *rerolled) +static const char *diff_title(struct strbuf *sb, const char *reroll_count, int reroll_count_is_integer, + const char*previous_count, const char *generic, const char *rerolled) { - if (reroll_count <= 0) + if (!reroll_count || (!reroll_count_is_integer && !previous_count)) strbuf_addstr(sb, generic); - else /* RFC may be v0, so allow -v1 to diff against v0 */ - strbuf_addf(sb, rerolled, reroll_count - 1); + else if (reroll_count_is_integer)/* RFC may be v0, so allow -v1 to diff against v0 */ + strbuf_addf(sb, rerolled, atoi(reroll_count) - 1); + else if (previous_count) + strbuf_addf(sb, rerolled, previous_count); return sb->buf; } @@ -1717,7 +1719,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) struct strbuf buf = STRBUF_INIT; int use_patch_format = 0; int quiet = 0; - int reroll_count = -1; + int reroll_count_is_integer = 0; + const char *reroll_count = NULL; + const char *previous_count = NULL; char *cover_from_description_arg = NULL; char *branch_name = NULL; char *base_commit = NULL; @@ -1751,8 +1755,10 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) N_("use <sfx> instead of '.patch'")), OPT_INTEGER(0, "start-number", &start_number, N_("start numbering patches at <n> instead of 1")), - OPT_INTEGER('v', "reroll-count", &reroll_count, - N_("mark the series as Nth re-roll")), + OPT_STRING('v', "reroll-count", &reroll_count, N_("reroll-count"), + N_("mark the series as specified version re-roll")), + OPT_STRING(0, "previous-count", &previous_count, N_("previous-count"), + N_("specified as the last version while we use --reroll-count")), OPT_INTEGER(0, "filename-max-length", &fmt_patch_name_max, N_("max length of output filename")), OPT_CALLBACK_F(0, "rfc", &rev, NULL, @@ -1861,10 +1867,20 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) if (cover_from_description_arg) cover_from_description_mode = parse_cover_from_description(cover_from_description_arg); - - if (0 < reroll_count) { + if (previous_count && !reroll_count) + usage(_("previous-count can only used when reroll-count is used")); + if (reroll_count) { struct strbuf sprefix = STRBUF_INIT; - strbuf_addf(&sprefix, "%s v%d", + char ch; + size_t i = 0 , reroll_count_len = strlen(reroll_count); + + for (; i != reroll_count_len; i++) { + ch = reroll_count[i]; + if(!isdigit(ch)) + break; + } + reroll_count_is_integer = i == reroll_count_len ? 1 : 0; + strbuf_addf(&sprefix, "%s v%s", rev.subject_prefix, reroll_count); rev.reroll_count = reroll_count; rev.subject_prefix = strbuf_detach(&sprefix, NULL); @@ -2079,8 +2095,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) rev.idiff_oid1 = &idiff_prev.oid[idiff_prev.nr - 1]; rev.idiff_oid2 = get_commit_tree_oid(list[0]); rev.idiff_title = diff_title(&idiff_title, reroll_count, - _("Interdiff:"), - _("Interdiff against v%d:")); + reroll_count_is_integer, previous_count, _("Interdiff:"), + reroll_count_is_integer ? _("Interdiff against v%d:") : + _("Interdiff against v%s:")); } if (creation_factor < 0) @@ -2098,8 +2115,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) rev.rdiff2 = rdiff2.buf; rev.creation_factor = creation_factor; rev.rdiff_title = diff_title(&rdiff_title, reroll_count, - _("Range-diff:"), - _("Range-diff against v%d:")); + reroll_count_is_integer, previous_count, _("Range-diff:"), + reroll_count_is_integer ? _("Range-diff against v%d:") : + _("Range-diff against v%s:")); } if (!signature) { diff --git a/log-tree.c b/log-tree.c index 4531cebfab38..5f2e08ebcaab 100644 --- a/log-tree.c +++ b/log-tree.c @@ -369,8 +369,8 @@ void fmt_output_subject(struct strbuf *filename, int start_len = filename->len; int max_len = start_len + info->patch_name_max - (strlen(suffix) + 1); - if (0 < info->reroll_count) - strbuf_addf(filename, "v%d-", info->reroll_count); + if (info->reroll_count) + strbuf_addf(filename, "v%s-", info->reroll_count); strbuf_addf(filename, "%04d-%s", nr, subject); if (max_len < filename->len) diff --git a/revision.h b/revision.h index e6be3c845e66..097d08354c61 100644 --- a/revision.h +++ b/revision.h @@ -235,7 +235,7 @@ struct rev_info { const char *mime_boundary; const char *patch_suffix; int numbered_files; - int reroll_count; + const char *reroll_count; char *message_id; struct ident_split from_ident; struct string_list *ref_message_ids; diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh index 1b26c4c2ef91..ffb91f27f75f 100755 --- a/t/t3206-range-diff.sh +++ b/t/t3206-range-diff.sh @@ -521,6 +521,22 @@ test_expect_success 'format-patch --range-diff as commentary' ' grep "> 1: .* new message" 0001-* ' +test_expect_success 'format-patch --range-diff reroll-count with a non-integer and previous-count ' ' + git format-patch --range-diff=HEAD~1 -v2.9 --previous-count=2.8 HEAD~1 >actual && + test_when_finished "rm v2.9-0001-*" && + test_line_count = 1 actual && + test_i18ngrep "^Range-diff ..* v2.8:$" v2.9-0001-* && + grep "> 1: .* new message" v2.9-0001-* +' + +test_expect_success 'format-patch --range-diff reroll-count with a integer previous-count' ' + git format-patch --range-diff=HEAD~1 -v2 --previous-count=1.8 HEAD~1 >actual && + test_when_finished "rm v2-0001-*" && + test_line_count = 1 actual && + test_i18ngrep "^Range-diff ..* v1:$" v2-0001-* && + grep "> 1: .* new message" v2-0001-* +' + test_expect_success 'range-diff overrides diff.noprefix internally' ' git -c diff.noprefix=true range-diff HEAD^... ' diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh index 66630c8413d5..1b3c1dc273aa 100755 --- a/t/t4014-format-patch.sh +++ b/t/t4014-format-patch.sh @@ -378,6 +378,14 @@ test_expect_success 'reroll count' ' ! grep -v "^Subject: \[PATCH v4 [0-3]/3\] " subjects ' +test_expect_success 'reroll count with a non-integer' ' + rm -fr patches && + git format-patch -o patches --cover-letter --reroll-count 4.4 main..side >list && + ! grep -v "^patches/v4.4-000[0-3]-" list && + sed -n -e "/^Subject: /p" $(cat list) >subjects && + ! grep -v "^Subject: \[PATCH v4.4 [0-3]/3\] " subjects +' + test_expect_success 'reroll count (-v)' ' rm -fr patches && git format-patch -o patches --cover-letter -v4 main..side >list && @@ -386,6 +394,14 @@ test_expect_success 'reroll count (-v)' ' ! grep -v "^Subject: \[PATCH v4 [0-3]/3\] " subjects ' +test_expect_success 'reroll count (-v) with a non-integer' ' + rm -fr patches && + git format-patch -o patches --cover-letter -v4.4 main..side >list && + ! grep -v "^patches/v4.4-000[0-3]-" list && + sed -n -e "/^Subject: /p" $(cat list) >subjects && + ! grep -v "^Subject: \[PATCH v4.4 [0-3]/3\] " subjects +' + check_threading () { expect="$1" && shift && @@ -2255,6 +2271,23 @@ test_expect_success 'interdiff: reroll-count' ' test_i18ngrep "^Interdiff ..* v1:$" v2-0000-cover-letter.patch ' +test_expect_success 'interdiff: reroll-count with a non-integer' ' + git format-patch --cover-letter --interdiff=boop~2 -v2.2 -1 boop && + test_i18ngrep "^Interdiff:$" v2.2-0000-cover-letter.patch +' + +test_expect_success 'interdiff: reroll-count with a non-integer and previous-count ' ' + git format-patch --cover-letter --interdiff=boop~2 -v2.2 --previous-count=2.1 -1 boop && + test_i18ngrep "^Interdiff ..* v2.1:$" v2.2-0000-cover-letter.patch +' + +test_expect_success 'interdiff: reroll-count with a integer and previous-count ' ' + git format-patch --cover-letter --interdiff=boop~2 -v2 --previous-count=1.5 -1 boop && + test_i18ngrep "^Interdiff ..* v1:$" v2-0000-cover-letter.patch +' +test_expect_success 'interdiff: previous-count without reroll-count ' ' + test_must_fail git format-patch --cover-letter --interdiff=boop~2 --previous-count=1.5 -1 boop +' test_expect_success 'interdiff: solo-patch' ' cat >expect <<-\EOF && +fleep