On Thu, Jan 07, 2021 at 09:35:55PM +0000, Elijah Newren via GitGitGadget wrote:
> From: Elijah Newren <newren@xxxxxxxxx>
>
> This function is based on the first half of get_directory_renames() from
> merge-recursive.c
>
> Signed-off-by: Elijah Newren <newren@xxxxxxxxx>
> ---
> merge-ort.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 51 insertions(+), 2 deletions(-)
>
> diff --git a/merge-ort.c b/merge-ort.c
> index 7e0cc597055..a8fcc026031 100644
> --- a/merge-ort.c
> +++ b/merge-ort.c
> @@ -721,7 +721,6 @@ static int handle_content_merge(struct merge_options *opt,
>
> /*** Function Grouping: functions related to directory rename detection ***/
>
> -MAYBE_UNUSED
> static void get_renamed_dir_portion(const char *old_path, const char *new_path,
> char **old_dir, char **new_dir)
> {
> @@ -825,11 +824,61 @@ static void get_renamed_dir_portion(const char *old_path, const char *new_path,
> *new_dir = xstrndup(new_path, end_of_new - new_path);
> }
>
> +static void increment_count(struct strmap *dir_rename_count,
> + char *old_dir,
> + char *new_dir)
> +{
> + struct strintmap *counts;
> + struct strmap_entry *e;
> +
> + /* Get the {new_dirs -> counts} mapping using old_dir */
> + e = strmap_get_entry(dir_rename_count, old_dir);
> + if (e) {
> + counts = e->value;
> + } else {
> + counts = xmalloc(sizeof(*counts));
> + strintmap_init_with_options(counts, 0, NULL, 1);
> + strmap_put(dir_rename_count, old_dir, counts);
> + }
> +
> + /* Increment the count for new_dir */
> + strintmap_incr(counts, new_dir, 1);
> +}
> +
> static void compute_rename_counts(struct diff_queue_struct *pairs,
> struct strmap *dir_rename_count,
> struct strset *dirs_removed)
> {
> - die("Not yet implemented!");
> + int i;
> +
> + for (i = 0; i < pairs->nr; ++i) {
> + char *old_dir, *new_dir;
> + struct diff_filepair *pair = pairs->queue[i];
> +
> + if (pair->status != 'R')
> + continue;
This had a useful comment in merge-recursive.c that I think would help
future readers here. Would you consider brining over the comment that
starts with "File not part of directory rename ..." here?
> + /* Get the old and new directory names */
> + get_renamed_dir_portion(pair->one->path, pair->two->path,
> + &old_dir, &new_dir);
This spacing is a little odd to me, but it does come directly from
merge-recursive.c. I don't feel strongly about it.
> + if (!old_dir)
> + /* Directory didn't change at all; ignore this one. */
> + 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.
> + */
> + if (strset_contains(dirs_removed, old_dir))
> + increment_count(dir_rename_count, old_dir, new_dir);
Much clearer. If you're rerolling anyway, it may be worth it to say
something about this movement in the patch message: "along the way,
factor out the routine to update the bookkeeping to track the number of
items renamed into new directories".
> + /* Free resources we don't need anymore */
> + free(old_dir);
> + free(new_dir);
> + }
And obviously this is new, but it makes sense here. Thanks.
Thanks,
Taylor
