Re: [PATCH 10/19] git-submodule.sh: convert test -a/-o to && and ||

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]