[PATCH 0/2] grep.c: teach --only-matching to 'git-grep(1)'

[PATCH 0/2] grep.c: teach --only-matching to 'git-grep(1)'

[Taylor Blau]

Hi,

Attached is a resurrection of my previous topic [1] to add
'--only-matching' (in the style of GNU grep) to 'git grep'.

The changes are described in detail in each of the attached patches.

Similar to the series to add --column to 'git grep', I have restarted
this thread in order to not clutter the old one. I rewrote the patches
from scratch today, and have based them on tb/grep-colno, on top of
which they should apply cleanly.

Thanks in advance for your kind review :-).


Thanks,
Taylor

[1]: https://public-inbox.org/git/cover.1525492696.git.me@ttaylorr.com/

Taylor Blau (2):
  grep.c: extract show_line_header()
  grep.c: teach 'git grep --only-matching'

 builtin/grep.c  |  6 ++++
 grep.c          | 90 +++++++++++++++++++++++++++++++------------------
 grep.h          |  1 +
 t/t7810-grep.sh | 15 +++++++++
 4 files changed, 79 insertions(+), 33 deletions(-)

--
2.18.0

[PATCH 1/2] grep.c: extract show_line_header()

[Taylor Blau]

The grep code invokes show_line() to display the contents of a matched
or context line in its output. Part of this execution is to print a line
header that includes information such as the kind, the line- and
column-number and etc. of that match.

To prepare for the addition of an option to print only the matching
component(s) of a non-context line, we must prepare for the possibility
that a single line may contain multiple matching parts, and thus will
need multiple headers printed for a single line.

Extracting show_line_header allows us to do just that. In the subsequent
commit, it will be used within the colorization loop to print out only
the matching parts of a line, optionally with LFs delimiting
sub-matches.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 grep.c | 44 +++++++++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 19 deletions(-)

diff --git a/grep.c b/grep.c
index 992673fe7e..4ff8a73043 100644
--- a/grep.c
+++ b/grep.c
@@ -1410,26 +1410,9 @@ static int next_match(struct grep_opt *opt, char *bol, char *eol,
 	return hit;
 }

-static void show_line(struct grep_opt *opt, char *bol, char *eol,
-		      const char *name, unsigned lno, ssize_t cno, char sign)
+static void show_line_header(struct grep_opt *opt, const char *name,
+			     unsigned lno, ssize_t cno, char sign)
 {
-	int rest = eol - bol;
-	const char *match_color, *line_color = NULL;
-
-	if (opt->file_break && opt->last_shown == 0) {
-		if (opt->show_hunk_mark)
-			opt->output(opt, "\n", 1);
-	} else if (opt->pre_context || opt->post_context || opt->funcbody) {
-		if (opt->last_shown == 0) {
-			if (opt->show_hunk_mark) {
-				output_color(opt, "--", 2, opt->color_sep);
-				opt->output(opt, "\n", 1);
-			}
-		} else if (lno > opt->last_shown + 1) {
-			output_color(opt, "--", 2, opt->color_sep);
-			opt->output(opt, "\n", 1);
-		}
-	}
 	if (opt->heading && opt->last_shown == 0) {
 		output_color(opt, name, strlen(name), opt->color_filename);
 		opt->output(opt, "\n", 1);
@@ -1457,6 +1440,29 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 		output_color(opt, buf, strlen(buf), opt->color_columnno);
 		output_sep(opt, sign);
 	}
+}
+
+static void show_line(struct grep_opt *opt, char *bol, char *eol,
+		      const char *name, unsigned lno, ssize_t cno, char sign)
+{
+	int rest = eol - bol;
+	const char *match_color, *line_color = NULL;
+
+	if (opt->file_break && opt->last_shown == 0) {
+		if (opt->show_hunk_mark)
+			opt->output(opt, "\n", 1);
+	} else if (opt->pre_context || opt->post_context || opt->funcbody) {
+		if (opt->last_shown == 0) {
+			if (opt->show_hunk_mark) {
+				output_color(opt, "--", 2, opt->color_sep);
+				opt->output(opt, "\n", 1);
+			}
+		} else if (lno > opt->last_shown + 1) {
+			output_color(opt, "--", 2, opt->color_sep);
+			opt->output(opt, "\n", 1);
+		}
+	}
+	show_line_header(opt, name, lno, cno, sign);
 	if (opt->color) {
 		regmatch_t match;
 		enum grep_context ctx = GREP_CONTEXT_BODY;
--
2.18.0

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

[Taylor Blau]

Teach 'git grep --only-matching', a new option to only print the
matching part(s) of a line.

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

The patch works mostly as one would expect, with the exception of a few
considerations that are worth mentioning here.

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.

Because a line might contain more than one match, there are special
considerations pertaining to when to print line headers, newlines, and
how to increment the match column offset. The line header and newlines
are handled as a special case within the main loop to avoid polluting
the surrounding code with conditionals that have large blocks.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 builtin/grep.c  |  6 ++++++
 grep.c          | 48 +++++++++++++++++++++++++++++++++---------------
 grep.h          |  1 +
 t/t7810-grep.sh | 15 +++++++++++++++
 4 files changed, 55 insertions(+), 15 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 61bcaf6e58..228b83990f 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -843,6 +843,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		OPT_BOOL_F('z', "null", &opt.null_following_name,
 			   N_("print NUL after filenames"),
 			   PARSE_OPT_NOCOMPLETE),
+		OPT_BOOL('o', "only-matching", &opt.only_matching,
+			N_("show only matching parts of a line")),
 		OPT_BOOL('c', "count", &opt.count,
 			N_("show the number of matches instead of matching lines")),
 		OPT__COLOR(&opt.color, N_("highlight matches")),
@@ -962,6 +964,10 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	if (!opt.pattern_list)
 		die(_("no pattern given."));

+	/* --only-matching has no effect with --invert. */
+	if (opt.invert)
+		opt.only_matching = 0;
+
 	/*
 	 * We have to find "--" in a separate pass, because its presence
 	 * influences how we will parse arguments that come before it.
diff --git a/grep.c b/grep.c
index 4ff8a73043..48cca6723e 100644
--- a/grep.c
+++ b/grep.c
@@ -51,6 +51,7 @@ void init_grep_defaults(void)
 	color_set(opt->color_match_selected, GIT_COLOR_BOLD_RED);
 	color_set(opt->color_selected, "");
 	color_set(opt->color_sep, GIT_COLOR_CYAN);
+	opt->only_matching = 0;
 	opt->color = -1;
 	opt->output = std_output;
 }
@@ -158,6 +159,7 @@ void grep_init(struct grep_opt *opt, const char *prefix)
 	opt->pattern_tail = &opt->pattern_list;
 	opt->header_tail = &opt->header_list;

+	opt->only_matching = def->only_matching;
 	opt->color = def->color;
 	opt->extended_regexp_option = def->extended_regexp_option;
 	opt->pattern_type_option = def->pattern_type_option;
@@ -1462,39 +1464,55 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 			opt->output(opt, "\n", 1);
 		}
 	}
-	show_line_header(opt, name, lno, cno, sign);
-	if (opt->color) {
+	if (!opt->only_matching) {
+		/*
+		 * In case the line we're being called with contains more than
+		 * one match, leave printing each header to the loop below.
+		 */
+		show_line_header(opt, name, lno, cno, sign);
+	}
+	if (opt->color || opt->only_matching) {
 		regmatch_t match;
 		enum grep_context ctx = GREP_CONTEXT_BODY;
 		int ch = *eol;
 		int eflags = 0;

-		if (sign == ':')
-			match_color = opt->color_match_selected;
-		else
-			match_color = opt->color_match_context;
-		if (sign == ':')
-			line_color = opt->color_selected;
-		else if (sign == '-')
-			line_color = opt->color_context;
-		else if (sign == '=')
-			line_color = opt->color_function;
+		if (opt->color) {
+			if (sign == ':')
+				match_color = opt->color_match_selected;
+			else
+				match_color = opt->color_match_context;
+			if (sign == ':')
+				line_color = opt->color_selected;
+			else if (sign == '-')
+				line_color = opt->color_context;
+			else if (sign == '=')
+				line_color = opt->color_function;
+		}
 		*eol = '\0';
 		while (next_match(opt, bol, eol, ctx, &match, eflags)) {
 			if (match.rm_so == match.rm_eo)
 				break;

-			output_color(opt, bol, match.rm_so, line_color);
+			if (opt->only_matching)
+				show_line_header(opt, name, lno, cno, sign);
+			else
+				output_color(opt, bol, match.rm_so, line_color);
 			output_color(opt, bol + match.rm_so,
 				     match.rm_eo - match.rm_so, match_color);
+			if (opt->only_matching)
+				opt->output(opt, "\n", 1);
 			bol += match.rm_eo;
+			cno += match.rm_eo;
 			rest -= match.rm_eo;
 			eflags = REG_NOTBOL;
 		}
 		*eol = ch;
 	}
-	output_color(opt, bol, rest, line_color);
-	opt->output(opt, "\n", 1);
+	if (!opt->only_matching) {
+		output_color(opt, bol, rest, line_color);
+		opt->output(opt, "\n", 1);
+	}
 }

 #ifndef NO_PTHREADS
diff --git a/grep.h b/grep.h
index 08a0b391c5..4d474d8ec4 100644
--- a/grep.h
+++ b/grep.h
@@ -150,6 +150,7 @@ struct grep_opt {
 	int relative;
 	int pathname;
 	int null_following_name;
+	int only_matching;
 	int color;
 	int max_depth;
 	int funcname;
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 9312c8daf5..d8c232dbf4 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -262,6 +262,21 @@ do
 		fi
 	'

+	test_expect_success "grep $L (with --column, --only-matching)" '
+		{
+			echo ${HC}file:1:5:mmap
+			echo ${HC}file:2:5:mmap
+			echo ${HC}file:3:5:mmap
+			echo ${HC}file:3:13:mmap
+			echo ${HC}file:4:5:mmap
+			echo ${HC}file:4:13:mmap
+			echo ${HC}file:5:5:mmap
+			echo ${HC}file:5:13:mmap
+		} >expected &&
+		git grep --column -n -o -e mmap $H >actual &&
+		test_cmp expected actual
+	'
+
 	test_expect_success "grep $L (t-1)" '
 		echo "${HC}t/t:1:test" >expected &&
 		git grep -n -e test $H >actual &&
--
2.18.0

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

[Junio C Hamano]

Taylor Blau <me@ttaylorr.com> writes:

> -		if (sign == ':')
> -			match_color = opt->color_match_selected;
> -		else
> -			match_color = opt->color_match_context;
> -		if (sign == ':')
> -			line_color = opt->color_selected;
> -		else if (sign == '-')
> -			line_color = opt->color_context;
> -		else if (sign == '=')
> -			line_color = opt->color_function;
> +		if (opt->color) {
> +			if (sign == ':')
> +				match_color = opt->color_match_selected;
> +			else
> +				match_color = opt->color_match_context;
> +			if (sign == ':')
> +				line_color = opt->color_selected;
> +			else if (sign == '-')
> +				line_color = opt->color_context;
> +			else if (sign == '=')
> +				line_color = opt->color_function;
> +		}

The above change (specifically, the fact that this now is enclosed
in "if (opt->color) { ... }") unfortunately leaves match_color
undefined at this point in the control flow.  The next loop then
calls output_color() with an undefined match_color and tricks stupid
compiler to issue a warning and makes -Werror build to fail.

>  		*eol = '\0';
>  		while (next_match(opt, bol, eol, ctx, &match, eflags)) {
>  			if (match.rm_so == match.rm_eo)
>  				break;
>
> -			output_color(opt, bol, match.rm_so, line_color);
> +			if (opt->only_matching)
> +				show_line_header(opt, name, lno, cno, sign);
> +			else
> +				output_color(opt, bol, match.rm_so, line_color);
>  			output_color(opt, bol + match.rm_so,
>  				     match.rm_eo - match.rm_so, match_color);

output_color() does check want_color(opt->color) before using its
last parameter, and want_color() gives false for opt->color that is
0 (i.e. leaves match_color to be undefined), so in this case, the
compiler is worried too much, but still, we should work it around if
it is easy to do so.

Just initializing match_color where it is defined at the beginning of
show_line() should be sufficient, I think.

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

[Taylor Blau]

On Wed, Jun 27, 2018 at 09:40:10AM -0700, Junio C Hamano wrote:
> Taylor Blau <me@ttaylorr.com> writes:
>
> > -		if (sign == ':')
> > -			match_color = opt->color_match_selected;
> > -		else
> > -			match_color = opt->color_match_context;
> > -		if (sign == ':')
> > -			line_color = opt->color_selected;
> > -		else if (sign == '-')
> > -			line_color = opt->color_context;
> > -		else if (sign == '=')
> > -			line_color = opt->color_function;
> > +		if (opt->color) {
> > +			if (sign == ':')
> > +				match_color = opt->color_match_selected;
> > +			else
> > +				match_color = opt->color_match_context;
> > +			if (sign == ':')
> > +				line_color = opt->color_selected;
> > +			else if (sign == '-')
> > +				line_color = opt->color_context;
> > +			else if (sign == '=')
> > +				line_color = opt->color_function;
> > +		}
>
> The above change (specifically, the fact that this now is enclosed
> in "if (opt->color) { ... }") unfortunately leaves match_color
> undefined at this point in the control flow.  The next loop then
> calls output_color() with an undefined match_color and tricks stupid
> compiler to issue a warning and makes -Werror build to fail.

Right, this is because we are using the `while (next_match(...))` loop
for non-colorized output, which is new. This seems like a definite
problem, and certainly causes the -Werror build to fail for me, too.

> >  		*eol = '\0';
> >  		while (next_match(opt, bol, eol, ctx, &match, eflags)) {
> >  			if (match.rm_so == match.rm_eo)
> >  				break;
> >
> > -			output_color(opt, bol, match.rm_so, line_color);
> > +			if (opt->only_matching)
> > +				show_line_header(opt, name, lno, cno, sign);
> > +			else
> > +				output_color(opt, bol, match.rm_so, line_color);
> >  			output_color(opt, bol + match.rm_so,
> >  				     match.rm_eo - match.rm_so, match_color);
>
> output_color() does check want_color(opt->color) before using its
> last parameter, and want_color() gives false for opt->color that is
> 0 (i.e. leaves match_color to be undefined), so in this case, the
> compiler is worried too much, but still, we should work it around if
> it is easy to do so.
>
> Just initializing match_color where it is defined at the beginning of
> show_line() should be sufficient, I think.

I think that we could also use the following, and leave the `if
(opt->color)` conditional where it is:

diff --git a/grep.c b/grep.c
index 48cca6723e..b985fb3ee0 100644
--- a/grep.c
+++ b/grep.c
@@ -1448,7 +1448,7 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 		      const char *name, unsigned lno, ssize_t cno, char sign)
 {
 	int rest = eol - bol;
-	const char *match_color, *line_color = NULL;
+	const char *match_color = NULL, *line_color = NULL;

 	if (opt->file_break && opt->last_shown == 0) {
 		if (opt->show_hunk_mark)

From my reading, output_color() appears happy to treat a NULL color as
meaningless, and instead redirect the call to `opt->output` without the
preceding colorization and the reset afterwords.

We could also move that `if (opt->color)` block up closer to where
line_color and match_color are initialized, which might appear cleaner.
I think the best time to do that would be in a preparatory patch in this
series.

What do you think?


Thanks,
Taylor

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

[Junio C Hamano]

Taylor Blau <me@ttaylorr.com> writes:

>> Just initializing match_color where it is defined at the beginning of
>> show_line() should be sufficient, I think.
>
> I think that we could also use the following, and leave the `if
> (opt->color)` conditional where it is:
>
> diff --git a/grep.c b/grep.c
> index 48cca6723e..b985fb3ee0 100644
> --- a/grep.c
> +++ b/grep.c
> @@ -1448,7 +1448,7 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
>  		      const char *name, unsigned lno, ssize_t cno, char sign)
>  {
>  	int rest = eol - bol;
> -	const char *match_color, *line_color = NULL;
> +	const char *match_color = NULL, *line_color = NULL;
>
>  	if (opt->file_break && opt->last_shown == 0) {
>  		if (opt->show_hunk_mark)

You say "we could also", but the above is exactly what I suggested,
so we seem to be on the same page, which is good ;-)

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

[Taylor Blau]

On Wed, Jun 27, 2018 at 02:11:13PM -0700, Junio C Hamano wrote:
> Taylor Blau <me@ttaylorr.com> writes:
>
> >> Just initializing match_color where it is defined at the beginning of
> >> show_line() should be sufficient, I think.
> >
> > I think that we could also use the following, and leave the `if
> > (opt->color)` conditional where it is:
> >
> > diff --git a/grep.c b/grep.c
> > index 48cca6723e..b985fb3ee0 100644
> > --- a/grep.c
> > +++ b/grep.c
> > @@ -1448,7 +1448,7 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
> >  		      const char *name, unsigned lno, ssize_t cno, char sign)
> >  {
> >  	int rest = eol - bol;
> > -	const char *match_color, *line_color = NULL;
> > +	const char *match_color = NULL, *line_color = NULL;
> >
> >  	if (opt->file_break && opt->last_shown == 0) {
> >  		if (opt->show_hunk_mark)
>
> You say "we could also", but the above is exactly what I suggested,
> so we seem to be on the same page, which is good ;-)


Ah, sorry -- I misinterpreted your meaning "initializing match_color
where it is defined" to mean bringing the large `if (opt->color)`
conditional up, instead of just assigning to NULL.

Let's do that and leave the `if (opt->color)` block where it is?


Thanks,
Taylor

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

[Jeff King]

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

> 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.

> ---
>  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

[PATCH v2 0/2] teach --only-matching to 'git-grep(1)'

[Taylor Blau]

Hi,

Attached is the second re-roll of my series to teach 'git grep
--only-matching'. Since last time, not much has changed. The change I
did include is summarized below, and an inter-diff is attached as
always.

  - Initialize both match_color and line_color to NULL, thereby
    silencing a compiler warning where match_color was given to
    opt->output_color uninitialized [1].

Thanks in advance for your review :-).


Thanks,
Taylor

[1]: https://public-inbox.org/git/xmqqsh58gp3p.fsf@gitster-ct.c.googlers.com/

Taylor Blau (2):
  grep.c: extract show_line_header()
  grep.c: teach 'git grep --only-matching'

 builtin/grep.c  |  6 ++++
 grep.c          | 91 +++++++++++++++++++++++++++++++------------------
 grep.h          |  1 +
 t/t7810-grep.sh | 15 ++++++++
 4 files changed, 80 insertions(+), 33 deletions(-)

Inter-diff (since v1):

diff --git a/grep.c b/grep.c
index 48cca6723e..49a744f96b 100644
--- a/grep.c
+++ b/grep.c
@@ -1448,7 +1448,8 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
                      const char *name, unsigned lno, ssize_t cno, char sign)
 {
        int rest = eol - bol;
-       const char *match_color, *line_color = NULL;
+       const char *match_color = NULL;
+       const char *line_color = NULL;

        if (opt->file_break && opt->last_shown == 0) {
                if (opt->show_hunk_mark)

--
2.18.0

[PATCH v2 1/2] grep.c: extract show_line_header()

[Taylor Blau]

The grep code invokes show_line() to display the contents of a matched
or context line in its output. Part of this execution is to print a line
header that includes information such as the kind, the line- and
column-number and etc. of that match.

To prepare for the addition of an option to print only the matching
component(s) of a non-context line, we must prepare for the possibility
that a single line may contain multiple matching parts, and thus will
need multiple headers printed for a single line.

Extracting show_line_header allows us to do just that. In the subsequent
commit, it will be used within the colorization loop to print out only
the matching parts of a line, optionally with LFs delimiting
sub-matches.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 grep.c | 44 +++++++++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 19 deletions(-)

diff --git a/grep.c b/grep.c
index 992673fe7e..4ff8a73043 100644
--- a/grep.c
+++ b/grep.c
@@ -1410,26 +1410,9 @@ static int next_match(struct grep_opt *opt, char *bol, char *eol,
 	return hit;
 }
 
-static void show_line(struct grep_opt *opt, char *bol, char *eol,
-		      const char *name, unsigned lno, ssize_t cno, char sign)
+static void show_line_header(struct grep_opt *opt, const char *name,
+			     unsigned lno, ssize_t cno, char sign)
 {
-	int rest = eol - bol;
-	const char *match_color, *line_color = NULL;
-
-	if (opt->file_break && opt->last_shown == 0) {
-		if (opt->show_hunk_mark)
-			opt->output(opt, "\n", 1);
-	} else if (opt->pre_context || opt->post_context || opt->funcbody) {
-		if (opt->last_shown == 0) {
-			if (opt->show_hunk_mark) {
-				output_color(opt, "--", 2, opt->color_sep);
-				opt->output(opt, "\n", 1);
-			}
-		} else if (lno > opt->last_shown + 1) {
-			output_color(opt, "--", 2, opt->color_sep);
-			opt->output(opt, "\n", 1);
-		}
-	}
 	if (opt->heading && opt->last_shown == 0) {
 		output_color(opt, name, strlen(name), opt->color_filename);
 		opt->output(opt, "\n", 1);
@@ -1457,6 +1440,29 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 		output_color(opt, buf, strlen(buf), opt->color_columnno);
 		output_sep(opt, sign);
 	}
+}
+
+static void show_line(struct grep_opt *opt, char *bol, char *eol,
+		      const char *name, unsigned lno, ssize_t cno, char sign)
+{
+	int rest = eol - bol;
+	const char *match_color, *line_color = NULL;
+
+	if (opt->file_break && opt->last_shown == 0) {
+		if (opt->show_hunk_mark)
+			opt->output(opt, "\n", 1);
+	} else if (opt->pre_context || opt->post_context || opt->funcbody) {
+		if (opt->last_shown == 0) {
+			if (opt->show_hunk_mark) {
+				output_color(opt, "--", 2, opt->color_sep);
+				opt->output(opt, "\n", 1);
+			}
+		} else if (lno > opt->last_shown + 1) {
+			output_color(opt, "--", 2, opt->color_sep);
+			opt->output(opt, "\n", 1);
+		}
+	}
+	show_line_header(opt, name, lno, cno, sign);
 	if (opt->color) {
 		regmatch_t match;
 		enum grep_context ctx = GREP_CONTEXT_BODY;
-- 
2.18.0

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

[Taylor Blau]

Teach 'git grep --only-matching', a new option to only print the
matching part(s) of a line.

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

The patch works mostly as one would expect, with the exception of a few
considerations that are worth mentioning here.

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.

Because a line might contain more than one match, there are special
considerations pertaining to when to print line headers, newlines, and
how to increment the match column offset. The line header and newlines
are handled as a special case within the main loop to avoid polluting
the surrounding code with conditionals that have large blocks.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 builtin/grep.c  |  6 ++++++
 grep.c          | 51 +++++++++++++++++++++++++++++++++----------------
 grep.h          |  1 +
 t/t7810-grep.sh | 15 +++++++++++++++
 4 files changed, 57 insertions(+), 16 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 61bcaf6e58..228b83990f 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -843,6 +843,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		OPT_BOOL_F('z', "null", &opt.null_following_name,
 			   N_("print NUL after filenames"),
 			   PARSE_OPT_NOCOMPLETE),
+		OPT_BOOL('o', "only-matching", &opt.only_matching,
+			N_("show only matching parts of a line")),
 		OPT_BOOL('c', "count", &opt.count,
 			N_("show the number of matches instead of matching lines")),
 		OPT__COLOR(&opt.color, N_("highlight matches")),
@@ -962,6 +964,10 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	if (!opt.pattern_list)
 		die(_("no pattern given."));
 
+	/* --only-matching has no effect with --invert. */
+	if (opt.invert)
+		opt.only_matching = 0;
+
 	/*
 	 * We have to find "--" in a separate pass, because its presence
 	 * influences how we will parse arguments that come before it.
diff --git a/grep.c b/grep.c
index 4ff8a73043..49a744f96b 100644
--- a/grep.c
+++ b/grep.c
@@ -51,6 +51,7 @@ void init_grep_defaults(void)
 	color_set(opt->color_match_selected, GIT_COLOR_BOLD_RED);
 	color_set(opt->color_selected, "");
 	color_set(opt->color_sep, GIT_COLOR_CYAN);
+	opt->only_matching = 0;
 	opt->color = -1;
 	opt->output = std_output;
 }
@@ -158,6 +159,7 @@ void grep_init(struct grep_opt *opt, const char *prefix)
 	opt->pattern_tail = &opt->pattern_list;
 	opt->header_tail = &opt->header_list;
 
+	opt->only_matching = def->only_matching;
 	opt->color = def->color;
 	opt->extended_regexp_option = def->extended_regexp_option;
 	opt->pattern_type_option = def->pattern_type_option;
@@ -1446,7 +1448,8 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 		      const char *name, unsigned lno, ssize_t cno, char sign)
 {
 	int rest = eol - bol;
-	const char *match_color, *line_color = NULL;
+	const char *match_color = NULL;
+	const char *line_color = NULL;
 
 	if (opt->file_break && opt->last_shown == 0) {
 		if (opt->show_hunk_mark)
@@ -1462,39 +1465,55 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 			opt->output(opt, "\n", 1);
 		}
 	}
-	show_line_header(opt, name, lno, cno, sign);
-	if (opt->color) {
+	if (!opt->only_matching) {
+		/*
+		 * In case the line we're being called with contains more than
+		 * one match, leave printing each header to the loop below.
+		 */
+		show_line_header(opt, name, lno, cno, sign);
+	}
+	if (opt->color || opt->only_matching) {
 		regmatch_t match;
 		enum grep_context ctx = GREP_CONTEXT_BODY;
 		int ch = *eol;
 		int eflags = 0;
 
-		if (sign == ':')
-			match_color = opt->color_match_selected;
-		else
-			match_color = opt->color_match_context;
-		if (sign == ':')
-			line_color = opt->color_selected;
-		else if (sign == '-')
-			line_color = opt->color_context;
-		else if (sign == '=')
-			line_color = opt->color_function;
+		if (opt->color) {
+			if (sign == ':')
+				match_color = opt->color_match_selected;
+			else
+				match_color = opt->color_match_context;
+			if (sign == ':')
+				line_color = opt->color_selected;
+			else if (sign == '-')
+				line_color = opt->color_context;
+			else if (sign == '=')
+				line_color = opt->color_function;
+		}
 		*eol = '\0';
 		while (next_match(opt, bol, eol, ctx, &match, eflags)) {
 			if (match.rm_so == match.rm_eo)
 				break;
 
-			output_color(opt, bol, match.rm_so, line_color);
+			if (opt->only_matching)
+				show_line_header(opt, name, lno, cno, sign);
+			else
+				output_color(opt, bol, match.rm_so, line_color);
 			output_color(opt, bol + match.rm_so,
 				     match.rm_eo - match.rm_so, match_color);
+			if (opt->only_matching)
+				opt->output(opt, "\n", 1);
 			bol += match.rm_eo;
+			cno += match.rm_eo;
 			rest -= match.rm_eo;
 			eflags = REG_NOTBOL;
 		}
 		*eol = ch;
 	}
-	output_color(opt, bol, rest, line_color);
-	opt->output(opt, "\n", 1);
+	if (!opt->only_matching) {
+		output_color(opt, bol, rest, line_color);
+		opt->output(opt, "\n", 1);
+	}
 }
 
 #ifndef NO_PTHREADS
diff --git a/grep.h b/grep.h
index 08a0b391c5..4d474d8ec4 100644
--- a/grep.h
+++ b/grep.h
@@ -150,6 +150,7 @@ struct grep_opt {
 	int relative;
 	int pathname;
 	int null_following_name;
+	int only_matching;
 	int color;
 	int max_depth;
 	int funcname;
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 9312c8daf5..d8c232dbf4 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -262,6 +262,21 @@ do
 		fi
 	'
 
+	test_expect_success "grep $L (with --column, --only-matching)" '
+		{
+			echo ${HC}file:1:5:mmap
+			echo ${HC}file:2:5:mmap
+			echo ${HC}file:3:5:mmap
+			echo ${HC}file:3:13:mmap
+			echo ${HC}file:4:5:mmap
+			echo ${HC}file:4:13:mmap
+			echo ${HC}file:5:5:mmap
+			echo ${HC}file:5:13:mmap
+		} >expected &&
+		git grep --column -n -o -e mmap $H >actual &&
+		test_cmp expected actual
+	'
+
 	test_expect_success "grep $L (t-1)" '
 		echo "${HC}t/t:1:test" >expected &&
 		git grep -n -e test $H >actual &&
-- 
2.18.0

Re: [PATCH v2 0/2] teach --only-matching to 'git-grep(1)'

[Jeff King]

On Mon, Jul 02, 2018 at 03:08:54PM -0500, Taylor Blau wrote:

> Attached is the second re-roll of my series to teach 'git grep
> --only-matching'. Since last time, not much has changed. The change I
> did include is summarized below, and an inter-diff is attached as
> always.
> 
>   - Initialize both match_color and line_color to NULL, thereby
>     silencing a compiler warning where match_color was given to
>     opt->output_color uninitialized [1].

This iteration looks fine to me. I think we'd ideally do
s/grep/& --column/ in the commit message of the second patch, but that's
not worth re-rolling.

-Peff

Re: [PATCH v2 0/2] teach --only-matching to 'git-grep(1)'

[Jeff King]

On Tue, Jul 03, 2018 at 10:37:29AM -0400, Jeff King wrote:

> On Mon, Jul 02, 2018 at 03:08:54PM -0500, Taylor Blau wrote:
> 
> > Attached is the second re-roll of my series to teach 'git grep
> > --only-matching'. Since last time, not much has changed. The change I
> > did include is summarized below, and an inter-diff is attached as
> > always.
> > 
> >   - Initialize both match_color and line_color to NULL, thereby
> >     silencing a compiler warning where match_color was given to
> >     opt->output_color uninitialized [1].
> 
> This iteration looks fine to me. I think we'd ideally do
> s/grep/& --column/ in the commit message of the second patch, but that's
> not worth re-rolling.

In the example it shows, I mean (a global s/ would not be good ;) ).

-Peff

Re: [PATCH v2 0/2] teach --only-matching to 'git-grep(1)'

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Mon, Jul 02, 2018 at 03:08:54PM -0500, Taylor Blau wrote:
>
>> Attached is the second re-roll of my series to teach 'git grep
>> --only-matching'. Since last time, not much has changed. The change I
>> did include is summarized below, and an inter-diff is attached as
>> always.
>> 
>>   - Initialize both match_color and line_color to NULL, thereby
>>     silencing a compiler warning where match_color was given to
>>     opt->output_color uninitialized [1].
>
> This iteration looks fine to me. I think we'd ideally do
> s/grep/& --column/ in the commit message of the second patch, but that's
> not worth re-rolling.

I'm OK doing this myself while queuing:

- $ git grep -no -e git -- README.md | grep ":27"
+ $ git grep -n --only-matching --column -e git -- README.md | grep ":27"

but the lack of changes to Documentation/git-grep.txt makes the
topic worth rerolling anyway (I needed to look up what "-no" means
there, and then found there was no such option in the manpage ;-).

[PATCH v3 0/2] grep.c: teach --only-matching to 'git-grep(1)'

[Taylor Blau]

Hi,

Attached here is a third re-roll of my series to teach 'git grep
--only-matching'. (I didn't mention it in the thread, but I _thought_
that twice would be enough, so I think Peff's advice about not counting
on anything being the final re-roll of something applies to my thoughts,
too ;-) ).

Since last time:

  - The second patch has been amended to include the full invocation of
    'git grep' (including `--column`, `--only-matching`, and
    `--line-number`) [1].

  - The second patch has been also amended to add the new option
    (`--only-matching`) to Documentation/git-grep.txt per [2].

An inter-diff is available below, and thanks as always for your review
:-).

Thanks,
Taylor

[1]: https://public-inbox.org/git/20180703143820.GC23556@sigill.intra.peff.net/
[2]: https://public-inbox.org/git/xmqq1sckoxk8.fsf@gitster-ct.c.googlers.com/

Taylor Blau (2):
  grep.c: extract show_line_header()
  grep.c: teach 'git grep --only-matching'

 Documentation/git-grep.txt |  6 ++-
 builtin/grep.c             |  6 +++
 grep.c                     | 91 ++++++++++++++++++++++++--------------
 grep.h                     |  1 +
 t/t7810-grep.sh            | 15 +++++++
 5 files changed, 85 insertions(+), 34 deletions(-)

Inter-diff (since v2):

diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index 0de3493b80..be13fc3253 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -17,7 +17,7 @@ SYNOPSIS
 	   [-l | --files-with-matches] [-L | --files-without-match]
 	   [(-O | --open-files-in-pager) [<pager>]]
 	   [-z | --null]
-	   [-c | --count] [--all-match] [-q | --quiet]
+	   [ -o | --only-matching ] [-c | --count] [--all-match] [-q | --quiet]
 	   [--max-depth <depth>]
 	   [--color[=<when>] | --no-color]
 	   [--break] [--heading] [-p | --show-function]
@@ -201,6 +201,10 @@ providing this option will cause it to die.
 	Output \0 instead of the character that normally follows a
 	file name.

+-o::
+--only-matching::
+  Output only the matching part of the lines.
+
 -c::
 --count::
 	Instead of showing every matched line, show the number of

--
2.18.0

[PATCH v3 1/2] grep.c: extract show_line_header()

[Taylor Blau]

The grep code invokes show_line() to display the contents of a matched
or context line in its output. Part of this execution is to print a line
header that includes information such as the kind, the line- and
column-number and etc. of that match.

To prepare for the addition of an option to print only the matching
component(s) of a non-context line, we must prepare for the possibility
that a single line may contain multiple matching parts, and thus will
need multiple headers printed for a single line.

Extracting show_line_header allows us to do just that. In the subsequent
commit, it will be used within the colorization loop to print out only
the matching parts of a line, optionally with LFs delimiting
sub-matches.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 grep.c | 44 +++++++++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 19 deletions(-)

diff --git a/grep.c b/grep.c
index 992673fe7e..4ff8a73043 100644
--- a/grep.c
+++ b/grep.c
@@ -1410,26 +1410,9 @@ static int next_match(struct grep_opt *opt, char *bol, char *eol,
 	return hit;
 }
 
-static void show_line(struct grep_opt *opt, char *bol, char *eol,
-		      const char *name, unsigned lno, ssize_t cno, char sign)
+static void show_line_header(struct grep_opt *opt, const char *name,
+			     unsigned lno, ssize_t cno, char sign)
 {
-	int rest = eol - bol;
-	const char *match_color, *line_color = NULL;
-
-	if (opt->file_break && opt->last_shown == 0) {
-		if (opt->show_hunk_mark)
-			opt->output(opt, "\n", 1);
-	} else if (opt->pre_context || opt->post_context || opt->funcbody) {
-		if (opt->last_shown == 0) {
-			if (opt->show_hunk_mark) {
-				output_color(opt, "--", 2, opt->color_sep);
-				opt->output(opt, "\n", 1);
-			}
-		} else if (lno > opt->last_shown + 1) {
-			output_color(opt, "--", 2, opt->color_sep);
-			opt->output(opt, "\n", 1);
-		}
-	}
 	if (opt->heading && opt->last_shown == 0) {
 		output_color(opt, name, strlen(name), opt->color_filename);
 		opt->output(opt, "\n", 1);
@@ -1457,6 +1440,29 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 		output_color(opt, buf, strlen(buf), opt->color_columnno);
 		output_sep(opt, sign);
 	}
+}
+
+static void show_line(struct grep_opt *opt, char *bol, char *eol,
+		      const char *name, unsigned lno, ssize_t cno, char sign)
+{
+	int rest = eol - bol;
+	const char *match_color, *line_color = NULL;
+
+	if (opt->file_break && opt->last_shown == 0) {
+		if (opt->show_hunk_mark)
+			opt->output(opt, "\n", 1);
+	} else if (opt->pre_context || opt->post_context || opt->funcbody) {
+		if (opt->last_shown == 0) {
+			if (opt->show_hunk_mark) {
+				output_color(opt, "--", 2, opt->color_sep);
+				opt->output(opt, "\n", 1);
+			}
+		} else if (lno > opt->last_shown + 1) {
+			output_color(opt, "--", 2, opt->color_sep);
+			opt->output(opt, "\n", 1);
+		}
+	}
+	show_line_header(opt, name, lno, cno, sign);
 	if (opt->color) {
 		regmatch_t match;
 		enum grep_context ctx = GREP_CONTEXT_BODY;
-- 
2.18.0

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

[Taylor Blau]

Teach 'git grep --only-matching', a new option to only print the
matching part(s) of a line.

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 --line-number --column --only-matching -e git -- \
    README.md | grep ":27"
  README.md:27:7:git
  README.md:27:16:git
  README.md:27:38:git

The patch works mostly as one would expect, with the exception of a few
considerations that are worth mentioning here.

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.

Because a line might contain more than one match, there are special
considerations pertaining to when to print line headers, newlines, and
how to increment the match column offset. The line header and newlines
are handled as a special case within the main loop to avoid polluting
the surrounding code with conditionals that have large blocks.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 Documentation/git-grep.txt |  6 ++++-
 builtin/grep.c             |  6 +++++
 grep.c                     | 51 ++++++++++++++++++++++++++------------
 grep.h                     |  1 +
 t/t7810-grep.sh            | 15 +++++++++++
 5 files changed, 62 insertions(+), 17 deletions(-)

diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index 0de3493b80..be13fc3253 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -17,7 +17,7 @@ SYNOPSIS
 	   [-l | --files-with-matches] [-L | --files-without-match]
 	   [(-O | --open-files-in-pager) [<pager>]]
 	   [-z | --null]
-	   [-c | --count] [--all-match] [-q | --quiet]
+	   [ -o | --only-matching ] [-c | --count] [--all-match] [-q | --quiet]
 	   [--max-depth <depth>]
 	   [--color[=<when>] | --no-color]
 	   [--break] [--heading] [-p | --show-function]
@@ -201,6 +201,10 @@ providing this option will cause it to die.
 	Output \0 instead of the character that normally follows a
 	file name.
 
+-o::
+--only-matching::
+  Output only the matching part of the lines.
+
 -c::
 --count::
 	Instead of showing every matched line, show the number of
diff --git a/builtin/grep.c b/builtin/grep.c
index 61bcaf6e58..228b83990f 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -843,6 +843,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		OPT_BOOL_F('z', "null", &opt.null_following_name,
 			   N_("print NUL after filenames"),
 			   PARSE_OPT_NOCOMPLETE),
+		OPT_BOOL('o', "only-matching", &opt.only_matching,
+			N_("show only matching parts of a line")),
 		OPT_BOOL('c', "count", &opt.count,
 			N_("show the number of matches instead of matching lines")),
 		OPT__COLOR(&opt.color, N_("highlight matches")),
@@ -962,6 +964,10 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	if (!opt.pattern_list)
 		die(_("no pattern given."));
 
+	/* --only-matching has no effect with --invert. */
+	if (opt.invert)
+		opt.only_matching = 0;
+
 	/*
 	 * We have to find "--" in a separate pass, because its presence
 	 * influences how we will parse arguments that come before it.
diff --git a/grep.c b/grep.c
index 4ff8a73043..49a744f96b 100644
--- a/grep.c
+++ b/grep.c
@@ -51,6 +51,7 @@ void init_grep_defaults(void)
 	color_set(opt->color_match_selected, GIT_COLOR_BOLD_RED);
 	color_set(opt->color_selected, "");
 	color_set(opt->color_sep, GIT_COLOR_CYAN);
+	opt->only_matching = 0;
 	opt->color = -1;
 	opt->output = std_output;
 }
@@ -158,6 +159,7 @@ void grep_init(struct grep_opt *opt, const char *prefix)
 	opt->pattern_tail = &opt->pattern_list;
 	opt->header_tail = &opt->header_list;
 
+	opt->only_matching = def->only_matching;
 	opt->color = def->color;
 	opt->extended_regexp_option = def->extended_regexp_option;
 	opt->pattern_type_option = def->pattern_type_option;
@@ -1446,7 +1448,8 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 		      const char *name, unsigned lno, ssize_t cno, char sign)
 {
 	int rest = eol - bol;
-	const char *match_color, *line_color = NULL;
+	const char *match_color = NULL;
+	const char *line_color = NULL;
 
 	if (opt->file_break && opt->last_shown == 0) {
 		if (opt->show_hunk_mark)
@@ -1462,39 +1465,55 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 			opt->output(opt, "\n", 1);
 		}
 	}
-	show_line_header(opt, name, lno, cno, sign);
-	if (opt->color) {
+	if (!opt->only_matching) {
+		/*
+		 * In case the line we're being called with contains more than
+		 * one match, leave printing each header to the loop below.
+		 */
+		show_line_header(opt, name, lno, cno, sign);
+	}
+	if (opt->color || opt->only_matching) {
 		regmatch_t match;
 		enum grep_context ctx = GREP_CONTEXT_BODY;
 		int ch = *eol;
 		int eflags = 0;
 
-		if (sign == ':')
-			match_color = opt->color_match_selected;
-		else
-			match_color = opt->color_match_context;
-		if (sign == ':')
-			line_color = opt->color_selected;
-		else if (sign == '-')
-			line_color = opt->color_context;
-		else if (sign == '=')
-			line_color = opt->color_function;
+		if (opt->color) {
+			if (sign == ':')
+				match_color = opt->color_match_selected;
+			else
+				match_color = opt->color_match_context;
+			if (sign == ':')
+				line_color = opt->color_selected;
+			else if (sign == '-')
+				line_color = opt->color_context;
+			else if (sign == '=')
+				line_color = opt->color_function;
+		}
 		*eol = '\0';
 		while (next_match(opt, bol, eol, ctx, &match, eflags)) {
 			if (match.rm_so == match.rm_eo)
 				break;
 
-			output_color(opt, bol, match.rm_so, line_color);
+			if (opt->only_matching)
+				show_line_header(opt, name, lno, cno, sign);
+			else
+				output_color(opt, bol, match.rm_so, line_color);
 			output_color(opt, bol + match.rm_so,
 				     match.rm_eo - match.rm_so, match_color);
+			if (opt->only_matching)
+				opt->output(opt, "\n", 1);
 			bol += match.rm_eo;
+			cno += match.rm_eo;
 			rest -= match.rm_eo;
 			eflags = REG_NOTBOL;
 		}
 		*eol = ch;
 	}
-	output_color(opt, bol, rest, line_color);
-	opt->output(opt, "\n", 1);
+	if (!opt->only_matching) {
+		output_color(opt, bol, rest, line_color);
+		opt->output(opt, "\n", 1);
+	}
 }
 
 #ifndef NO_PTHREADS
diff --git a/grep.h b/grep.h
index 08a0b391c5..4d474d8ec4 100644
--- a/grep.h
+++ b/grep.h
@@ -150,6 +150,7 @@ struct grep_opt {
 	int relative;
 	int pathname;
 	int null_following_name;
+	int only_matching;
 	int color;
 	int max_depth;
 	int funcname;
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 9312c8daf5..d8c232dbf4 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -262,6 +262,21 @@ do
 		fi
 	'
 
+	test_expect_success "grep $L (with --column, --only-matching)" '
+		{
+			echo ${HC}file:1:5:mmap
+			echo ${HC}file:2:5:mmap
+			echo ${HC}file:3:5:mmap
+			echo ${HC}file:3:13:mmap
+			echo ${HC}file:4:5:mmap
+			echo ${HC}file:4:13:mmap
+			echo ${HC}file:5:5:mmap
+			echo ${HC}file:5:13:mmap
+		} >expected &&
+		git grep --column -n -o -e mmap $H >actual &&
+		test_cmp expected actual
+	'
+
 	test_expect_success "grep $L (t-1)" '
 		echo "${HC}t/t:1:test" >expected &&
 		git grep -n -e test $H >actual &&
-- 
2.18.0

[PATCH v2] grep.c: teach 'git grep --only-matching'

[Taylor Blau]

Teach 'git grep --only-matching', a new option to only print the
matching part(s) of a line.

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 --line-number --column --only-matching -e git -- \
    README.md | grep ":27"
  README.md:27:7:git
  README.md:27:16:git
  README.md:27:38:git

The patch works mostly as one would expect, with the exception of a few
considerations that are worth mentioning here.

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.

Because a line might contain more than one match, there are special
considerations pertaining to when to print line headers, newlines, and
how to increment the match column offset. The line header and newlines
are handled as a special case within the main loop to avoid polluting
the surrounding code with conditionals that have large blocks.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 Documentation/git-grep.txt |  6 ++++-
 builtin/grep.c             |  6 +++++
 grep.c                     | 51 ++++++++++++++++++++++++++------------
 grep.h                     |  1 +
 t/t7810-grep.sh            | 15 +++++++++++
 5 files changed, 62 insertions(+), 17 deletions(-)

diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index 0de3493b80..078b4e3730 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -17,7 +17,7 @@ SYNOPSIS
 	   [-l | --files-with-matches] [-L | --files-without-match]
 	   [(-O | --open-files-in-pager) [<pager>]]
 	   [-z | --null]
-	   [-c | --count] [--all-match] [-q | --quiet]
+	   [ -o | --only-matching ] [-c | --count] [--all-match] [-q | --quiet]
 	   [--max-depth <depth>]
 	   [--color[=<when>] | --no-color]
 	   [--break] [--heading] [-p | --show-function]
@@ -201,6 +201,10 @@ providing this option will cause it to die.
 	Output \0 instead of the character that normally follows a
 	file name.
 
+-o::
+--only-matching::
+	Output only the matching part of the lines.
+
 -c::
 --count::
 	Instead of showing every matched line, show the number of
diff --git a/builtin/grep.c b/builtin/grep.c
index 61bcaf6e58..228b83990f 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -843,6 +843,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		OPT_BOOL_F('z', "null", &opt.null_following_name,
 			   N_("print NUL after filenames"),
 			   PARSE_OPT_NOCOMPLETE),
+		OPT_BOOL('o', "only-matching", &opt.only_matching,
+			N_("show only matching parts of a line")),
 		OPT_BOOL('c', "count", &opt.count,
 			N_("show the number of matches instead of matching lines")),
 		OPT__COLOR(&opt.color, N_("highlight matches")),
@@ -962,6 +964,10 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	if (!opt.pattern_list)
 		die(_("no pattern given."));
 
+	/* --only-matching has no effect with --invert. */
+	if (opt.invert)
+		opt.only_matching = 0;
+
 	/*
 	 * We have to find "--" in a separate pass, because its presence
 	 * influences how we will parse arguments that come before it.
diff --git a/grep.c b/grep.c
index 4ff8a73043..49a744f96b 100644
--- a/grep.c
+++ b/grep.c
@@ -51,6 +51,7 @@ void init_grep_defaults(void)
 	color_set(opt->color_match_selected, GIT_COLOR_BOLD_RED);
 	color_set(opt->color_selected, "");
 	color_set(opt->color_sep, GIT_COLOR_CYAN);
+	opt->only_matching = 0;
 	opt->color = -1;
 	opt->output = std_output;
 }
@@ -158,6 +159,7 @@ void grep_init(struct grep_opt *opt, const char *prefix)
 	opt->pattern_tail = &opt->pattern_list;
 	opt->header_tail = &opt->header_list;
 
+	opt->only_matching = def->only_matching;
 	opt->color = def->color;
 	opt->extended_regexp_option = def->extended_regexp_option;
 	opt->pattern_type_option = def->pattern_type_option;
@@ -1446,7 +1448,8 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 		      const char *name, unsigned lno, ssize_t cno, char sign)
 {
 	int rest = eol - bol;
-	const char *match_color, *line_color = NULL;
+	const char *match_color = NULL;
+	const char *line_color = NULL;
 
 	if (opt->file_break && opt->last_shown == 0) {
 		if (opt->show_hunk_mark)
@@ -1462,39 +1465,55 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 			opt->output(opt, "\n", 1);
 		}
 	}
-	show_line_header(opt, name, lno, cno, sign);
-	if (opt->color) {
+	if (!opt->only_matching) {
+		/*
+		 * In case the line we're being called with contains more than
+		 * one match, leave printing each header to the loop below.
+		 */
+		show_line_header(opt, name, lno, cno, sign);
+	}
+	if (opt->color || opt->only_matching) {
 		regmatch_t match;
 		enum grep_context ctx = GREP_CONTEXT_BODY;
 		int ch = *eol;
 		int eflags = 0;
 
-		if (sign == ':')
-			match_color = opt->color_match_selected;
-		else
-			match_color = opt->color_match_context;
-		if (sign == ':')
-			line_color = opt->color_selected;
-		else if (sign == '-')
-			line_color = opt->color_context;
-		else if (sign == '=')
-			line_color = opt->color_function;
+		if (opt->color) {
+			if (sign == ':')
+				match_color = opt->color_match_selected;
+			else
+				match_color = opt->color_match_context;
+			if (sign == ':')
+				line_color = opt->color_selected;
+			else if (sign == '-')
+				line_color = opt->color_context;
+			else if (sign == '=')
+				line_color = opt->color_function;
+		}
 		*eol = '\0';
 		while (next_match(opt, bol, eol, ctx, &match, eflags)) {
 			if (match.rm_so == match.rm_eo)
 				break;
 
-			output_color(opt, bol, match.rm_so, line_color);
+			if (opt->only_matching)
+				show_line_header(opt, name, lno, cno, sign);
+			else
+				output_color(opt, bol, match.rm_so, line_color);
 			output_color(opt, bol + match.rm_so,
 				     match.rm_eo - match.rm_so, match_color);
+			if (opt->only_matching)
+				opt->output(opt, "\n", 1);
 			bol += match.rm_eo;
+			cno += match.rm_eo;
 			rest -= match.rm_eo;
 			eflags = REG_NOTBOL;
 		}
 		*eol = ch;
 	}
-	output_color(opt, bol, rest, line_color);
-	opt->output(opt, "\n", 1);
+	if (!opt->only_matching) {
+		output_color(opt, bol, rest, line_color);
+		opt->output(opt, "\n", 1);
+	}
 }
 
 #ifndef NO_PTHREADS
diff --git a/grep.h b/grep.h
index 08a0b391c5..4d474d8ec4 100644
--- a/grep.h
+++ b/grep.h
@@ -150,6 +150,7 @@ struct grep_opt {
 	int relative;
 	int pathname;
 	int null_following_name;
+	int only_matching;
 	int color;
 	int max_depth;
 	int funcname;
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 9312c8daf5..d8c232dbf4 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -262,6 +262,21 @@ do
 		fi
 	'
 
+	test_expect_success "grep $L (with --column, --only-matching)" '
+		{
+			echo ${HC}file:1:5:mmap
+			echo ${HC}file:2:5:mmap
+			echo ${HC}file:3:5:mmap
+			echo ${HC}file:3:13:mmap
+			echo ${HC}file:4:5:mmap
+			echo ${HC}file:4:13:mmap
+			echo ${HC}file:5:5:mmap
+			echo ${HC}file:5:13:mmap
+		} >expected &&
+		git grep --column -n -o -e mmap $H >actual &&
+		test_cmp expected actual
+	'
+
 	test_expect_success "grep $L (t-1)" '
 		echo "${HC}t/t:1:test" >expected &&
 		git grep -n -e test $H >actual &&
-- 
2.18.0

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

[Taylor Blau]

On Mon, Jun 25, 2018 at 02:59:07PM -0500, Taylor Blau wrote:
> Teach 'git grep --only-matching', a new option to only print the
> matching part(s) of a line.
>
> [ ... ]
>
> diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
> index 0de3493b80..078b4e3730 100644
> --- a/Documentation/git-grep.txt
> +++ b/Documentation/git-grep.txt
> @@ -17,7 +17,7 @@ SYNOPSIS
>  	   [-l | --files-with-matches] [-L | --files-without-match]
>  	   [(-O | --open-files-in-pager) [<pager>]]
>  	   [-z | --null]
> -	   [-c | --count] [--all-match] [-q | --quiet]
> +	   [ -o | --only-matching ] [-c | --count] [--all-match] [-q | --quiet]
>  	   [--max-depth <depth>]
>  	   [--color[=<when>] | --no-color]
>  	   [--break] [--heading] [-p | --show-function]
> @@ -201,6 +201,10 @@ providing this option will cause it to die.
>  	Output \0 instead of the character that normally follows a
>  	file name.
>
> +-o::
> +--only-matching::
> +	Output only the matching part of the lines.

Junio,

My apologies that I sent the previous patch with incorrect indentation on this
line. Would you mind queueing this one instead?


Thanks,
Taylor

Re: [PATCH v3 0/2] grep.c: teach --only-matching to 'git-grep(1)'

[Jeff King]

On Tue, Jul 03, 2018 at 04:51:52PM -0500, Taylor Blau wrote:

> diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
> index 0de3493b80..be13fc3253 100644
> --- a/Documentation/git-grep.txt
> +++ b/Documentation/git-grep.txt
> @@ -17,7 +17,7 @@ SYNOPSIS
>  	   [-l | --files-with-matches] [-L | --files-without-match]
>  	   [(-O | --open-files-in-pager) [<pager>]]
>  	   [-z | --null]
> -	   [-c | --count] [--all-match] [-q | --quiet]
> +	   [ -o | --only-matching ] [-c | --count] [--all-match] [-q | --quiet]
>  	   [--max-depth <depth>]
>  	   [--color[=<when>] | --no-color]
>  	   [--break] [--heading] [-p | --show-function]
> @@ -201,6 +201,10 @@ providing this option will cause it to die.
>  	Output \0 instead of the character that normally follows a
>  	file name.
> 
> +-o::
> +--only-matching::
> +  Output only the matching part of the lines.
> +

Putting myself into the shoes of a naive reader, I have to wonder what
happens when there are multiple matching parts on the same line. I know
the answer from your commit message, but maybe that should be covered
here? Maybe:

  Output only the matching part of the lines. If there are multiple
  matching parts, each is output on a separate line.

-Peff

Re: [PATCH v3 0/2] grep.c: teach --only-matching to 'git-grep(1)'

[Taylor Blau]

On Thu, Jul 05, 2018 at 10:21:11AM -0400, Jeff King wrote:
> On Tue, Jul 03, 2018 at 04:51:52PM -0500, Taylor Blau wrote:
>
> > diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
> > index 0de3493b80..be13fc3253 100644
> > --- a/Documentation/git-grep.txt
> > +++ b/Documentation/git-grep.txt
> > @@ -17,7 +17,7 @@ SYNOPSIS
> >  	   [-l | --files-with-matches] [-L | --files-without-match]
> >  	   [(-O | --open-files-in-pager) [<pager>]]
> >  	   [-z | --null]
> > -	   [-c | --count] [--all-match] [-q | --quiet]
> > +	   [ -o | --only-matching ] [-c | --count] [--all-match] [-q | --quiet]
> >  	   [--max-depth <depth>]
> >  	   [--color[=<when>] | --no-color]
> >  	   [--break] [--heading] [-p | --show-function]
> > @@ -201,6 +201,10 @@ providing this option will cause it to die.
> >  	Output \0 instead of the character that normally follows a
> >  	file name.
> >
> > +-o::
> > +--only-matching::
> > +  Output only the matching part of the lines.
> > +
>
> Putting myself into the shoes of a naive reader, I have to wonder what
> happens when there are multiple matching parts on the same line. I know
> the answer from your commit message, but maybe that should be covered
> here? Maybe:
>
>   Output only the matching part of the lines. If there are multiple
>   matching parts, each is output on a separate line.

I think that this might be clear enough on its own, especially since
this is the same as BSD grep on my machine. I think that part_s_ of a
line indicates that behavior, but perhaps not. On GNU grep, this is:

  Print only the matched (non-empty) parts of a matching line, with each
  such part on a separate output line.

I'm happy to pick either and re-send this patch (2/2) again, if it
wouldn't be too much to juggle. Otherwise, I can re-roll to v4.


Thanks,
Taylor

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

[Junio C Hamano]

Taylor Blau <me@ttaylorr.com> writes:

> On Mon, Jun 25, 2018 at 02:59:07PM -0500, Taylor Blau wrote:
>> Teach 'git grep --only-matching', a new option to only print the
>> matching part(s) of a line.
>>
>> [ ... ]
>>
>> diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
>> index 0de3493b80..078b4e3730 100644
>> --- a/Documentation/git-grep.txt
>> +++ b/Documentation/git-grep.txt
>> @@ -17,7 +17,7 @@ SYNOPSIS
>>  	   [-l | --files-with-matches] [-L | --files-without-match]
>>  	   [(-O | --open-files-in-pager) [<pager>]]
>>  	   [-z | --null]
>> -	   [-c | --count] [--all-match] [-q | --quiet]
>> +	   [ -o | --only-matching ] [-c | --count] [--all-match] [-q | --quiet]
>>  	   [--max-depth <depth>]
>>  	   [--color[=<when>] | --no-color]
>>  	   [--break] [--heading] [-p | --show-function]
>> @@ -201,6 +201,10 @@ providing this option will cause it to die.
>>  	Output \0 instead of the character that normally follows a
>>  	file name.
>>
>> +-o::
>> +--only-matching::
>> +	Output only the matching part of the lines.
>
> Junio,
>
> My apologies that I sent the previous patch with incorrect indentation on this
> line. Would you mind queueing this one instead?

Surely, and thanks for telling me what difference to look for
between the versions.  Will replace with the one with indent before
"Output only ...".

Re: [PATCH v3 0/2] grep.c: teach --only-matching to 'git-grep(1)'

[Junio C Hamano]

Taylor Blau <me@ttaylorr.com> writes:

> I think that this might be clear enough on its own, especially since
> this is the same as BSD grep on my machine. I think that part_s_ of a
> line indicates that behavior, but perhaps not. On GNU grep, this is:
>
>   Print only the matched (non-empty) parts of a matching line, with each
>   such part on a separate output line.

Interesting.  I wonder what "git grep -o '^'" would do ;-)

> I'm happy to pick either and re-send this patch (2/2) again, if it
> wouldn't be too much to juggle. Otherwise, I can re-roll to v4.

Please do not re-send a different version of a patch with the same
v$n value.  Either re-send, otherwise re-roll, will give us v4, not
v3.

In any case, I find that the GNU phrasing is the most clear among
the ones I've seen in this thread so far.

Re: [PATCH v3 0/2] grep.c: teach --only-matching to 'git-grep(1)'

[Taylor Blau]

On Fri, Jul 06, 2018 at 11:21:06AM -0700, Junio C Hamano wrote:
> Taylor Blau <me@ttaylorr.com> writes:
>
> > I think that this might be clear enough on its own, especially since
> > this is the same as BSD grep on my machine. I think that part_s_ of a
> > line indicates that behavior, but perhaps not. On GNU grep, this is:
> >
> >   Print only the matched (non-empty) parts of a matching line, with each
> >   such part on a separate output line.
>
> Interesting.  I wonder what "git grep -o '^'" would do ;-)

That invocation prints nothing, but on BSD grep it prints quite a few
blank lines :-).

I'm hesitant on sending a patch per the hunk of your reply below because
of this. Should we mirror BSD grep's behavior exactly here? I suppose
that we could somehow, but it seems like we might be doing too much to
support what appears to me to be an odd use-case.

> > I'm happy to pick either and re-send this patch (2/2) again, if it
> > wouldn't be too much to juggle. Otherwise, I can re-roll to v4.
>
> Please do not re-send a different version of a patch with the same
> v$n value.  Either re-send, otherwise re-roll, will give us v4, not
> v3.
>
> In any case, I find that the GNU phrasing is the most clear among
> the ones I've seen in this thread so far.

OK. I'm happy to re-send that patch with the GNU phrasing depending on
what others think (and the above). I'll let this cook and collect some
thoughts over the weekend.


Thanks,
Taylor

Re: [PATCH v3 0/2] grep.c: teach --only-matching to 'git-grep(1)'

[Jeff King]

On Fri, Jul 06, 2018 at 03:15:22PM -0500, Taylor Blau wrote:

> On Fri, Jul 06, 2018 at 11:21:06AM -0700, Junio C Hamano wrote:
> > Taylor Blau <me@ttaylorr.com> writes:
> >
> > > I think that this might be clear enough on its own, especially since
> > > this is the same as BSD grep on my machine. I think that part_s_ of a
> > > line indicates that behavior, but perhaps not. On GNU grep, this is:
> > >
> > >   Print only the matched (non-empty) parts of a matching line, with each
> > >   such part on a separate output line.
> >
> > Interesting.  I wonder what "git grep -o '^'" would do ;-)
> 
> That invocation prints nothing, but on BSD grep it prints quite a few
> blank lines :-).
> 
> I'm hesitant on sending a patch per the hunk of your reply below because
> of this. Should we mirror BSD grep's behavior exactly here? I suppose
> that we could somehow, but it seems like we might be doing too much to
> support what appears to me to be an odd use-case.

IMHO the GNU behavior (omitting non-empty matches) makes more sense. And
it's also what your patch already does. ;)

Although amusingly "git grep -o ^" will still print a ton of "Binary
file ... matches". That _also_ matches what GNU grep does. I'm not sure
if there's a saner behavior (it really has nothing to do with the funny
empty match; any binary file with -o cannot show the normal text line).

> > In any case, I find that the GNU phrasing is the most clear among
> > the ones I've seen in this thread so far.
> 
> OK. I'm happy to re-send that patch with the GNU phrasing depending on
> what others think (and the above). I'll let this cook and collect some
> thoughts over the weekend.

FWIW, I like the GNU phrasing. I thought the "non-empty" part was not
all that interesting, but after hearing that BSD behaves differently, it
is probably worth mentioning.

I think the actual behavior of your patch matches GNU grep, and does not
need changing.

-Peff

Re: [PATCH v3 0/2] grep.c: teach --only-matching to 'git-grep(1)'

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> FWIW, I like the GNU phrasing. I thought the "non-empty" part was not
> all that interesting, but after hearing that BSD behaves differently, it
> is probably worth mentioning.
>
> I think the actual behavior of your patch matches GNU grep, and does not
> need changing.

Great ;-)

[PATCH v4] grep.c: teach 'git grep --only-matching'

[Taylor Blau]

Teach 'git grep --only-matching', a new option to only print the
matching part(s) of a line.

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 --line-number --column --only-matching -e git -- \
    README.md | grep ":27"
  README.md:27:7:git
  README.md:27:16:git
  README.md:27:38:git

The patch works mostly as one would expect, with the exception of a few
considerations that are worth mentioning here.

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.

Because a line might contain more than one match, there are special
considerations pertaining to when to print line headers, newlines, and
how to increment the match column offset. The line header and newlines
are handled as a special case within the main loop to avoid polluting
the surrounding code with conditionals that have large blocks.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 Documentation/git-grep.txt |  7 +++++-
 builtin/grep.c             |  6 +++++
 grep.c                     | 51 ++++++++++++++++++++++++++------------
 grep.h                     |  1 +
 t/t7810-grep.sh            | 15 +++++++++++
 5 files changed, 63 insertions(+), 17 deletions(-)

diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index 0de3493b80..a3049af1a3 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -17,7 +17,7 @@ SYNOPSIS
 	   [-l | --files-with-matches] [-L | --files-without-match]
 	   [(-O | --open-files-in-pager) [<pager>]]
 	   [-z | --null]
-	   [-c | --count] [--all-match] [-q | --quiet]
+	   [ -o | --only-matching ] [-c | --count] [--all-match] [-q | --quiet]
 	   [--max-depth <depth>]
 	   [--color[=<when>] | --no-color]
 	   [--break] [--heading] [-p | --show-function]
@@ -201,6 +201,11 @@ providing this option will cause it to die.
 	Output \0 instead of the character that normally follows a
 	file name.
 
+-o::
+--only-matching::
+	Print only the matched (non-empty) parts of a matching line, with each such
+	part on a separate output line.
+
 -c::
 --count::
 	Instead of showing every matched line, show the number of
diff --git a/builtin/grep.c b/builtin/grep.c
index 61bcaf6e58..228b83990f 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -843,6 +843,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		OPT_BOOL_F('z', "null", &opt.null_following_name,
 			   N_("print NUL after filenames"),
 			   PARSE_OPT_NOCOMPLETE),
+		OPT_BOOL('o', "only-matching", &opt.only_matching,
+			N_("show only matching parts of a line")),
 		OPT_BOOL('c', "count", &opt.count,
 			N_("show the number of matches instead of matching lines")),
 		OPT__COLOR(&opt.color, N_("highlight matches")),
@@ -962,6 +964,10 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	if (!opt.pattern_list)
 		die(_("no pattern given."));
 
+	/* --only-matching has no effect with --invert. */
+	if (opt.invert)
+		opt.only_matching = 0;
+
 	/*
 	 * We have to find "--" in a separate pass, because its presence
 	 * influences how we will parse arguments that come before it.
diff --git a/grep.c b/grep.c
index 4ff8a73043..49a744f96b 100644
--- a/grep.c
+++ b/grep.c
@@ -51,6 +51,7 @@ void init_grep_defaults(void)
 	color_set(opt->color_match_selected, GIT_COLOR_BOLD_RED);
 	color_set(opt->color_selected, "");
 	color_set(opt->color_sep, GIT_COLOR_CYAN);
+	opt->only_matching = 0;
 	opt->color = -1;
 	opt->output = std_output;
 }
@@ -158,6 +159,7 @@ void grep_init(struct grep_opt *opt, const char *prefix)
 	opt->pattern_tail = &opt->pattern_list;
 	opt->header_tail = &opt->header_list;
 
+	opt->only_matching = def->only_matching;
 	opt->color = def->color;
 	opt->extended_regexp_option = def->extended_regexp_option;
 	opt->pattern_type_option = def->pattern_type_option;
@@ -1446,7 +1448,8 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 		      const char *name, unsigned lno, ssize_t cno, char sign)
 {
 	int rest = eol - bol;
-	const char *match_color, *line_color = NULL;
+	const char *match_color = NULL;
+	const char *line_color = NULL;
 
 	if (opt->file_break && opt->last_shown == 0) {
 		if (opt->show_hunk_mark)
@@ -1462,39 +1465,55 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 			opt->output(opt, "\n", 1);
 		}
 	}
-	show_line_header(opt, name, lno, cno, sign);
-	if (opt->color) {
+	if (!opt->only_matching) {
+		/*
+		 * In case the line we're being called with contains more than
+		 * one match, leave printing each header to the loop below.
+		 */
+		show_line_header(opt, name, lno, cno, sign);
+	}
+	if (opt->color || opt->only_matching) {
 		regmatch_t match;
 		enum grep_context ctx = GREP_CONTEXT_BODY;
 		int ch = *eol;
 		int eflags = 0;
 
-		if (sign == ':')
-			match_color = opt->color_match_selected;
-		else
-			match_color = opt->color_match_context;
-		if (sign == ':')
-			line_color = opt->color_selected;
-		else if (sign == '-')
-			line_color = opt->color_context;
-		else if (sign == '=')
-			line_color = opt->color_function;
+		if (opt->color) {
+			if (sign == ':')
+				match_color = opt->color_match_selected;
+			else
+				match_color = opt->color_match_context;
+			if (sign == ':')
+				line_color = opt->color_selected;
+			else if (sign == '-')
+				line_color = opt->color_context;
+			else if (sign == '=')
+				line_color = opt->color_function;
+		}
 		*eol = '\0';
 		while (next_match(opt, bol, eol, ctx, &match, eflags)) {
 			if (match.rm_so == match.rm_eo)
 				break;
 
-			output_color(opt, bol, match.rm_so, line_color);
+			if (opt->only_matching)
+				show_line_header(opt, name, lno, cno, sign);
+			else
+				output_color(opt, bol, match.rm_so, line_color);
 			output_color(opt, bol + match.rm_so,
 				     match.rm_eo - match.rm_so, match_color);
+			if (opt->only_matching)
+				opt->output(opt, "\n", 1);
 			bol += match.rm_eo;
+			cno += match.rm_eo;
 			rest -= match.rm_eo;
 			eflags = REG_NOTBOL;
 		}
 		*eol = ch;
 	}
-	output_color(opt, bol, rest, line_color);
-	opt->output(opt, "\n", 1);
+	if (!opt->only_matching) {
+		output_color(opt, bol, rest, line_color);
+		opt->output(opt, "\n", 1);
+	}
 }
 
 #ifndef NO_PTHREADS
diff --git a/grep.h b/grep.h
index 08a0b391c5..4d474d8ec4 100644
--- a/grep.h
+++ b/grep.h
@@ -150,6 +150,7 @@ struct grep_opt {
 	int relative;
 	int pathname;
 	int null_following_name;
+	int only_matching;
 	int color;
 	int max_depth;
 	int funcname;
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 9312c8daf5..d8c232dbf4 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -262,6 +262,21 @@ do
 		fi
 	'
 
+	test_expect_success "grep $L (with --column, --only-matching)" '
+		{
+			echo ${HC}file:1:5:mmap
+			echo ${HC}file:2:5:mmap
+			echo ${HC}file:3:5:mmap
+			echo ${HC}file:3:13:mmap
+			echo ${HC}file:4:5:mmap
+			echo ${HC}file:4:13:mmap
+			echo ${HC}file:5:5:mmap
+			echo ${HC}file:5:13:mmap
+		} >expected &&
+		git grep --column -n -o -e mmap $H >actual &&
+		test_cmp expected actual
+	'
+
 	test_expect_success "grep $L (t-1)" '
 		echo "${HC}t/t:1:test" >expected &&
 		git grep -n -e test $H >actual &&
-- 
2.18.0

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

[Taylor Blau]

On Mon, Jul 09, 2018 at 03:33:47PM -0500, Taylor Blau wrote:
> [ ... ]
> ---
>  Documentation/git-grep.txt |  7 +++++-
>  builtin/grep.c             |  6 +++++
>  grep.c                     | 51 ++++++++++++++++++++++++++------------
>  grep.h                     |  1 +
>  t/t7810-grep.sh            | 15 +++++++++++
>  5 files changed, 63 insertions(+), 17 deletions(-)
>
> diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
> index 0de3493b80..a3049af1a3 100644
> --- a/Documentation/git-grep.txt
> +++ b/Documentation/git-grep.txt
> @@ -17,7 +17,7 @@ SYNOPSIS
>  	   [-l | --files-with-matches] [-L | --files-without-match]
>  	   [(-O | --open-files-in-pager) [<pager>]]
>  	   [-z | --null]
> -	   [-c | --count] [--all-match] [-q | --quiet]
> +	   [ -o | --only-matching ] [-c | --count] [--all-match] [-q | --quiet]
>  	   [--max-depth <depth>]
>  	   [--color[=<when>] | --no-color]
>  	   [--break] [--heading] [-p | --show-function]
> @@ -201,6 +201,11 @@ providing this option will cause it to die.
>  	Output \0 instead of the character that normally follows a
>  	file name.
>
> +-o::
> +--only-matching::
> +	Print only the matched (non-empty) parts of a matching line, with each such
> +	part on a separate output line.

OK, it seems as if the consensus is (1) take the description as-is from
GNU grep, and (2) don't change the existing behavior of "git grep -o
'^'".

This patch does both of those things, and can be queued as 2/2 in this
series.

Thanks, everybody :-).

> --
> 2.18.0

Thanks,
Taylor