Re: [PATCH 2/2] merge: avoid searching for strategies with fewer than 0 conflicts

[Junio C Hamano <gitster@pobox.com>]

Elijah Newren <newren@gmail.com> writes:

> No, the original code would not have taken 'ort'.  You have overlooked
> the part of the code immediately above these two lines:
>
>     if (ret < 2) {
>         if (!ret) {
>             if (option_commit) {
>                 /* Automerge succeeded. */
>                 automerge_was_ok = 1;
>                 break;
>             }
>         merge_was_ok = 1;
>         }
>
> In particular, the "break" is key.

I noticed that much but then would "git merge --no-commit -s A -s B"
have the issue?

> ...  In contrast, if the user did specify
> --no-commit and the previous strategy succeeded, then we will continue
> the loop.  That seems rather inconsistent, since --no-commit should
> not affect the choice of strategy.

Yeah, exactly.

> However, I missed two things in my reading.  You are correct that I
> missed the "<=" as opposed to "<" when I wrote my commit message,
> though that turns out to not matter much due to the second thing.  The
> second thing I missed was part of the code at the beginning of the
> loop:
>
>     for (i = 0; !merge_was_ok && i < use_strategies_nr; i++) {

Ahhhh, that explains it.  We leave as soon as we find merge_was_ok
so this patch is not necessary.  There is nothing to fix.  The
original was fine as-is.

Thanks.