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

Jonathan Nieder <jrnieder@gmail.com> writes:

This is way off tangent, but I am somewhat sympathetic to Felipe's "compare actual with expect", with reservations.

If one ignores all other constraints and focuses solely on the order of argument test_cmp takes, one would realize that it is backwards. A(ny) sanely defined "compare A with B" function should yield the result of subtracting B from A, i.e. cmp(A,B) should be like (A-B). That is what you feed qsort() and bsearch() (it is not limited to C; you see the same in "sort { $a <=> $b }"). The definition naturally makes "cmp(A,B) < 0" like "A < B" and "cmp(A,B) > 0" like "A > B".

But unfortunately, test_cmp is defined in terms of "diff -u" by feeding its parameters in the same order as given. "test_cmp A B" just turns into "diff -u A B".

Now, we _do_ want to see "diff -u expect actual", so that in the output, what is _missing_ in the actual output compared to the expected output is marked with "-", and what is _excess_ is marked with "+". If you think about it, this output from "diff -u expect actual" is giving us the result of subtracting expect from actual.

If we want to express it in terms of "cmp", we should be writing "cmp(actual,expect)", not "cmp(expect,actual)". The latter is to subtract actual from expect, which is backwards.

It would have been better if a wrapper around "diff" to give us a higher level "comparison" semantics, test_cmp, had been written in such a way that hid this backward behaviour of "diff", when it was introduced to replace hardcoded "diff -u".

I'd actually be ecstatic if one morning when I get up, I find that test_cmp implementation were replaced with (the moral equivalent of)

	test_cmp () {
		diff -u "$2" "$1"
	}

and all the callsites of test_cmp in the test scripts in all the topic branches in flight were also swapped to "test_cmp actual expect" without anybody having to worry about mismerges, merge conflicts and confusing people who are used to the current order for the past 5 years.

But I do not think that is going to happen without a careful planning. We may be able to manage the code somehow (we can drop all the topics in flight immediately after a release, apply the "swap the order" big patch, and rebase all the topics on top), but retraining people would not be instantaneous "flag day" event, and we will see mistakes for a few months after doing so.

Oh, well.