Re: [PATCH 2/5] grep: remove redundant grep pattern type assignment

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Thu, Jun 29 2017, Stefan Beller jotted:

> On Wed, Jun 28, 2017 at 2:58 PM, Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>> Remove a redundant assignment to extended_regexp_option to make it
>> zero if grep.extendedRegexp is not set. This is always called right
>> after init_grep_defaults() which memsets the entire structure to 0.
>>
>> This is a logical follow-up to my commit to remove redundant regflags
>> assignments[1]. This logic was originally introduced in [2], but as
>> explained in the former commit it's working around a pattern in our
>> code that no longer exists, and is now confusing as it leads the
>> reader to think that this needs to be flipped back & forth.
>>
>> 1. e0b9f8ae09 ("grep: remove redundant regflags assignments",
>>    2017-05-25)
>> 2. b22520a37c ("grep: allow -E and -n to be turned on by default via
>>    configuration", 2011-03-30)
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>>  grep.c | 2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/grep.c b/grep.c
>> index 29439886e7..6614042fdc 100644
>> --- a/grep.c
>> +++ b/grep.c
>> @@ -80,8 +80,6 @@ int grep_config(const char *var, const char *value, void *cb)
>>         if (!strcmp(var, "grep.extendedregexp")) {
>>                 if (git_config_bool(var, value))
>>                         opt->extended_regexp_option = 1;
>> -               else
>> -                       opt->extended_regexp_option = 0;
>>                 return 0;
>
> Instead of having a condition here, have you considered to remove the
> condition alltogether?
>
>     if (!strcmp(var, "grep.extendedregexp")) {
>         opt->extended_regexp_option = git_config_bool(var, value);
>         return 0;
>     }
>
> This does not have the effect of not assigning the value in case of 0,
> but it may be easier to reason about when reading the code.
>
> This would also conform to the code below in that function, that parses
> grep.linenumber or grep.fullname

I didn't think about that. Good point. I'll do that instead in v2.