Re: [PATCH v2 5/6] grep: remove regflags from the public grep_opt API

[Junio C Hamano <gitster@pobox.com>]

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

> @@ -169,6 +167,24 @@ void grep_init(struct grep_opt *opt, const char *prefix)
>  
>  static void grep_set_pattern_type_option(enum grep_pattern_type pattern_type, struct grep_opt *opt)
>  {
> +	/*
> +	 * When committing to the pattern type by setting the relevant
> +	 * fields in grep_opt it's generally not necessary to zero out
> +	 * the fields we're not choosing, since they won't have been
> +	 * set by anything. The extended_regexp_option field is the
> +	 * only exception to this.
> +	 *
> +	 * This is because in the process of parsing grep.patternType
> +	 * & grep.extendedRegexp we set opt->pattern_type_option and
> +	 * opt->extended_regexp_option, respectively. We then
> +	 * internally use opt->extended_regexp_option to see if we're
> +	 * compiling an ERE. It must be unset if that's not actually
> +	 * the case.
> +	 */
> +	if (pattern_type != GREP_PATTERN_TYPE_ERE &&
> +	    opt->extended_regexp_option)
> +		opt->extended_regexp_option = 0;

Good to have the reasoning in an in-code comment like the above.
But after reading these two paragraphs and then before reading the
three line code, a more natural embodiment in the code of the
commentary that came to my mind was

	if (pattern_type != GREP_PATTERN_TYPE_ERE)
		opt->extended_regexp_option = 0;

The end-result is the same as yours, of course, but I somehow found
it match the reasoning better.

Now, I wonder if this can further be tweaked to

	opt->extended_regexp_option = (pattern_type == GREP_PATTERN_TYPE_ERE);

which might lead us in a direction to really unify the two related
fields extended_regexp_option and pattern_type_option.

Even if that were a good longer term direction to go in, it is
outside the scope of this step, of course.  I am merely bringing it
up as an conversation item ;-).