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

From: Stefan Beller <sbeller@google.com>
Date: Jun 29, 2017, 17:03 UTC
Message-ID: <CAGZ79kb0a2EEALj3eEAMUwQSWamhGuOFOYPrWbv6uCtC7EXgAw@mail.gmail.com>
In-Reply-To: <20170628215809.23060-3-avarab@gmail.com>

On Wed, Jun 28, 2017 at 2:58 PM, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 31 quoted lines

> 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

Thanks, Stefan