Subject: Re: [PATCH 2/9] merge-recursive: clarify code in was_tracked()

From: Junio C Hamano <[email-masked]>
Date: Wed, 29 Jun 2016 11:35:26 -0700

Johannes Schindelin <[email-masked]> writes:

> It can be puzzling to see that was_tracked() tries to match an index
> entry by name even if cache_name_pos() returned a negative value. Let's
> clarify that cache_name_pos() implicitly looks for stage 0, while we are
> also okay with finding other stages.
>
> Signed-off-by: Johannes Schindelin <[email-masked]>
> ---
>  merge-recursive.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/merge-recursive.c b/merge-recursive.c
> index 98f4632..bcb53f0 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -658,6 +658,7 @@ static int was_tracked(const char *path)
>  {
>  	int pos = cache_name_pos(path, strlen(path));
>  
> +	/* cache_name_pos() looks for stage == 0, so pos may be < 0 */

It returns >= if found at stage #0, or a negative (counting from -1)
to indicate where the path would be inserted if it were to be added
at stage #0.

The new comment does not explain how "pos may be < 0" leads to
"hence pos = -1 - pos is the right thing to do here".  It is
misleading and we probably are better off without.

>  	if (pos < 0)
>  		pos = -1 - pos;
>  	while (pos < active_nr &&