Re: [PATCH v6] format-patch: allow a non-integral version numbers

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Mar 19, 2021, 06:00 UTC
Message-ID: <CAPig+cTv55r+Nr9pb1jQh99YjzkWgjfeYFUEgcMr1LM9g=5xXA@mail.gmail.com>
In-Reply-To: <pull.885.v6.git.1616047200968.gitgitgadget@gmail.com>

On Thu, Mar 18, 2021 at 2:00 AM ZheNing Hu via GitGitGadget <gitgitgadget@gmail.com> wrote:

Show 31 quoted lines

> Usually we can only use `format-patch -v<n>` to generate integral
> version numbers patches, but sometimes a same fixup should be
> labeled as a non-integral version like `v1.1`, so teach `format-patch`
> to allow a non-integral version which may be helpful to send those
> patches.
>
> `<n>` can be any string, such as `-v1.1`.  In the case where it
> is a non-integral value, the "Range-diff" and "Interdiff"
> headers will not include the previous version.
>
> Signed-off-by: ZheNing Hu <adlternative@gmail.com>
> ---
> diff --git a/builtin/log.c b/builtin/log.c
> @@ -1662,13 +1662,18 @@ static void print_bases(struct base_tree_info *bases, FILE *file)
> +static const char *diff_title(struct strbuf *sb,
> +                             const char *reroll_count,
> +                             const char *generic,
> +                             const char *rerolled)
>  {
> -       if (reroll_count <= 0)
> +       int v;
> +
> +       /* RFC may be v0, so allow -v1 to diff against v0 */
> +       if (reroll_count && !strtol_i(reroll_count, 10, &v))
> +               strbuf_addf(sb, rerolled, v - 1);
> +       else
>                 strbuf_addstr(sb, generic);
> -       else /* RFC may be v0, so allow -v1 to diff against v0 */
> -               strbuf_addf(sb, rerolled, reroll_count - 1);
>         return sb->buf;
>  }

The comment about RFC and v0 doesn't really make sense anymore. Its original purpose was to explain why the `if` condition (which goes away with this patch) was `<=0` rather than `<=1`. It might make sense to keep the comment if the code is written like this:

    if (reroll_count &&
            !strtol_i(reroll_count, 10, &v) &&
            reroll_count >= 1)
        strbuf_addf(sb, rerolled, v - 1);
    else
        ...

However, I'm not sure it's worth re-rolling just to make this change.

Thanks.