Re: [PATCH v5 2/4] worktree add: refactor opt exclusion tests
- From
Jacob Abel <jacobabel@nullpo.dev>
- Date
- Dec 20, 2022, 23:29 UTC
- Message-ID
- <20221220232920.wppkxkvuxkcoih4x@phi>
- In-Reply-To
- <xmqqsfhawwqf.fsf@gitster.g>
On 22/12/20 01:00PM, Junio C Hamano wrote:
Show 18 quoted lines
> Jacob Abel <jacobabel@nullpo.dev> writes:
>
> > Pull duplicate test code into a function so that additional opt
> > combinations can be tested succinctly.
> >
> > Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
> > ---
> > t/t2400-worktree-add.sh | 20 ++++++++++----------
> > 1 file changed, 10 insertions(+), 10 deletions(-)
>
> OK.
>
> > +# Helper function to test mutually exclusive options.
> > +test_wt_add_excl() {
>
> Have SP on both sides of (), i.e.
>
> test_wt_add_excl () {Done.
Show 13 quoted lines
> > > + local opts="$@" && > > + test_expect_success "'worktree add' with '$opts' has mutually exclusive options" ' > > + test_must_fail git worktree add $opts > > + ' > > +} > > In this particular case with the given arguments, it does not make a > difference, but make it a habit to think twice if "$*" is what you > want when you write "$@". "$@" does a lot more than just concatenate > $1, $2, ... with SP in between, and the use of $opts in the message > merely wants the "concatenate with SP" behaviour, which is what "$*" > is for.
Understood.
Show 9 quoted lines
>
> I actually would write the above if I were doing this patch:
>
> test_wt_add_excl () {
> test_expect_success "'worktree add' with $* has mutually exclusive options" '
> test_must_fail git worktree add "$@"
> '
> }
>Changed.
Show 33 quoted lines
> Notice the use of "$@" on the "git worktree add" invocation? This
> allows callers of test_wt_add_excl pass parameters with SP in it,
> thanks to the magic "$@". Again, as I said earlier, for these
> callers ...
>
> > +test_wt_add_excl -b poodle -B poodle bamboo main
> > +test_wt_add_excl -b poodle --detach bamboo main
> > +test_wt_add_excl -B poodle --detach bamboo main
>
> ... the distinction does not matter, but
>
> helper --lock --reason "my reason with multiple words" bamboo main
>
> must be written with "$@", like so:
>
> helper () {
> git worktree add "$@"
> }
>
> not
>
> helper () { # BAD
> local opt="$@"
> git worktree add $opt
> }
>
> $opt in this case is a SP-concatenated string
>
> opt="--lock --reason my reason with multiple words bamboo main"
>
> and passing it without quotes around it to "git worktree add"
> will give only "my" as the parameter to the option "--reason",
> with three extra words on the comand line.This makes sense. I'm not the best with shell scripting so I appreciate the explanation.