Re: [PATCH 0/4] Re: [PATCH 3/4] t: rev-parse-parents: avoid yoda conditions

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 4, 2013, 19:14 UTC
Message-ID: <xmqqob88mmax.fsf@gitster.dls.corp.google.com>
In-Reply-To: <20130904183559.GA3465@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 16 quoted lines

> On Wed, Sep 04, 2013 at 10:38:03AM -0700, Junio C Hamano wrote:
>
>> >> This is way off tangent, but I am somewhat sympathetic to Felipe's
>> >> "compare actual with expect", with reservations.
>> >
>> > This isn't an argument either way, but note that JUnit (and NUnit and
>> > PHPUnit) all have assertEquals methods that take the arguments in the
>> > order "expect, actual".  I've always assumed that Git's test framework
>> > was imitating that,...
>> 
>> No.  See 82ebb0b6 (add test_cmp function for test scripts,
>> 2008-03-12).  The "test_cmp" was a replacement for "diff -u", and
>> the same order we fed "diff -u", i.e. expect then actual, was
>> carried over.
>
> I don't think it was intentional at the time.

I do not think so, either. The only thing we cared about was "are they equal". And from the point of view of test_cmp exit status, that still the only thing we care about. Comparison to see which is greater is a superset of equality check we needed, and in that context, the order did not and does not matter.

One only starts to notice the order of comparison when one starts thinking about the comparison in terms of "what is subtracted from what", and at that point, one realizes that "diff A B" that gives us what was lost from A to B as "-" and what was gained in B relative to A as "+" is showing the result of subtracting A from B. And that subtraction is backwards from the cmp(A,B) that subtracts B from A, which is the usual convention.

> Though I prefer the current, I can certainly live and adapt to a changed
> standard, and I do not mind doing so if there is a good reason. But I've
> yet to see any argument beyond "it is not what I like". Which to me
> argues for the status quo as the path of least resistance.

As I think test_cmp is primarily about equality comparison, I do not think it is worth switching and adapting. Switching makes sense only in my dreams where we did not have to worry about in-flight topics and people's brains that are used to the current order.

That is exactly where my "with reservations" came from.