Re: [PATCH 05/11] grep/icase: avoid kwsset when -F is specified

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 23, 2016, 20:25 UTC
Message-ID: <xmqqtwgjhcub.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160623162907.23295-6-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

Show 13 quoted lines

> @@ -408,7 +427,11 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)
>  	ascii_only     = !has_non_ascii(p->pattern);
>  
>  	if (opt->fixed) {
> -		p->fixed = 1;
> +		p->fixed = !icase || ascii_only;
> +		if (!p->fixed) {
> +			compile_fixed_regexp(p, opt);
> +			return;
> +		}
>  	} else if ((!icase || ascii_only) &&
>  		   is_fixed(p->pattern, p->patternlen))
>  		p->fixed = 1;

Makes me feel somewhat dirty to see that we do the same "!icase || ascii_only" on both sides of the if/else cascade.

I wonder if it would be more readable to structure it like this instead?

	if (opt->fixed || is_fixed(...))
		p->fixed = (!icase || ascii_only);
	else
        	p->fixed = 0;

	if (p->fixed)
        	/* do the kws thing and return */
	else if (opt->fixed)
        	/* do the "quote into regexp" thing and return */

	/* pcre and other from the original follows */