Eric Sunshine <sunshine@xxxxxxxxxxxxxx> 于2021年2月26日周五 上午1:57写道： > > On Thu, Feb 25, 2021 at 11:19 AM ZheNing Hu via GitGitGadget > <gitgitgadget@xxxxxxxxx> wrote: > > Usually we can only use `format-patch -v<n>` to generate integral > > version numbers patches, but if we can provide `format-patch` with > > non-integer versions numbers of patches, this may help us to send patches > > such as "v1.1" versions sometimes. > > On the Git project itself, fractional or non-numeric re-roll "numbers" > are not necessarily encouraged[1], so this feature may not be > particularly useful here, though perhaps some other project might > benefit from it(?). Usually, you would want to justify why the change > is desirable. Denton did give a bit of justification in his > proposal[2] for this feature, so perhaps update this commit message by > copying some of what he wrote as justification. > OK, I will remember it. > [1]: I think I've only seen Denton send fractional re-rolls; other > people sometimes send a periodic "fixup!" patch, but both approaches > place extra burden on the project maintainer than merely re-rolling > the entire series with a new integer re-roll count. > > [2]: https://github.com/gitgitgadget/git/issues/882 > > > Signed-off-by: ZheNing Hu <adlternative@xxxxxxxxx> > > --- > > diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt > > @@ -215,12 +215,12 @@ populated with placeholder text. > > -v <n>:: > > --reroll-count=<n>:: > > - Mark the series as the <n>-th iteration of the topic. The > > + Mark the series as the specified version of the topic. The > > output filenames have `v<n>` prepended to them, and the > > subject prefix ("PATCH" by default, but configurable via the > > `--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. > > + `--reroll-count 4.4` may produce `v4.4-0001-add-makefile.patch` > > + file that has "Subject: [PATCH v4.4 1/20] Add makefile" in it. > > I'm not sure we want to encourage the use of fractional re-roll counts > by using it in an example like this. It would probably be better to > leave the example as-is. If you really want people to know that > fractional re-roll counts are supported, perhaps add separate sentence > saying that they are. Yes, but the original description `<n>-th iteration` may imply that the version number is an integer. Is there any good way to solve it? > > > diff --git a/builtin/log.c b/builtin/log.c > > @@ -1662,13 +1662,13 @@ static void print_bases(struct base_tree_info *bases, FILE *file) > > -static const char *diff_title(struct strbuf *sb, int reroll_count, > > +static const char *diff_title(struct strbuf *sb, const char *reroll_count, > > const char *generic, const char *rerolled) > > { > > - if (reroll_count <= 0) > > + if (!reroll_count) > > strbuf_addstr(sb, generic); > > else /* RFC may be v0, so allow -v1 to diff against v0 */ > > - strbuf_addf(sb, rerolled, reroll_count - 1); > > + strbuf_addf(sb, rerolled, "last version"); > > return sb->buf; > > } > > There are a couple problems here (at least). First, the string "last > version" should be localizable, `_("last version")`. Second, in > Denton's proposal[2], he suggested using the string "last version" > _only_ if the re-roll count is not an integer. What you have here > applies "last version" unconditionally when -v is used so that the > outcome is _always_ "Range-diff since last version". If that's what > you intend to do, there's no reason to do any sort of interpolation > using the template "Range-diff since %". What Denton had in mind was > this (using pseudo-code): > > if re-roll count not specified: > message = "Range-diff" > else if re-roll count is integer: > message = "Range-diff since v%d", re-roll > else: > message = "Range-diff since v%s", re-roll > > However, there isn't a good reason to favor "Range-diff since last > version" over the simpler generic message "Range-diff". So, the above > should be collapsed to:interpolation > > if re-roll count is specified and integer: > message = "Range-diff since v%d", re-roll > else: > message = "Range-diff" > You mean using "Range-diff since %" may not be as good as" Range-diff" without sorting. I agree with you. > > @@ -2080,7 +2080,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) > > - _("Interdiff against v%d:")); > > + _("Interdiff against %s:")); > > @@ -2099,7 +2099,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) > > - _("Range-diff against v%d:")); > > + _("Range-diff against %s:")); > > If you follow my recommendation above using the simplified > conditional, then you don't need to drop the "v" since you won't be > saying "last version". Your suggestion is very good and easy to implement, but I may have made some changes in Junio suggestion later, that is, I used the `previous_count` method to provide it to `diff_title()`. I will explain my thoughts and problems in my reply to Junio. You'll see it later. Thank you for your help. -- ZheNing Hu
