grep.c: teach --only-matching to 'git-grep(1)'

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/[email protected]/

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

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.

---
 grep.c | 44 +++++++++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 19 deletions(-)

index 992673fe7e..4ff8a73043 100644
@@ -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

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.

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

index 61bcaf6e58..228b83990f 100644
@@ -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.

index 08a0b391c5..4d474d8ec4 100644
@@ -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;

index 9312c8daf5..d8c232dbf4 100755
@@ -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

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.

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

index 48cca6723e..b985fb3ee0 100644
@@ -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

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

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

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.

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

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/[email protected]/

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

index 48cca6723e..49a744f96b 100644
@@ -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

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.

---
 grep.c | 44 +++++++++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 19 deletions(-)

index 992673fe7e..4ff8a73043 100644
@@ -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

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.

---
 builtin/grep.c  |  6 ++++++
 grep.c          | 51 +++++++++++++++++++++++++++++++++----------------
 grep.h          |  1 +
 t/t7810-grep.sh | 15 +++++++++++++++
 4 files changed, 57 insertions(+), 16 deletions(-)

index 61bcaf6e58..228b83990f 100644
@@ -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.

index 08a0b391c5..4d474d8ec4 100644
@@ -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;

index 9312c8daf5..d8c232dbf4 100755
@@ -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

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

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

-Peff

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

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/[email protected]/
[2]: https://public-inbox.org/git/[email protected]/

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

index 0de3493b80..be13fc3253 100644
@@ -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

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.

---
 grep.c | 44 +++++++++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 19 deletions(-)

index 992673fe7e..4ff8a73043 100644
@@ -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

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.

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

index 0de3493b80..be13fc3253 100644
@@ -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

index 61bcaf6e58..228b83990f 100644
@@ -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.

index 08a0b391c5..4d474d8ec4 100644
@@ -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;

index 9312c8daf5..d8c232dbf4 100755
@@ -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

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.

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

index 0de3493b80..078b4e3730 100644
@@ -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

index 61bcaf6e58..228b83990f 100644
@@ -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.

index 08a0b391c5..4d474d8ec4 100644
@@ -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;

index 9312c8daf5..d8c232dbf4 100755
@@ -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

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

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

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

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.

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.

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

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

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

Great ;-)

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.

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

index 0de3493b80..a3049af1a3 100644
@@ -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

index 61bcaf6e58..228b83990f 100644
@@ -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.

index 08a0b391c5..4d474d8ec4 100644
@@ -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;

index 9312c8daf5..d8c232dbf4 100755
@@ -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

Thanks,
Taylor