On 2/9/2021 6:32 AM, Elijah Newren via GitGitGadget wrote:
> + num_sources = rename_src_nr;
> +
> + if (want_copies || break_idx) {
> + /*
> + * Cull sources:
> + * - remove ones corresponding to exact renames
> + */
> + trace2_region_enter("diff", "cull after exact", options->repo);
> + remove_unneeded_paths_from_src(want_copies);
> + trace2_region_leave("diff", "cull after exact", options->repo);
Isn't this the same as
> + } else {
> + /* Determine minimum score to match basenames */
> + int min_basename_score = (int)(5*minimum_score + 0*MAX_SCORE)/5;
> +
> + /*
> + * Cull sources:
> + * - remove ones involved in renames (found via exact match)
> + */
> + trace2_region_enter("diff", "cull exact", options->repo);
> + remove_unneeded_paths_from_src(want_copies);
> + trace2_region_leave("diff", "cull exact", options->repo);
...this? (except the regions are renamed)
Could this be simplified as:
+ num_sources = rename_src_nr;
+
+ trace2_region_enter("diff", "cull after exact", options->repo);
+ remove_unneeded_paths_from_src(want_copies);
+ trace2_region_leave("diff", "cull after exact", options->repo);
+
+ if (!want_copies && !break_idx) {
+ /* Determine minimum score to match basenames */
I realize you probably changed the region names on purpose to distinguish
that there are two "cull" regions in the case of no copies, but I think
that isn't really worth different names. Better to have a consistent region
name around the same activity in both cases.
> + int min_basename_score = (int)(5*minimum_score + 0*MAX_SCORE)/5;
Did you intend for this to be 5*min + 0*MAX? This seems wrong if you want
this value to be different from minimum_score.
> +
> + /* Utilize file basenames to quickly find renames. */
> + trace2_region_enter("diff", "basename matches", options->repo);
> + rename_count += find_basename_matches(options,
> + min_basename_score,
> + rename_src_nr);
> + trace2_region_leave("diff", "basename matches", options->repo);
> +
> + /*
> + * Cull sources, again:
> + * - remove ones involved in renames (found via basenames)
> + */
> + trace2_region_enter("diff", "cull basename", options->repo);
> + remove_unneeded_paths_from_src(want_copies);
> + trace2_region_leave("diff", "cull basename", options->repo);
> + }
> +
Thanks,
-Stolee
