Re: [PATCH v3 02/10] diffcore-rename: add a new idx_possible_rename function

From: Derrick Stolee <stolee@gmail.com>
Date: Feb 26, 2021, 15:52 UTC
Message-ID: <9edad7cc-f578-3b55-394a-08b351eaa7c5@gmail.com>
In-Reply-To: <40f57bcc2055773a7dd35bde50c54354a643d07a.1614304700.git.gitgitgadget@gmail.com>

On 2/25/2021 8:58 PM, Elijah Newren via GitGitGadget wrote:

> Anyway, that's a lot of background so that we can actually describe the
> new function.  Add an idx_possible_rename() function which combines the
> recently added dir_rename_guess and idx_map fields to provide the index
> within rename_dst of a potential match for a given file.

This paragraph could use rewording now that idx_possible_rename() is not new to this commit, and dir_rename_guess() is introduced here (but not yet initialized). This would be a good time to introduce the dir_rename_info struct and how it _will_ be populated.

> -static int idx_possible_rename(char *filename)
> +static int idx_possible_rename(char *filename, struct dir_rename_info *info)
>  {

...

> +	if (!info->setup)
> +		return -1;

It might be worth noting in the commit message that since info.setup is always zero, this implementation is not run (yet).

In general, I'm finding this a lot easier to read the code in the presented order. The commit messages need to be massaged to match.

Thanks, -Stolee