Re: [RFC/ PATCH 1/5] tree-walk: do not stop when an error is detected

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

Diane Gasselin <diane.gasselin@ensimag.imag.fr> writes:

> When an error is detected, traverse_trees() is not stopped anymore.
> The whole tree is traversed so that all the merging errors can be detected.

A small worry is if we have some codepath that uses this function like
this:

    if (traverse trees finishes successfully) {
    	be happy, all is well;
    } else {
	attempt a different strategy to achieve
        what we wanted to with traverse trees, if
        it worked fine.
    }

In such a case, spending extra cycles in traverse-trees only to collect
more errors would actively degrade performance in the "alternative
implementation" codepath.  For "try 'quick but limited' version first, and
if it doesn't work, try more elaborate version spending extra cycles"
pattern to work well, the 'quick but limited' version needs to fail
quickly without wasting extra cycles when it hits its limitation.  In the
original code, we deliberately returned early upon seeing the first error
exactly for this reason.

I don't think of concrete examples offhand (fallbacks "merge -s resolve -s
recursive" or "am -3" use come close, perhaps), though, so I may be
worried needlessly in this case.  I honestly don't know offhand.

With our attention focused only on UI issues, I however would agree that
it makes a lot of sense to collect all errors and give them all to the
user, especially because the extra cycles (compared to the current code)
spent to do so is only in the error codepath.