Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
> I agree that the comment is not very good currently. But I disagree that
> we are better off without any comment here.
I meant we are better off without your particular version of comment
which is misleading. I am all for a better comment to help those
who are new to the codepath.
> I would like to propose this diff instead (it is larger, but with a net
> savings of one line):
>
> -- snipsnap --
> diff --git a/merge-recursive.c b/merge-recursive.c
> index d5a593c..0eda51a 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -658,24 +658,22 @@ static int was_tracked(const char *path)
> {
> int pos = cache_name_pos(path, strlen(path));
>
> - if (pos < 0)
> - pos = -1 - pos;
> - while (pos < active_nr &&
> - !strcmp(path, active_cache[pos]->name)) {
> + if (pos >= 0)
> + return pos < active_nr;
> + /*
> + * cache_name_pos() looks for stage == 0, even if we did not ask for
> + * it. Let's look for stage == 2 now.
> + */
I think this keeps the same phrasing from the original that makes
the comment misleading. It "looks for stage == 0" is not the whole
story but only half. It looks for a place to insert the path at
stage #0" is. Your half is used by the "if (0 <= pos)" you split
out into a separate statement above already, and the untold half is
needed to explain why this loop is correct.
It returns the place to insert stage #0 entry, so if you are looking
for stage #1 or higher, you only have to loop while the path
matches, because the entries are sorted by <path, stage>.
And with that understanding, there is no strong reason to special
case "ah, we found stage #0 entry" at all.
> + for (pos = -1 - pos; pos < active_nr &&
> + !strcmp(path, active_cache[pos]->name); pos++)
> /*
> * If stage #0, it is definitely tracked.
> * If it has stage #2 then it was tracked
> * before this merge started. All other
> * cases the path was not tracked.
> */
> - switch (ce_stage(active_cache[pos])) {
> - case 0:
> - case 2:
> + if (ce_stage(active_cache[pos]) == 2)
> return 1;
> - }
> - pos++;
> - }
> return 0;
> }
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html