Re: [PATCH] git-prompt: use builtin test

[Jeff King <peff@peff.net>]

On Sun, Jun 19, 2022 at 01:26:08AM +0000, Brad Forschinger wrote:

> > But at some point we may say "you have made the environment too hostile
> > for us to function". Is redefining "test" to something that doesn't
> > behave the same way such a case? Part of me wants to say yes. :)
> 
> I'd be inclined to agree!  But disregarding a user with malicious
> intent, these environment changes can also be unintentional: I came
> across it when I stubbed out a quick test() function while prototyping
> something unrelated.

I kind of wonder what else would have trouble with your accidental
breakage. I poked at the bash-completion package as the only other
prominent "source this into your bash script" package I could think of.
They do indeed seem to mostly avoid "test", though there are a few cases
in program-specific completions.

In the general case, though, I think it's an infinite rabbit hole. I can
similarly redefine "declare" or "local" and cause all sorts of trouble.
And there's no real defense for scripts there.

> >   - my biggest concern on cost is that this is an unusual style for our
> >     project (which usually writes in POSIX shell, though of course this
> >     file is meant to be bash/zsh specific). Will it be a maintenance
> >     burden going forward?
> 
> That's possible, but I suspect the burden is minimal.  As you said, this
> is bash and zsh specific, and for those shell coders who only write
> Bourne dialect it's to be read as a "strong" left square bracket.  For
> example, to minimize any shock to the eyeballs I've intentionally not
> re-written string operations `[ a = b ] && [ c = d ]` to `[[ a == b && c
> == d ]]`.  I promise it wasn't mere laziness!

I guess my concern was less about doing it once, and more about: is this
something we want to continue enforcing as time goes on? That is, would
we want to catch it in review and complain about people using "test"?
That's a subtle thing to remember to look for, though I guess we could
automate it via the tests. Or would we rely on people who cared to
notice new instances and submit patches? That's how we deal with some
other portability issues (if nobody is screaming, how broken could it
be?). But it sounds from your description like this was a one-off even
for you.

So I dunno. I'm not really opposed, but I'm not convinced it's really
accomplishing much here.

-Peff