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

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 12, 2019, 06:52 UTC
Message-ID: <xmqq5zjpfux2.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <3c58df27c4b9f405bfc5c163e36a928a75ce8239.1573517561.git.liu.denton@gmail.com>

Denton Liu <liu.denton@gmail.com> writes:

Show 11 quoted lines

> +# 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
> +	op='='
> +	if test $# -ge 1 && test "x$1" = 'x!'
> +	then
> +	    op='!='
> +	    shift
> +	fi

Not that it is all that important, but how about setting up comp_out up here at the same time? I.e.

	local op comp_out

	if ...
	then
		op="!=" comp_out="the same"
		shift
	else
		op="=" comp_out=different
	fi

That way, we can lose 7 lines down there, while spending only one line up here, collecting all the things that conditional on '!' on the command line in one place.

I would have named the message variable "$wrong_result" or something, by the way, i.e. "two revisions point to $wrong_result objects".

Thanks.  Will queue the whole thing.

Show 25 quoted lines

>  	if test $# != 2
>  	then
>  		error "bug in the test script: test_cmp_rev requires two revisions, but got $#"
>  	else
>  		local r1 r2
>  		r1=$(git rev-parse --verify "$1") &&
> -		r2=$(git rev-parse --verify "$2") &&
> -		if test "$r1" != "$r2"
> +		r2=$(git rev-parse --verify "$2") || return 1
> +
> +		if ! test "$r1" "$op" "$r2"
>  		then
> +			local comp_out
> +			if "x$op" = 'x='
> +			then
> +				comp_out='different'
> +			else
> +				comp_out='the same'
> +			fi
>  			cat >&4 <<-EOF
> -			error: two revisions point to different objects:
> +			error: two revisions point to $comp_out objects:
>  			  '$1': $r1
>  			  '$2': $r2
>  			EOF