Re: [PATCH v3 03/20] t1092: clean up script quoting

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Tue, Mar 16 2021, Derrick Stolee via GitGitGadget wrote:

> From: Derrick Stolee <dstolee@microsoft.com>
>
> This test was introduced in 19a0acc83e4 (t1092: test interesting
> sparse-checkout scenarios, 2021-01-23), but these issues with quoting
> were not noticed until starting this follow-up series. The old mechanism
> would drop quoting such as in

the "but these issues" follows a partial sentence where we haven't
introduces "what issues?".

Perhaps leading with some summary about $@ v.s. $*:

    Fix a bug in the sparse checkout tests of "$@" being conflated with
    "$*". The bug was introduced in 19a0acc83e4 ([...]), but had no
    effect until now because XYZ ...


>    test_all_match git commit -m "touch README.md"
>
> The above happened to work because README.md is a file in the
> repository, so 'git commit -m touch REAMDE.md' would succeed by
> accident.
>
> Other cases included quoting for no good reason, so clean that up now.

Maybe just my taste, per your comment on another series of mine we might
not have the same sense of splitting up commits, but...

I think in this case it's clearer to have these be two commits. We have
3 hunks fixing the bug, and 6 on an unrelated cleanup. It's a lot easier
for eyeballing a fix to be able to glance just at the 3, especially with
something like $@ v.s. $*.

> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
>  t/t1092-sparse-checkout-compatibility.sh | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
> index 8cd3e5a8d227..3725d3997e70 100755
> --- a/t/t1092-sparse-checkout-compatibility.sh
> +++ b/t/t1092-sparse-checkout-compatibility.sh
> @@ -96,20 +96,20 @@ init_repos () {
>  run_on_sparse () {
>  	(
>  		cd sparse-checkout &&
> -		$* >../sparse-checkout-out 2>../sparse-checkout-err
> +		"$@" >../sparse-checkout-out 2>../sparse-checkout-err
>  	)
>  }
>  
>  run_on_all () {
>  	(
>  		cd full-checkout &&
> -		$* >../full-checkout-out 2>../full-checkout-err
> +		"$@" >../full-checkout-out 2>../full-checkout-err
>  	) &&
> -	run_on_sparse $*
> +	run_on_sparse "$@"
>  }
>  
>  test_all_match () {
> -	run_on_all $* &&
> +	run_on_all "$@" &&
>  	test_cmp full-checkout-out sparse-checkout-out &&
>  	test_cmp full-checkout-err sparse-checkout-err
>  }
> @@ -119,7 +119,7 @@ test_expect_success 'status with options' '
>  	test_all_match git status --porcelain=v2 &&
>  	test_all_match git status --porcelain=v2 -z -u &&
>  	test_all_match git status --porcelain=v2 -uno &&
> -	run_on_all "touch README.md" &&
> +	run_on_all touch README.md &&
>  	test_all_match git status --porcelain=v2 &&
>  	test_all_match git status --porcelain=v2 -z -u &&
>  	test_all_match git status --porcelain=v2 -uno &&
> @@ -135,7 +135,7 @@ test_expect_success 'add, commit, checkout' '
>  	write_script edit-contents <<-\EOF &&
>  	echo text >>$1
>  	EOF
> -	run_on_all "../edit-contents README.md" &&
> +	run_on_all ../edit-contents README.md &&
>  
>  	test_all_match git add README.md &&
>  	test_all_match git status --porcelain=v2 &&
> @@ -144,7 +144,7 @@ test_expect_success 'add, commit, checkout' '
>  	test_all_match git checkout HEAD~1 &&
>  	test_all_match git checkout - &&
>  
> -	run_on_all "../edit-contents README.md" &&
> +	run_on_all ../edit-contents README.md &&
>  
>  	test_all_match git add -A &&
>  	test_all_match git status --porcelain=v2 &&
> @@ -153,7 +153,7 @@ test_expect_success 'add, commit, checkout' '
>  	test_all_match git checkout HEAD~1 &&
>  	test_all_match git checkout - &&
>  
> -	run_on_all "../edit-contents deep/newfile" &&
> +	run_on_all ../edit-contents deep/newfile &&
>  
>  	test_all_match git status --porcelain=v2 -uno &&
>  	test_all_match git status --porcelain=v2 &&
> @@ -186,7 +186,7 @@ test_expect_success 'diff --staged' '
>  	write_script edit-contents <<-\EOF &&
>  	echo text >>README.md
>  	EOF
> -	run_on_all "../edit-contents" &&
> +	run_on_all ../edit-contents &&
>  
>  	test_all_match git diff &&
>  	test_all_match git diff --staged &&
> @@ -280,7 +280,7 @@ test_expect_success 'clean' '
>  	echo bogus >>.gitignore &&
>  	run_on_all cp ../.gitignore . &&
>  	test_all_match git add .gitignore &&
> -	test_all_match git commit -m ignore-bogus-files &&
> +	test_all_match git commit -m "ignore bogus files" &&
>  
>  	run_on_sparse mkdir folder1 &&
>  	run_on_all touch folder1/bogus &&