Re: [PATCH 3/5] merge-recursive: clarify the rename_dir/RENAME_DIR meaning

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Elijah,

On Fri, 18 May 2018, Elijah Newren wrote:

> We had an enum of rename types which included RENAME_DIR; this name felt
> misleading since it was not about an entire directory but was a status for
> each individual file add that occurred within a renamed directory.  Since
> this type is for signifying that the files in question were being renamed
> due to directory rename detection, rename this enum value to
> RENAME_VIA_DIR.  Make a similar change to the conflict_rename_dir()
> function, and add a comment to the top of that function explaining its
> purpose (it may not be quite as obvious as for the other
> conflict_rename_*() functions).
> 
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---

Make sense. I think the reading flow could be improved a little bit by
splitting this paragraph into three.

> diff --git a/merge-recursive.c b/merge-recursive.c
> index 01306c87eb..d30085d9c7 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> -static int conflict_rename_dir(struct merge_options *o,
> -			       struct diff_filepair *pair,
> -			       const char *rename_branch,
> -			       const char *other_branch)
> +static int conflict_rename_via_dir(struct merge_options *o,
> +				   struct diff_filepair *pair,
> +				   const char *rename_branch,
> +				   const char *other_branch)
>  {
> +	/*
> +	 * Handle file adds that need to be renamed due to directory rename
> +	 * detection.  This differs from handle_rename_normal, because
> +	 * there is no content merge to do; just move the path into the

Technically, we do not move the path, but the file.

The rest of the diff looks obviously correct to me.

Thanks,
Dscho