On Fri, May 3, 2024 at 2:00 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > Dragan Simic <dsimic@xxxxxxxxxxx> writes: > > Just a small suggestion... Perhaps the creation factor needs adjusting > > for the range diff to actually be produced. [...] > > I see this happen to too many series I see on the list. There are > cases when the user knows that they are comparing an old and a new > iterations of the same series, e.g. running it from format-patch. > We probably should use a much higher creation factor than default to > run range-diff in such a context. > > IOW, this shouldn't have to be done by individual users, but by the > tool. > > Perhaps something along this line may not be a bad idea. > > ----- >8 --------- >8 --------- >8 --------- >8 ----- > Subject: [PATCH] format-patch: run range-diff with larger creation-factor > > We see too often that a range-diff added to format-patch output > shows too many "unmatched" patches. This is because the default > value for creation-factor is set to a relatively low value. > > It may be justified for other uses (like you have a yet-to-be-sent > new iteration of your series, and compare it against the 'seen' > branch that has an older iteration, probably with the '--left-only' > option, to pick out only your patches while ignoring the others) of > "range-diff" command, but when the command is run as part of the > format-patch, the user _knows_ and expects that the patches in the > old and the new iterations roughly correspond to each other, so we > can and should use a much higher default. > > Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx> Ævar had posted[1] pretty much the exact same patch a few years ago. At the time, I had trouble understanding why `git range-diff` and `git format-patch --range-dif` would need separate default creation factors[2], and I still have trouble understanding why they should be different. Aren't both commands addressing the same use-case of comparing one version of a series against a subsequent version? In your response[3], you seemed to agree with that observation and suggested instead simply increasing the default creation factor for both commands (which sounds reasonable to me). [1]: https://lore.kernel.org/git/87y35g9l18.fsf@xxxxxxxxxxxxxxxxxxx/ [2]: https://lore.kernel.org/git/CAPig+cRMiEcXVRYrgp+B3tcDreh41-a5_k0zABe+HNce0G=Cyw@xxxxxxxxxxxxxx/ [3]: https://lore.kernel.org/git/xmqqzhps4uyq.fsf@xxxxxxxxxxxxxxxxxxxxxxxxx/ > --- > diff --git c/builtin/log.c w/builtin/log.c > index 4da7399905..7a019476c3 100644 > --- c/builtin/log.c > +++ w/builtin/log.c > @@ -2294,7 +2294,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) > if (creation_factor < 0) > - creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT; > + creation_factor = CREATION_FACTOR_FOR_THE_SAME_SERIES; > else if (!rdiff_prev) > die(_("the option '%s' requires '%s'"), "--creation-factor", "--range-diff"); > diff --git c/range-diff.h w/range-diff.h > index 04ffe217be..2f69f6a434 100644 > --- c/range-diff.h > +++ w/range-diff.h > @@ -6,6 +6,12 @@ > #define RANGE_DIFF_CREATION_FACTOR_DEFAULT 60 > > +/* > + * A much higher value than the default, when we KNOW we are comparing > + * the same series (e.g., used when format-patch calls range-diff). > + */ > +#define CREATION_FACTOR_FOR_THE_SAME_SERIES 999
