Re: [PATCH 04/15] t0003: use named parameters in attr_check()

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 17, 2019, 18:35 UTC
Message-ID: <xmqqsglihkab.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <f1acb2a0dfb39a4ff047d721edde821e9e296e72.1576583819.git.liu.denton@gmail.com>

Denton Liu <liu.denton@gmail.com> writes:

Show 7 quoted lines

> We had named the parameters in attr_check() but $2 was being used
> instead of $expect. Make all variable accesses in attr_check() use named
> variables instead of numbered arguments for clarity.
>
> While we're at it, add variable assignments to the &&-chain. These
> aren't ever expected to fail but for stylistic purposes, include them
> anyway for stylistic purposes.

That is OK but do so like this instead for brevity?

 attr_check () {
- 	path="$1" expect="$2"
+ 	path="$1" expect="$2" git_opts="$3" &&

Giving (temporary) names to incoming parameters is so common that such a convention to save vertical screen real estate may be worth it.

It is sad that you can pass only $IFS-free things in $3 due to the way it is used---perhaps it is sufficient for the purpose of tests in this script, but it still looks sad (just a comment without anything that is actionable).

Show 37 quoted lines

> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
>  t/t0003-attributes.sh | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
> index 71e63d8b50..c47d4cfbcd 100755
> --- a/t/t0003-attributes.sh
> +++ b/t/t0003-attributes.sh
> @@ -5,19 +5,20 @@ test_description=gitattributes
>  . ./test-lib.sh
>  
>  attr_check () {
> -	path="$1" expect="$2"
> +	path="$1" &&
> +	expect="$2" &&
> +	git_opts="$3" &&
>  
> -	git $3 check-attr test -- "$path" >actual 2>err &&
> -	echo "$path: test: $2" >expect &&
> +	git $git_opts check-attr test -- "$path" >actual 2>err &&
> +	echo "$path: test: $expect" >expect &&
>  	test_cmp expect actual &&
>  	test_line_count = 0 err
>  }
>  
>  attr_check_quote () {
> -
> -	path="$1"
> -	quoted_path="$2"
> -	expect="$3"
> +	path="$1" &&
> +	quoted_path="$2" &&
> +	expect="$3" &&
>  
>  	git check-attr test -- "$path" >actual &&
>  	echo "\"$quoted_path\": test: $expect" >expect &&