From: Eric Sunshine <sunshine@sunshineco.com>
Date: Wed, 17 Feb 2021 23:22:17 GMT
Subject: Re: [RFC PATCH 3/7] t3705: add tests for `git add` in sparse checkouts
Message-ID: <CAPig+cT_ASSSe0jzEWiW_EPEa5FfHigfM+6fGQyG_kwj57XPfQ@mail.gmail.com>
In-Reply-To: <xmqqpn0yi9ii.fsf@gitster.g>

On Wed, Feb 17, 2021 at 6:04 PM Junio C Hamano <gitster@pobox.com> wrote:
> Matheus Tavares <matheus.bernardino@usp.br> writes:
> > +for opt in "" -f -u --ignore-removal
> > +do
> > +     if test -n "$opt"
> > +     then
> > +             opt=" $opt"
> > +     fi
> > +     test_expect_success "git add$opt does not update SKIP_WORKTREE entries" '
>
> The above is cumulative, and as a consequence, "git add -u <path>"
> is not tested, but "git add -f -u <path>" is.  Intended?  How was
> the order of the options listed in "for opt in ..." chosen?

I may be misreading, but I don't think this is cumulative (though it's
easy to mistake it as such due to the way it inserts a space before
$opt). My interpretation is that `opt` gets overwritten with a new
value on each iteration, and it is inserting the space merely to make
the test title print nicely. A more idiomatic way to do this would
have been:

    for opt in "" -f -u --ignore-removal
    do
        test_expect_success " git add${opt:+ $opt} does ..." '