Re: [PATCH v2 1/2] t: don't spuriously close and reopen quotes

[Eric Sunshine <sunshine@sunshineco.com>]

On Fri, Aug 7, 2020 at 4:45 AM Martin Ågren <martin.agren@gmail.com> wrote:
> On Thu, 6 Aug 2020 at 22:26, Eric Sunshine <sunshine@sunshineco.com> wrote:
> > On Thu, Aug 6, 2020 at 4:09 PM Martin Ågren <martin.agren@gmail.com> wrote:
> > > -    echo "$fifth      branch 'fifth' of ." |
> > > +    echo "$fifth      branch fifth of ." |
> >
> > This one is a bit weird. It really seems as if the intent was to quote
> > the word "fifth" in the merge message, so dropping the quotes
> > altogether seems wrong. However, the file 'msg' is never even
> > consulted in this test (or any other test), so is this just "dead
> > code" (including the leading 'fifth=' assignment which also is
> > otherwise unused outside the 'echo')?
>
> Huh, good catch. [...] So I should be able to safely drop this
> "dead code" entirely.

That could be done atop this series if there is no other reason to
re-roll.

> > > -    git tag -a -m 'annotated' anno HEAD &&
> > > +    git tag -a -m "annotated" anno HEAD &&
> >
> > There are a fair number of these quoted single-token arguments
> > containing no special characters which don't actually need to be
> > quoted at all, so an alternative would be simply to drop the quotes
> > altogether, making the commands less syntactically noisy. However,
> > that might be outside the intended scope of this change.
>
> If we say that "don't use quotes if you don't need to" is a reasonable
> thing to strive for, I can drop these in a reroll. I think I'll be
> injecting a patch anyway for the "msg" you pointed out in t4200, so I
> can certainly tweak this patch to be a bit more aggressive in dropping
> unnecessary quotes.

No need. Matching the local convention makes sense, and I don't insist
upon such a change at all. Mine was a non-actionable observation, and
it's entirely subjective anyhow.

I don't feel strongly about whether the series should be re-rolled.
It's true that dropping that dead code (mentioned above) would make more
sense coming before the patch which fixes up the quoting, but it
wouldn't bother me if the dead-code removal was done as a follow-on
patch.