| Message ID | 95a3fcb8be08cce186144d175a6cda71ab42d445.1607223276.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | diffcore-rename improvements | expand |
On Sun, Dec 06, 2020 at 02:54:30AM +0000, Elijah Newren via GitGitGadget wrote: > From: Elijah Newren <newren@gmail.com> > > too_many_rename_candidates() got the number of rename targets via an > argument to the function, but the number of rename sources via a global > variable. That felt rather inconsistent. Pass in the number of rename > sources as an argument as well. > > While we are at it... We had a local variable, num_src, that served two > purposes. Initially it was set to this global value, but later was used > for counting a subset of the number of sources. Since we now have a > function argument for the former usage, introduce a clearer variable > name for the latter usage. > > This patch has no behavioral changes; it's just renaming and passing an > argument instead of grabbing it from the global namespace. (You may > find it easier to view the patch using git diff's --color-words option.) > > Signed-off-by: Elijah Newren <newren@gmail.com> > --- > diffcore-rename.c | 23 +++++++++++------------ > 1 file changed, 11 insertions(+), 12 deletions(-) > > diff --git a/diffcore-rename.c b/diffcore-rename.c > index d367a6d2443..68ddf51a2a1 100644 > --- a/diffcore-rename.c > +++ b/diffcore-rename.c > @@ -434,12 +434,11 @@ static void record_if_better(struct diff_score m[], struct diff_score *o) > * 1 if we need to disable inexact rename detection; > * 2 if we would be under the limit if we were given -C instead of -C -C. > */ > -static int too_many_rename_candidates(int num_create, > +static int too_many_rename_candidates(int num_targets, int num_sources, OK, so num_create becomes num_targets, and the global num_src becomes a new parameter named num_sources. Makes sense, but it took me a second to figure all of that out. I don't think that you need to split this across two patches (e.g., one to rename num_targets, and another to introduce the num_sources parameter), but including the new names for each of these variables in the patch message might make this clearer to follow. > struct diff_options *options) > { > int rename_limit = options->rename_limit; > - int num_src = rename_src_nr; > - int i; > + int i, limited_sources; > [...] Everything else from here looks fine, and indeed viewing this with --color-words made it much easier to verify. Thanks, Taylor
diff --git a/diffcore-rename.c b/diffcore-rename.c index d367a6d2443..68ddf51a2a1 100644 --- a/diffcore-rename.c +++ b/diffcore-rename.c @@ -434,12 +434,11 @@ static void record_if_better(struct diff_score m[], struct diff_score *o) * 1 if we need to disable inexact rename detection; * 2 if we would be under the limit if we were given -C instead of -C -C. */ -static int too_many_rename_candidates(int num_create, +static int too_many_rename_candidates(int num_targets, int num_sources, struct diff_options *options) { int rename_limit = options->rename_limit; - int num_src = rename_src_nr; - int i; + int i, limited_sources; options->needed_rename_limit = 0; @@ -447,30 +446,30 @@ static int too_many_rename_candidates(int num_create, * This basically does a test for the rename matrix not * growing larger than a "rename_limit" square matrix, ie: * - * num_create * num_src > rename_limit * rename_limit + * num_targets * num_sources > rename_limit * rename_limit */ if (rename_limit <= 0) rename_limit = 32767; - if ((num_create <= rename_limit || num_src <= rename_limit) && - ((uint64_t)num_create * (uint64_t)num_src + if ((num_targets <= rename_limit || num_sources <= rename_limit) && + ((uint64_t)num_targets * (uint64_t)num_sources <= (uint64_t)rename_limit * (uint64_t)rename_limit)) return 0; options->needed_rename_limit = - num_src > num_create ? num_src : num_create; + num_sources > num_targets ? num_sources : num_targets; /* Are we running under -C -C? */ if (!options->flags.find_copies_harder) return 1; /* Would we bust the limit if we were running under -C? */ - for (num_src = i = 0; i < rename_src_nr; i++) { + for (limited_sources = i = 0; i < num_sources; i++) { if (diff_unmodified_pair(rename_src[i].p)) continue; - num_src++; + limited_sources++; } - if ((num_create <= rename_limit || num_src <= rename_limit) && - ((uint64_t)num_create * (uint64_t)num_src + if ((num_targets <= rename_limit || limited_sources <= rename_limit) && + ((uint64_t)num_targets * (uint64_t)limited_sources <= (uint64_t)rename_limit * (uint64_t)rename_limit)) return 2; return 1; @@ -576,7 +575,7 @@ void diffcore_rename(struct diff_options *options) if (!num_create) goto cleanup; - switch (too_many_rename_candidates(num_create, options)) { + switch (too_many_rename_candidates(num_create, rename_src_nr, options)) { case 1: goto cleanup; case 2: