Re: [PATCH v2 1/5] t5505: remove sub shell use in favor of git -C
- From
Jacob Keller <jacob.keller@gmail.com>
- Date
- Jun 16, 2022, 21:10 UTC
- Message-ID
- <CA+P7+xp95ndNeFY_5icL1=xaBy+ikkT8UhgAXQhoPNfZqo0cWQ@mail.gmail.com>
- In-Reply-To
- <xmqq8rpws4ck.fsf@gitster.g>
On Thu, Jun 16, 2022 at 2:09 PM Junio C Hamano <gitster@pobox.com> wrote:
Show 41 quoted lines
>
> 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?
>Yea that might be an improvement. They were previously only used in "test" but I like the idea to clean that up.
Show 34 quoted lines
> > @@ -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).
>I think these ones can be fixed up by supporting -C in the function instead so that it will be clear.