Re: [PATCH 2/2] grep.c: teach 'git grep --only-matching'

From: Jeff King <peff@peff.net>
Date: Jun 28, 2018, 18:32 UTC
Message-ID: <20180628183231.GA12657@sigill.intra.peff.net>
In-Reply-To: <f156bd7c897359926ec407e805ecb0630a8b12da.1529961706.git.me@ttaylorr.com>

On Mon, Jun 25, 2018 at 04:26:00PM -0500, Taylor Blau wrote:

Show 10 quoted lines

> For instance, a line containing the following (taken from README.md:27):
> 
>   (`man gitcvs-migration` or `git help cvs-migration` if git is
> 
> Is printed as follows:
> 
>   $ git grep -no -e git -- README.md | grep ":27"
>   README.md:27:7:git
>   README.md:27:16:git
>   README.md:27:38:git

This is with "--column", too, right?

> Like GNU grep, this patch ignores --only-matching when --invert (-v) is
> given. There is a sensible answer here, but parity with the behavior of
> other tools is preferred.

Yeah, after all of our discussion about inverted matching and columns, I'm sure we could come up with _some_ answer. But I agree that what you have here is quite sensible, and matching GNU grep seems like a good idea.

Show 6 quoted lines

> ---
>  builtin/grep.c  |  6 ++++++
>  grep.c          | 48 +++++++++++++++++++++++++++++++++---------------
>  grep.h          |  1 +
>  t/t7810-grep.sh | 15 +++++++++++++++
>  4 files changed, 55 insertions(+), 15 deletions(-)

The patch itself looks pretty straightforward to me (especially with "-w"). I didn't hit the compiler warning that Junio did (I have gcc 7.3.0). But I agree it's better to avoid even passing an uninitialized variable to another function.

-Peff