Re: [PATCH v6 2/3] am: support --empty option to handle empty patches

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Nov 19, 2021, 12:20 UTC
Message-ID: <CAPig+cQq3Ek7RL9NKuvR5V9OVULRf7=N3QEYvy9=xqdZB3EEWg@mail.gmail.com>
In-Reply-To: <211119.861r3c4a4g.gmgdl@evledraar.gmail.com>

On Fri, Nov 19, 2021 at 7:12 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 16 quoted lines

> On Fri, Nov 19 2021, Bagas Sanjaya wrote:
> > On 18/11/21 17.50, Aleen via GitGitGadget wrote:
> >> +test_expect_success 'still output error with --empty when meeting empty files' '
> >> +    test_must_fail git am --empty=drop empty.patch 2>actual &&
> >> +    echo Patch format detection failed. >expected &&
> >> +    test_cmp expected actual
> >> +'
> >
> > Why isn't the echo string quoted?
>
> There's no need to quote the arguments given to echo in cases like
> these.
>
> It's not the same, i.e. it'll get N arguments on argv and not on, but it
> makes it easier to spot things that do need quoting, rather than
> over-quoting everything.

I recently expressed an opposing opinion in [1], stating effectively that omitting the quotes like this is "an accident waiting to happen":

    ... the lack of quotes ... in the `echo ... >expect` statement
    gives me a moment's pause since it relies upon the fact that
    `echo` will insert exactly one space between the ... arguments
    (which happens to match the single space in the [command's output]
    ). For clarity and that extra bit of robustness, I'd probably have
    used a single double-quoted string argument with `echo`.

But, it's a fairly minor objection.

[1]: https://lore.kernel.org/git/CAPig+cQVSUg1aqry_hMydJ=Uo=-VhOog6TUTpG=0on0LUcw8Dg@mail.gmail.com/