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

From: Junio C Hamano <[email-masked]>
Date: Mon, 22 Feb 2016 00:04:05 -0800

[email-masked] writes:

> 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?

> +	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.