On Mon, Jul 29 2019, Carlo Arenas wrote:
> On Fri, Jul 26, 2019 at 8:09 AM Ævar Arnfjörð Bjarmason
> <avarab@xxxxxxxxx> wrote:
>>
>> It's less confusing to use that variable consistently that switch back
>> & forth between the two.
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
>> ---
>> grep.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/grep.c b/grep.c
>> index 9c2b259771..b94e998680 100644
>> --- a/grep.c
>> +++ b/grep.c
>> @@ -616,7 +616,7 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)
>> die(_("given pattern contains NULL byte (via -f <file>). This is only supported with -P under PCRE v2"));
>>
>> pat_is_fixed = is_fixed(p->pattern, p->patternlen);
>> - if (opt->fixed || pat_is_fixed) {
>> + if (p->fixed || pat_is_fixed) {
>
> at the end of this series we have:
>
> if (p->fixed || p->is_fixed)
>
> which doesn't make sense; at least with opt->fixed it was clear that
> what was meant is that grep was passed -P
I assume you mean "was passed -F...".
> maybe is_fixed shouldn't exist and fixed when applied to the pattern
> means we had determined it was a fixed
> pattern and overridden the user selection of engine.
They're two flags because p->fixed is "--fixed-strings", and p->is_fixed
is "there's no metachars here". So the former case needs escaping, as
the code just below might do (the two aren't mutually exclusive).
I don't get how you think we can always fold them into one flag, but
maybe I'm missing something...
> that at least will give us a logical way to fix the pattern reported
> in [1] and that currently requires the user to know
> git's grep internals and know he can skip the "is_fixed" optimization
> by doing something like :
>
> $ git grep 'foo[ ]bar'
>
> [1] https://public-inbox.org/git/20190728235427.41425-1-carenas@xxxxxxxxx/
As I noted in a reply there this seems like a way to fix a bug in "next"
with a config knob. Yes we should fix the bug, but we've had the kwset
code in git for years without needing this distinction, so after we work
out the bugs I don't see why we'd need this.
The reason we ignore the user's choice here is because you might
e.g. set grep.patternType=extended in your config, and you'd still want
grepping for a fixed "foo" to be fast.
