From: Eric Sunshine <sunshine@sunshineco.com>
Date: Thu, 22 Apr 2021 22:33:43 GMT
Subject: Re: [PATCH v3] add git-p4.fallbackEncoding config setting, to prevent git-p4 from crashing on non UTF-8 changeset descriptions
Message-ID: <CAPig+cQUaJq4Bu1NDSBnsQoR2HXhQ+s+4aQHeVP82DM_BuEL8Q@mail.gmail.com>
In-Reply-To: <CAPig+cQE0oHHY89D6fLyymduY3=zSe8y246cz1P2MjTZhrMHNQ@mail.gmail.com>

On Thu, Apr 22, 2021 at 12:17 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
> We normally want to avoid using bare single-quotes inside the body of
> the test since the body itself is a single-quoted string. These
> single-quotes make it harder for a reader to reason about what is
> going on; especially with the $2 in there, one has to spend extra
> cycles wondering if $2 is correctly expanded when the test runs or
> when it is first defined. So, an easier-to-understand rewrite might
> be:
>
>     desc=$(head -1 log | awk ''{print \$2}'') &&

Of course, the quotes surrounding the {print...} should be
double-quotes, not pairs of single-quotes:

    desc=$(head -1 log | awk "{print \$2}") &&

(I didn't notice the problem when originally composing the email since
the compose window wasn't using a fixed-width font, and only noticed
it later when re-reading it in a mail reader which does use
fixed-width. Sorry for any potential confusion.)

> Many existing tests in this project use `cut` for word-plucking, so an
> alternative would be:
>
>     desc=$(head -1 log | cut -d" " -f2) &&

At any rate, using `cut` would be a good option since there's plenty
of precedent in existing test scripts.