Re: [PATCH 2/2] t2200: modify code syntax

[Amanda Shafack <shafack.likhene@gmail.com>]

I agree with you. I have made these changes as well.
Thanks


On Sun, Oct 18, 2020 at 6:56 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
>
> On Sat, Oct 17, 2020 at 12:02 PM Amanda Shafack via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
> > Using the logical not operator on both the git and grep command is
> > redundant, since the main check is done at the level of the grep
> > command.
> >
> > Apply the logical not operator only to the grep command.
> > ---
> > diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh
> > @@ -179,10 +179,8 @@ test_expect_success 'add -u resolves unmerged paths' '
> > -       ! (
> > -               git ls-files >actual &&
> > -               grep "non-existent" actual
> > -       )
> > +       git ls-files >actual &&
> > +       ! grep "non-existent" actual
>
> The commit message talks only about redundancy of applying the logical
> NOT operator to the combined subshell content, but what it misses is
> that transformation of:
>
>     ! (git ls-files | grep "non-existent")
>
> in patch [1/2] into:
>
>     ! (
>         git ls-files >actual &&
>         grep "non-existent" actual
>     )
>
> is actively wrong. In particular, if `git ls-files` itself fails, then
> the `grep` will never get run, and the subshell will exit with a
> failure, however, the NOT then turns that failure into a success,
> which is quite undesirable. The test should fail if either `git
> ls-files` fails or if the `grep` expectation is not met; it should not
> succeed when `git ls-files` fails.
>
> So, the correct thing to do is to merge [1/2] and [2/2] into a single
> patch which directly transforms:
>
>     ! (git ls-files | grep "non-existent")
>
> into:
>
>     git ls-files >actual &&
>     ! grep "non-existent" actual



-- 

Cheers!

Amanda  Shafack