Re: [PATCH 3/5] merge-recursive: clarify the rename_dir/RENAME_DIR meaning
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- May 21, 2018, 14:28 UTC
- Message-ID
- <nycvar.QRO.7.76.6.1805211626240.77@tvgsbejvaqbjf.bet>
- In-Reply-To
- <20180519020700.2241-4-newren@gmail.com>
Hi Elijah,
On Fri, 18 May 2018, Elijah Newren wrote:
Show 12 quoted lines
> 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.
Show 17 quoted lines
> 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 theTechnically, we do not move the path, but the file.
The rest of the diff looks obviously correct to me.
Thanks, Dscho