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

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 29, 2016, 18:35 UTC
Message-ID: <xmqq4m8b4zdd.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <dd3e2cf842fd5e11e31914aa55b8b995e8d3d75c.1467199553.git.johannes.schindelin@gmx.de>

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

Show 19 quoted lines

> 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 <johannes.schindelin@gmx.de>
> ---
>  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 &&