Re: [PATCH v2 21/29] grep: factor test for \0 in grep patterns into a function

From: Junio C Hamano <gitster@pobox.com>
Date: May 15, 2017, 06:24 UTC
Message-ID: <xmqqo9uuvdra.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170513231509.7834-22-avarab@gmail.com>

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

Show 8 quoted lines

> Factor the test for \0 in grep patterns into a function. Since commit
> 9eceddeec6 ("Use kwset in grep", 2011-08-21) any pattern containing a
> \0 is considered fixed as regcomp() can't handle it.
>
> This limitation was never documented, and other some regular
> expression engines are capable of compiling a pattern containing a
> \0. Factoring this out makes a subsequent change which does that
> smaller.

The latter half of the first sentence of this paragraph does not parse well, around "and other some". "never documented" makes readers expect "... so let's document it", but I think you are driving in a different direction, something like...

        Since some other regular expression engines are capable of a
        pattern to match a string with NUL in it, it would be nice
        if we can use such an engine and match against NUL; as this
        "patterns with NUL always use --fixed match" is not documented,
	let's hope that nobody depend on that current behaviour.
        This step does not yet change the behaviour yet, but it
        makes it easier to do so in later steps.

perhaps? I tend to agree that it is OK to break users who depended on that "patterns with NULL match with --fixed" (partly because it is so unintuitive and not useful behaviour, and partly because it is easy for them to explicitly say -F), but let's make sure we clearly say that we will be knowingly breaking them.

Thanks.