Junio C Hamano wrote: > SZEDER Gábor <szeder.dev@xxxxxxxxx> writes: > > > 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. Even if we add a prereq on BASH4, he is right that the test wouldn't be all that useful, because it's checking only for one conditional branch, and the function has quite a few, from the top of my head there are about 10. A more useuful test would at least add one check for each one of the cases. It still would be dependent on the current implementation, but would be more useful. I can add that in a future patch series, once the other issues are resolved. Cheers. -- Felipe Contreras
