Re: [PATCH 0/2] grep: introduce and use grep_and_expr()

From: René Scharfe <l.s.r@web.de>
Date: Jan 7, 2022, 12:57 UTC
Message-ID: <c573cc00-d5b3-9af1-a627-6c2462cae3be@web.de>
In-Reply-To: <cover.1641498525.git.me@ttaylorr.com>

Am 06.01.22 um 20:50 schrieb Taylor Blau:

Show 11 quoted lines

> René,
>
> Here are a couple of extra patches on top of your series which introduce
> and use a new grep_and_expr() function.
>
> Like the final patch says, this isn't about reducing code duplication,
> but rather about adding visual consistency with the other
> `compile_pattern_xyz()` functions.
>
> Taylor Blau (2):
>   grep: extract grep_binexp() from grep_or_expr()

I considered extracting such a function as well. I'd have called it grep_binary_expr(), though, to match the existing names.

I decided against it because it can be misused by passing a non-binary kind to it. (That's a weak objection, but the benefit of such a function was low already in my mind because it doesn't do much.) You solve this by keeping grep_or_expr() and adding grep_and_expr(), which cannot be misused in this way -- OK.

>   grep: use grep_and_expr() in compile_pattern_and()

I think reversing the order of changes would make more sense. You wouldn't have to talk about the respective other patch in the commit messages -- each would stand on their own. Not worth a reroll, though.

Show 6 quoted lines

>
>  grep.c | 22 +++++++++++++++-------
>  1 file changed, 15 insertions(+), 7 deletions(-)
>
> --
> 2.34.1.455.gd6eb6fd089