Re: [PATCH try2 0/4] completion: bash: a bunch of fixes

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 23, 2020, 20:25 UTC
Message-ID: <xmqq1rfg8e3j.fsf@gitster.c.googlers.com>
In-Reply-To: <20201223141950.GA23264@szeder.dev>

SZEDER Gábor <szeder.dev@gmail.com> writes:

Show 17 quoted lines

> On Wed, Dec 23, 2020 at 07:38:12AM -0600, Felipe Contreras wrote:
> ...
>> I see 5 courses of action:
>> 
>>  1. Drop the offending patch: this is wrong because the bug is still
>>     there, we are just not checking for it.
>>  2. Add a BASH prereq just for that test, or test_expect_unstable (we
>>     would need to add extra code for both of those).
>>  3. Add the fix, but not the test for the fix.
>
> I'm for this option 3: this patch does fix a bug for users of Bash
> v4.0 or later, while it doesn't change the behavior with v3.2 or
> earlier (and with zsh, if I understand correctly).  OTOH, the test
> doesn't seem to be all that useful: while it does demonstrate the
> issue, it checks only one of those callsites that passed the wrong
> suffix, and, more importantly, it doesn't protect us from adding
> another callsites with similarly wrong suffex in the future.

Yeah, I might have preferred, if we didn't read your "doesn't seem to be all that useful", to keep the test with prereq on bash 4, but I think either way is fine.

> In any case, the commit message should note that the fix doesn't work
> with all Bash versions and why.

Very true.

Thanks, all.