From: Johannes Sixt <j.sixt@viscovery.net>
Date: Tue, 20 May 2014 14:39:50 GMT
Subject: Re: [PATCH 10/19] git-submodule.sh: convert test -a/-o to && and ||
Message-ID: <537B6936.2080101@viscovery.net>
In-Reply-To: <1400593832-6510-11-git-send-email-gitter.spiros@gmail.com>

Am 5/20/2014 15:50, schrieb Elia Pinto:
>  			# If we don't already have a -f flag and the submodule has never been checked out
> -			if test -z "$subsha1" -a -z "$force"
> +			if test -z "$subsha1" || test -z "$force"

Should not be ||, but &&!

>  		while read mod_src mod_dst sha1_src sha1_dst status sm_path
>  		do
>  			# Always show modules deleted or type-changed (blob<->module)
> -			test $status = D -o $status = T && echo "$sm_path" && continue
> +			{
> +			test "$status" = D ||
> +			test "$status" = T
> +			} &&
> +			echo "$sm_path"
> +			&& continue

As Matthieu noted, this is incorrect. It's not just a style violation,
it's a syntax error. Why did your test runs not hickup on that?

In this case you could even leave the original code structure without
changing the meaning:

			test $status = D || test $status = T && echo "$sm_path" && continue

But a better idiom is
			case "$status" in
			[DT])
				printf '%s\n' "$sm_path" &&
				continue
			esac

> @@ -1233,7 +1238,7 @@ cmd_status()
>  			say "U$sha1 $displaypath"
>  			continue
>  		fi
> -		if test -z "$url" || ! test -d "$sm_path"/.git -o -f "$sm_path"/.git
> +		if test -z "$url" || ! test -d "$sm_path"/.git || test -f "$sm_path"/.git

Wrong grouping. This could be more correct (I didn't test):

		if test -z "$url" ||
			{
				! test -d "$sm_path"/.git &&
				! test -f "$sm_path"/.git
			}

-- Hannes