Re: [PATCH v6 01/14] t: teach test_cmp_rev to accept ! for not-equals

[Junio C Hamano <gitster@xxxxxxxxx>]

Denton Liu <liu.denton@xxxxxxxxx> writes:

> +# Tests that its two parameters refer to the same revision, or if '!' is
> +# provided first, that its other two parameters refer to different
> +# revisions.
>  test_cmp_rev () {
> +	local op wrong_result
> +	op='='
> +	wrong_result='different'
> +	if test $# -ge 1 && test "x$1" = 'x!'
> +	then
> +	    op='!='
> +	    wrong_result='the same'
> +	    shift
> +	fi

I'd prefer

	local op wrong_result

	if test $# -ge 1 && test "x$1" = 'x!'
	then
	    op='!='
	    wrong_result='the same'
	    shift
	else
	    op='='
	    wrong_result='different'
	fi

that clarifies that the variants with and without '!' are equals, as
opposed to the form with '!' is an afterthought exception.

On the other hand, if we want to insist that the form without '!' is
the norm, then

	local op='=' wrong_result='different'

	if test $# -ge 1 && test "x$1" = 'x!'
	then
	    op='!='
	    wrong_result='the same'
	    shift
	fi

would be shorter (yes, I made sure we already use assignment to a
variable on the same line where it is declared "local"; we used to
avoid "local" that is outside POSIX so I want to make sure our use
is safe).

I'll queue the patches as-is, but if enough people agree with me (on
either variants), I'd locally amend to make it so (i.e. no need to
resend only for this change).

Thanks.