Re: [PATCH v2 1/5] t5505: remove sub shell use in favor of git -C

[Junio C Hamano <gitster@pobox.com>]

Jacob Keller <jacob.keller@gmail.com> writes:

>  check_remote_track () {
> -	actual=$(git remote show "$1" | sed -ne 's|^    \(.*\) tracked$|\1|p')
> +	actual=$(git -C test remote show "$1" | sed -ne 's|^    \(.*\) tracked$|\1|p')

This has become less generic.

>  	shift &&
>  	tokens_match "$*" "$actual"
>  }
>  
>  check_tracking_branch () {
>  	f="" &&
> -	r=$(git for-each-ref "--format=%(refname)" |
> +	r=$(git -C test for-each-ref "--format=%(refname)" |
>  		sed -ne "s|^refs/remotes/$1/||p") &&

Likewise.

They used to be usable in any test repository, but now they have to
be used in a repository whose name is "test".

I wonder if something like

	check_foo () {
		if test "$1" = -C
		then
			in_repo=$2
			shift 2
		else
			in_repo=
		fi &&
		actual=$(git ${in_repo:+-C "$in_repo"} subcmd ...) &&
		do things ... &&
		do more things
	}

would work better?  

> @@ -44,10 +44,7 @@ check_tracking_branch () {
>  test_expect_success setup '
>  	setup_repository one &&
>  	setup_repository two &&
> -	(
> -		cd two &&
> -		git branch another
> -	) &&
> +	git -C two branch another &&
>  	git clone one test
>  '

Clear improvement.

> @@ -57,25 +54,19 @@ test_expect_success 'add remote whose URL agrees with url.<...>.insteadOf' '
>  '
>  
>  test_expect_success 'remote information for the origin' '
> -	(
> -		cd test &&
> -		tokens_match origin "$(git remote)" &&
> -		check_remote_track origin main side &&
> -		check_tracking_branch origin HEAD main side
> -	)
> +	tokens_match origin "$(git -C test remote)" &&

This is a clear improvement.

> +	check_remote_track origin main side &&
> +	check_tracking_branch origin HEAD main side

These two are a bit questionable (it hides the fact that all these
names refer to stuff in "test" subdirectory).