Re: [PATCH 1/6] t4061: use POSIX compliance regex(7)
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 19, 2020, 22:02 UTC
- Message-ID
- <xmqqwo7gc8aq.fsf@gitster.c.googlers.com>
- In-Reply-To
- <CAPig+cRzHtGu2KfjoO4DaPWzDrJOhEQGkgo2Fm7egUPM9m7Vyw@mail.gmail.com>
Eric Sunshine <sunshine@sunshineco.com> writes:
Show 16 quoted lines
> On Thu, Mar 19, 2020 at 11:53 AM Jeff King <peff@peff.net> wrote: >> On Thu, Mar 19, 2020 at 09:00:02PM +0700, Đoàn Trần Công Danh wrote: >> > Fix it by using literal `+` instead. >> >> This makes sense, I think. It could hurt a sed which is expected ERE and >> needs the "+" escaped, but I think such a sed would be wrong (and I >> imagine would break things elsewhere). > > I had the same thought and considered suggesting a character class: > > sed -n -e "1,4d" -e "s/^[+]//p" <"$1" >.tmp-1 > > to make it painfully obvious that "+" is not special in the > expression. But then I thought better of it -- for the same reason as > you (to wit: such a 'sed' would be wrong) -- and decided against > saying anything.
I have only one thing that needs fixing, which is s/compliance/compliant/; on the title. Other than that, it looks good.
Having said that, I would have done the [+] thing if I were doing this patch myself. As long as we see no "wrong" sed that is broken by this change, I am OK with it, though.
Thanks.