On Wed, Feb 24, 2021 at 7:25 AM Derrick Stolee <stolee@xxxxxxxxx> wrote:
>
> On 2/23/2021 6:43 PM, Elijah Newren via GitGitGadget wrote:
> > ... While the
> > diffstat looks large, viewing this commit with --color-moved makes it
> > clear that only about 20 lines changed. With this patch, the
> > computation of dir_rename_count is still only done after inexact rename
> > detection, but subsequent commits will add a preliminary computation of
> > dir_rename_count after exact rename detection, followed by some updates
> > after inexact rename detection.
>
> The --color-moved recommendation is a good one. Everything seems to be
> pretty standard, except this function:
>
> > +static void compute_dir_rename_counts(struct strmap *dir_rename_count,
> > + struct strset *dirs_removed)
> > +{
> > + int i;
> > +
> > + /* Set up dir_rename_count */
> > + for (i = 0; i < rename_dst_nr; ++i) {
> > + /*
> > + * Make dir_rename_count contain a map of a map:
> > + * old_directory -> {new_directory -> count}
> > + * In other words, for every pair look at the directories for
> > + * the old filename and the new filename and count how many
> > + * times that pairing occurs.
> > + */
> > + update_dir_rename_counts(dir_rename_count, dirs_removed,
> > + rename_dst[i].p->one->path,
> > + rename_dst[i].p->two->path);
> > + }
> > +}
>
> is very similar to this:
>
> > -static void compute_rename_counts(struct diff_queue_struct *pairs,
> > - struct strmap *dir_rename_count,
> > - struct strset *dirs_removed)
> > -{
> > - int i;
> > -
> > - for (i = 0; i < pairs->nr; ++i) {
> > - struct diff_filepair *pair = pairs->queue[i];
> > -
> > - /* File not part of directory rename if it wasn't renamed */
> > - if (pair->status != 'R')
> > - continue;
> > -
> > - /*
> > - * Make dir_rename_count contain a map of a map:
> > - * old_directory -> {new_directory -> count}
> > - * In other words, for every pair look at the directories for
> > - * the old filename and the new filename and count how many
> > - * times that pairing occurs.
> > - */
> > - update_dir_rename_counts(dir_rename_count, dirs_removed,
> > - pair->one->path,
> > - pair->two->path);
> > - }
> > -}
> > -
>
> but we dropped that "File not part of directory rename" check.
>
> It seems that is no longer possible to use with the new data structure,
> but I wonder if this will cause a slowdown in the directory renames when
> merging? Or, has the data already been filtered before calling
> compute_dir_rename_counts()?
Oh, indeed, good catch. The
if (pair->status != 'R')
continue;
check should have been translated to
if (!rename_dst[i].is_rename)
continue;
Such a check _was_ added later in the series except with more code
than just the "continue" that didn't make sense at this stage; the
extra code made me overlook it when I was splitting. Oops. I'll add
this check back in; thanks for catching.
