Hi Ævar and Eric, On Fri, 15 Mar 2019, Ævar Arnfjörð Bjarmason wrote: > > On Fri, Mar 15 2019, Eric Sunshine wrote: > > > On Fri, Mar 15, 2019 at 12:09 PM Ævar Arnfjörð Bjarmason > > <avarab@xxxxxxxxx> wrote: > >> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt > >> @@ -261,6 +261,10 @@ material (this may change in the future). > >> +Defaults to 90, whereas the linkgit:git-range-diff[1] default is > >> +60. It's assumed that you're submitting a new patch series & that we > >> +should try harder than normal to find similarities. > > > > My understanding was that the primary use-case of git-range-diff > > itself (which existed before the --range-diff option was added to > > git-format-patch) was to generate a "range diff" for a cover letter of > > a re-rolled series. So, I'm confused about why this tweaks the default > > value of one command but not the other. > > > >> diff --git a/range-diff.h b/range-diff.h > >> @@ -4,6 +4,7 @@ > >> #define RANGE_DIFF_CREATION_FACTOR_DEFAULT 60 > >> +#define RANGE_DIFF_CREATION_FACTOR_FORMAT_PATCH_DEFAULT 90 > > > > The point of introducing RANGE_DIFF_CREATION_FACTOR_DEFAULT in the > > first place was to ensure that the default creation-factor didn't get > > out-of-sync between git-range-diff and git-format-patch., Thus, > > introducing this sort of inconsistency between the two would likely > > lead to confusion on the part of users. After all, --range-diff was > > added to git-format-patch merely as a convenience over having to run > > git-range-diff separately and copy/pasting its output into a cover > > letter generated by git-format-patch. If the two programs default to > > different values, then that "convenience equality" breaks down. > > > > So, I'm fairly negative on this change. However, that doesn't mean I > > would oppose tweaking the value shared between the two programs (and > > also the default used by GitGitGadget, if it specifies it manually), > > though I defer to Dscho in that regard. GitGitGadget does not specify the creation factor manually. > I think that was the intention initially, but at least I'm now using > range-diff as a general comparison tool of different non-ff-branches, > e.g. the force-pushes to "pu". Interesting ;-) > I'd expect the tool in general to be used like that, whereas with > format-patch it's safe to say we're dealing with a re-roll of the same > thing. > > Hence the hypothesis that for format-patch we can be more aggressive > about finding similarities. I do agree that `format-patch`'s `--range-diff` is certainly exclusively used for comparing different iterations of the same patch series. As such, I do agree with Ævar that it makes sense to have a *different* default for the creation factor. Having said that, I did notice while porting `tbdiff` to C that it would be a neat idea to put more weight behind the differences of the commit message, and maybe even use a *different* measure on the commit message, too. Personally, I would try to use a variation of the word diff (maybe something that reflects my experience that it is common to change the case in the oneline, to add a sentence here and there, or to delete a sentence, but not so much to replace entire sentences), to account for the fact that the commit message rarely changes substantially between iterations. So I guess there is a lot of room for improvement here: code (read: diff) changes simply are not the same as commit message changes. Ciao, Dscho
