Subject: Re: [PATCH v2 1/7] t0027: Add tests for get_stream_filter()

From: Junio C Hamano <[email-masked]>
Date: Thu, 04 Feb 2016 11:52:06 -0800

[email-masked] writes:

> +		while test "$#" != 0
> +		do
> +			case "$1" in
> +				auto)	 echo '*.txt text=auto' ;;
> +				i)     echo '* ident' ;;
> +				text)	 echo '*.txt text' ;;
> +				-text) echo '*.txt -text' ;;
> +				crlf)  echo '*.txt eol=crlf' ;;
> +				lf)    echo '*.txt eol=lf' ;;
> +				"") echo '' ;;
> +				*)
> +				echo >&2 invalid attribute: $attr
> +				exit 1
> +				;;

Perhaps use the right style as you are rewriting pretty much
everything in this helper function?  case/esac aligns with the
labels of case arms, and commands under each arm are indented one
level down, i.e.

	case "$var" in
        $arm1)	... do short thing ...;;
        $arm2)
        	... do a lot of
                ... things
                ;;
	esac

I still do not see the value of accepting a cryptic 'i' (or an empty
string) to this thing, though.  Also what $attr does the error
message refer to now?  Perhaps it wants to refer to "$1" (quoted)
instead?