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

"ZheNing Hu via GitGitGadget" [off-list ref] writes:

From: ZheNing Hu <redacted>

Usually we can only use `format-patch -v<n>` to generate integral
version numbers patches, but sometimes a same fixup should be

I guess that you meant "a small fixup" here?

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`.

I would probably use "such as '3.1' or '4rev2'" (without the leading
"-v", as that is how you introduce <n> in the first sentence)
instead in the proposed log message, and give a pair of examples to
show both fractional numbers and non numbers are accepted in the
documentation, if I were doing this patch.

In the case where it
is a non-integral value, the "Range-diff" and "Interdiff"
headers will not include the previous version.

Which is rather unfortunate, but I do not care strongly enough (I
would rather discurage use of partial rerolls with these fractinal 
bits anyway).

Range-diff vs v6:

 1:  d5f5e3f073de ! 1:  95cfe75ee7da format-patch: allow a non-integral version numbers
     @@ builtin/log.c: static void print_bases(struct base_tree_info *bases, FILE *file)
      +	int v;
      +
      +	/* RFC may be v0, so allow -v1 to diff against v0 */
     -+	if (reroll_count && !strtol_i(reroll_count, 10, &v))
     ++	if (reroll_count && !strtol_i(reroll_count, 10, &v) &&
     ++	    v >= 1)
      +		strbuf_addf(sb, rerolled, v - 1);
      +	else
       		strbuf_addstr(sb, generic);

In one of your review responses earlier, you hinted that limiting
the range of v like this was needed for correctness.  There should
be a new test to cover the case that this change makes a difference.

Junio C Hamano [off-list ref] 于2021年3月20日周六 上午1:29写道：

"ZheNing Hu via GitGitGadget" [off-list ref] writes:

quoted

From: ZheNing Hu <redacted>

Usually we can only use `format-patch -v<n>` to generate integral
version numbers patches, but sometimes a same fixup should be

I guess that you meant "a small fixup" here?

Yes.

quoted

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`.

I would probably use "such as '3.1' or '4rev2'" (without the leading
"-v", as that is how you introduce <n> in the first sentence)
instead in the proposed log message, and give a pair of examples to
show both fractional numbers and non numbers are accepted in the
documentation, if I were doing this patch.

Sure, either a "fractional" version or a "non-nummbers" version can be
accepted. I will complete the expansion of these documents.

quoted

In the case where it
is a non-integral value, the "Range-diff" and "Interdiff"
headers will not include the previous version.

Which is rather unfortunate, but I do not care strongly enough (I
would rather discurage use of partial rerolls with these fractinal
bits anyway).

Well, now this patch (no previous-count) was born in the previous
suggestion of Denton Liu. I also hope it can be more practical.

quoted

Range-diff vs v6:

 1:  d5f5e3f073de ! 1:  95cfe75ee7da format-patch: allow a non-integral version numbers
     @@ builtin/log.c: static void print_bases(struct base_tree_info *bases, FILE *file)
      +       int v;
      +
      +       /* RFC may be v0, so allow -v1 to diff against v0 */
     -+       if (reroll_count && !strtol_i(reroll_count, 10, &v))
     ++       if (reroll_count && !strtol_i(reroll_count, 10, &v) &&
     ++           v >= 1)
      +               strbuf_addf(sb, rerolled, v - 1);
      +       else
                      strbuf_addstr(sb, generic);

In one of your review responses earlier, you hinted that limiting
the range of v like this was needed for correctness.  There should
be a new test to cover the case that this change makes a difference.

I will do it.

Thanks.