On Sun, Dec 06, 2020 at 02:54:30AM +0000, Elijah Newren via GitGitGadget wrote:
> From: Elijah Newren <newren@xxxxxxxxx>
>
> 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@xxxxxxxxx>
> ---
> 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
