Re: [PATCH 1/1] convert.c: correct attr_action

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 22, 2016, 08:04 UTC
Message-ID: <xmqqd1rp893e.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1456117898-30357-1-git-send-email-tboegi@web.de>

tboegi@web.de writes:

Show 11 quoted lines

> diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
> index fc4c628..f9c92c6 100755
> --- a/t/t0027-auto-crlf.sh
> +++ b/t/t0027-auto-crlf.sh
> @@ -159,6 +159,39 @@ stats_ascii () {
>  
>  }
>  
> +attr_ascii () {
> +	case "$1" in
> +	"-text") echo "-text"	;;

 - I do not think you need to quote the case label.

 - Why the excessive trailing whitespaces before double-semicolons?

Show 26 quoted lines

> +	lf)	echo "text eol=lf" ;;
> +	crlf)	echo "text eol=crlf" ;;
> +	text)
> +		case "$2" in
> +		"")	echo "text" ;;
> +		lf)	echo "text eol=lf" ;;
> +		crlf)	echo "text eol=crlf" ;;
> +		*) echo invalid_attr2 "$2" ;;
> +		esac
> +		;;
> +	auto)
> +		case "$2" in
> +		"")	echo "text=auto" ;;
> +		lf)	echo "text=auto eol=lf" ;;
> +		crlf)	echo "text=auto eol=crlf" ;;
> +		*) echo invalid_attr2 "$2"	;;
> +		esac
> +		;;
> +	"")
> +		case "$2" in
> +		"")	echo "" ;;
> +		lf)	echo "text eol=lf" ;;
> +		crlf)	echo "text eol=crlf" ;;
> +		*) echo invalid_attr2 "$2" ;;
> +		esac
> +		;;

I wonder if the above is easier to read if written like this:

	case "$1,$2" in
        "-text,*")		echo "-text" ;;
        lf,*)   	        echo text eol=lf" ;;
	...
	auto,)			echo "text=auto" ;;
	auto,lf | auto,crlf)	echo "text=auto eol=$2" ;;
	auto,*)			echo invalid_attr2 "$2" ;;
	...
        esac

as $1 and $2 are not arbitrary words, but you know exactly from what vocabulary they are taken.