git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 0/7] grep.c: teach --column to 'git-grep(1)'

From
Taylor Blau <me@ttaylorr.com>
Date
Jun 20, 2018, 20:05 UTC
Message-ID
<cover.1529524852.git.me@ttaylorr.com>
In-Reply-To
<cover.1529365072.git.me@ttaylorr.com>
Hi,

Here is a re-roll of my series to add --column to 'git-grep(1)'. Since last time, not much has changed other than the following:

  - Fix a typo where 'col', 'icol' were spelled as 'match', 'imatch'
    [1].
  - Disable short-circuiting OR when --column is given [2].
  - Disable early-return in match_line() when multiple patterns are
    given and --column is, too [3].
  - Add some more tests in t7810.

Thanks again for your kind and through review; hopefully this re-roll should be OK to queue as-is.

Thanks, Taylor

[1]: https://public-inbox.org/git/xmqqwouuvi0e.fsf@gitster-ct.c.googlers.com/ [2]: https://public-inbox.org/git/20180619174452.GA47272@syl.attlocal.net/ [3]: https://public-inbox.org/git/80b9a0b1-3849-7097-fe1a-dd80835d62ae@web.de/

Taylor Blau (7):
  Documentation/config.txt: camel-case lineNumber for consistency
  grep.c: expose {,inverted} match column in match_line()
  grep.[ch]: extend grep_opt to allow showing matched column
  grep.c: display column number of first match
  builtin/grep.c: add '--column' option to 'git-grep(1)'
  grep.c: add configuration variables to show matched option
  contrib/git-jump/git-jump: jump to exact location
 Documentation/config.txt   |   7 ++-
 Documentation/git-grep.txt |   9 ++-
 builtin/grep.c             |   1 +
 contrib/git-jump/README    |  12 +++-
 contrib/git-jump/git-jump  |   2 +-
 grep.c                     | 126 ++++++++++++++++++++++++++++---------
 grep.h                     |   2 +
 t/t7810-grep.sh            |  84 +++++++++++++++++++++++++
 8 files changed, 210 insertions(+), 33 deletions(-)
Inter-diff (since v1):
diff --git a/grep.c b/grep.c
index 8ffa94c791..08d3df2855 100644
--- a/grep.c
+++ b/grep.c
@@ -1257,8 +1257,8 @@ static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,
 	return hit;
 }

-static int match_expr_eval(struct grep_expr *x, char *bol, char *eol,
-			   enum grep_context ctx, ssize_t *col,
+static int match_expr_eval(struct grep_opt *opt, struct grep_expr *x, char *bol,
+			   char *eol, enum grep_context ctx, ssize_t *col,
 			   ssize_t *icol, int collect_hits)
 {
 	int h = 0;
@@ -1280,29 +1280,36 @@ static int match_expr_eval(struct grep_expr *x, char *bol, char *eol,
 		break;
 	case GREP_NODE_NOT:
 		/*
-		 * Upon visiting a GREP_NODE_NOT, imatch and match become
-		 * swapped.
+		 * Upon visiting a GREP_NODE_NOT, col and icol become swapped.
 		 */
-		h = !match_expr_eval(x->u.unary, bol, eol, ctx, icol, col, 0);
+		h = !match_expr_eval(opt, x->u.unary, bol, eol, ctx, icol, col,
+				     0);
 		break;
 	case GREP_NODE_AND:
-		if (!match_expr_eval(x->u.binary.left, bol, eol, ctx, col,
+		if (!match_expr_eval(opt, x->u.binary.left, bol, eol, ctx, col,
 				     icol, 0))
 			return 0;
-		h = match_expr_eval(x->u.binary.right, bol, eol, ctx, col,
+		h = match_expr_eval(opt, x->u.binary.right, bol, eol, ctx, col,
 				    icol, 0);
 		break;
 	case GREP_NODE_OR:
-		if (!collect_hits)
-			return (match_expr_eval(x->u.binary.left, bol, eol, ctx,
-						col, icol, 0) ||
-				match_expr_eval(x->u.binary.right, bol, eol,
-						ctx, col, icol, 0));
-		h = match_expr_eval(x->u.binary.left, bol, eol, ctx, col,
+		if (!(collect_hits || opt->columnnum)) {
+			/*
+			 * Don't short-circuit OR when given --column (or
+			 * collecting hits) to ensure we don't skip a later
+			 * child that would produce an earlier match.
+			 */
+			return (match_expr_eval(opt, x->u.binary.left, bol, eol,
+						ctx, col, icol, 0) ||
+				match_expr_eval(opt, x->u.binary.right, bol,
+						eol, ctx, col, icol, 0));
+		}
+		h = match_expr_eval(opt, x->u.binary.left, bol, eol, ctx, col,
 				    icol, 0);
-		x->u.binary.left->hit |= h;
-		h |= match_expr_eval(x->u.binary.right, bol, eol, ctx, col,
-				     icol, 1);
+		if (collect_hits)
+			x->u.binary.left->hit |= h;
+		h |= match_expr_eval(opt, x->u.binary.right, bol, eol, ctx, col,
+				     icol, collect_hits);
 		break;
 	default:
 		die("Unexpected node type (internal error) %d", x->node);
@@ -1317,7 +1324,7 @@ static int match_expr(struct grep_opt *opt, char *bol, char *eol,
 		      ssize_t *icol, int collect_hits)
 {
 	struct grep_expr *x = opt->pattern_expression;
-	return match_expr_eval(x, bol, eol, ctx, col, icol, collect_hits);
+	return match_expr_eval(opt, x, bol, eol, ctx, col, icol, collect_hits);
 }

 static int match_line(struct grep_opt *opt, char *bol, char *eol,
@@ -1325,6 +1332,7 @@ static int match_line(struct grep_opt *opt, char *bol, char *eol,
 		      enum grep_context ctx, int collect_hits)
 {
 	struct grep_pat *p;
+	int hit = 0;

 	if (opt->extended)
 		return match_expr(opt, bol, eol, ctx, col, icol,
@@ -1334,11 +1342,21 @@ static int match_line(struct grep_opt *opt, char *bol, char *eol,
 	for (p = opt->pattern_list; p; p = p->next) {
 		regmatch_t tmp;
 		if (match_one_pattern(p, bol, eol, ctx, &tmp, 0)) {
-			*col = tmp.rm_so;
-			return 1;
+			hit |= 1;
+			if (!opt->columnnum) {
+				/*
+				 * Without --column, any single match on a line
+				 * is enough to know that it needs to be
+				 * printed. With --column, scan _all_ patterns
+				 * to find the earliest.
+				 */
+				break;
+			}
+			if (*col < 0 || tmp.rm_so < *col)
+				*col = tmp.rm_so;
 		}
 	}
-	return 0;
+	return hit;
 }

 static int match_next_pattern(struct grep_pat *p, char *bol, char *eol,
@@ -1387,7 +1405,7 @@ static int next_match(struct grep_opt *opt, char *bol, char *eol,
 }

 static void show_line(struct grep_opt *opt, char *bol, char *eol,
-		      const char *name, unsigned lno, unsigned cno, char sign)
+		      const char *name, unsigned lno, ssize_t cno, char sign)
 {
 	int rest = eol - bol;
 	const char *match_color, *line_color = NULL;
@@ -1429,7 +1447,7 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 	 */
 	if (opt->columnnum && cno) {
 		char buf[32];
-		xsnprintf(buf, sizeof(buf), "%d", cno);
+		xsnprintf(buf, sizeof(buf), "%zu", cno);
 		output_color(opt, buf, strlen(buf), opt->color_columnno);
 		output_sep(opt, sign);
 	}
@@ -1871,8 +1889,11 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle
 			cno = opt->invert ? icol : col;
 			if (cno < 0) {
 				/*
-				 * A negative cno means that there was no match.
-				 * Clamp to the beginning of the line.
+				 * A negative cno indicates that there was no
+				 * match on the line. We are thus inverted and
+				 * being asked to show all lines that _don't_
+				 * match a given expression. Therefore, set cno
+				 * to 0 to suggest the whole line matches.
 				 */
 				cno = 0;
 			}
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index daaf7b4c44..bf0b572dab 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -139,6 +139,15 @@ do
 		test_cmp expected actual
 	'

+	test_expect_success "grep $L (with --column, double-negation)" '
+		{
+			echo ${HC}file:1:foo_mmap bar mmap baz
+		} >expected &&
+		git grep --column --not \( --not -e foo --or --not -e baz \) $H -- file \
+			>actual &&
+		test_cmp expected actual
+	'
+
 	test_expect_success "grep -w $L (with --column, -C)" '
 		{
 			echo ${HC}file:5:foo mmap bar
@@ -162,6 +171,18 @@ do
 		test_cmp expected actual
 	'

+	test_expect_success "grep -w $L (with non-extended patterns, --column)" '
+		{
+			echo ${HC}file:5:foo mmap bar
+			echo ${HC}file:10:foo_mmap bar
+			echo ${HC}file:10:foo_mmap bar mmap
+			echo ${HC}file:5:foo mmap bar_mmap
+			echo ${HC}file:10:foo_mmap bar mmap baz
+		} >expected &&
+		git grep --column -w -e bar -e mmap $H >actual &&
+		test_cmp expected actual
+	'
+
 	test_expect_success "grep -w $L" '
 		{
 			echo ${HC}file:1:foo mmap bar
--
2.17.0.582.gccdcbd54c
Previous: Taylor BlauNext: Taylor Blau
Message 31 of 56 in “grep.c: teach --column to 'git-grep(1)'”
  1. 0/7 grep.c: teach --column to 'git-grep(1)'Taylor Blau, Jun 18, 2018
  2. 1/7 Documentation/config.txt: camel-case lineNumber for consistencyTaylor Blau, Jun 18, 2018
  3. 2/7 grep.c: expose {,inverted} match column in match_line()Taylor Blau, Jun 18, 2018
  4. Junio C HamanoJun 19, 2018
  5. Taylor BlauJun 19, 2018
  6. 3/7 grep.[ch]: extend grep_opt to allow showing matched columnTaylor Blau, Jun 18, 2018
  7. 4/7 grep.c: display column number of first matchTaylor Blau, Jun 18, 2018
  8. Jeff KingJun 19, 2018
  9. Taylor BlauJun 19, 2018
  10. 5/7 builtin/grep.c: add '--column' option to 'git-grep(1)'Taylor Blau, Jun 18, 2018
  11. 6/7 grep.c: add configuration variables to show matched optionTaylor Blau, Jun 18, 2018
  12. 7/7 contrib/git-jump/git-jump: jump to exact locationTaylor Blau, Jun 18, 2018
  13. Jeff KingJun 19, 2018
  14. René ScharfeJun 19, 2018
  15. Taylor BlauJun 19, 2018
  16. René ScharfeJun 19, 2018
  17. René ScharfeJun 19, 2018
  18. Jeff KingJun 19, 2018
  19. Taylor BlauJun 19, 2018
  20. Junio C HamanoJun 19, 2018
  21. Taylor BlauJun 19, 2018
  22. Jeff KingJun 19, 2018
  23. Junio C HamanoJun 19, 2018
  24. René ScharfeJun 19, 2018
  25. Jeff KingJun 19, 2018
  26. René ScharfeJun 19, 2018
  27. Junio C HamanoJun 19, 2018
  28. Junio C HamanoJun 19, 2018
  29. Taylor BlauJun 19, 2018
  30. Taylor BlauJun 19, 2018
  31. 0/7 grep.c: teach --column to 'git-grep(1)'Taylor Blau, Jun 20, 2018
  32. 1/7 Documentation/config.txt: camel-case lineNumber for consistencyTaylor Blau, Jun 20, 2018
  33. 2/7 grep.c: expose {,inverted} match column in match_line()Taylor Blau, Jun 20, 2018
  34. 3/7 grep.[ch]: extend grep_opt to allow showing matched columnTaylor Blau, Jun 20, 2018
  35. 4/7 grep.c: display column number of first matchTaylor Blau, Jun 20, 2018
  36. 6/7 grep.c: add configuration variables to show matched optionTaylor Blau, Jun 20, 2018
  37. 7/7 contrib/git-jump/git-jump: jump to exact locationTaylor Blau, Jun 20, 2018
  38. 5/7 builtin/grep.c: add '--column' option to 'git-grep(1)'Taylor Blau, Jun 20, 2018
  39. Jeff KingJun 21, 2018
  40. Jeff KingJun 21, 2018
  41. Johannes SchindelinJun 22, 2018
  42. Jeff KingJun 22, 2018
  43. Junio C HamanoJun 21, 2018
  44. Taylor BlauJun 21, 2018
  45. Jeff KingJun 22, 2018
  46. 0/7 grep.c: teach --column to 'git-grep(1)'Taylor Blau, Jun 22, 2018
  47. 1/7 Documentation/config.txt: camel-case lineNumber for consistencyTaylor Blau, Jun 22, 2018
  48. 2/7 grep.c: expose {,inverted} match column in match_line()Taylor Blau, Jun 22, 2018
  49. 3/7 grep.[ch]: extend grep_opt to allow showing matched columnTaylor Blau, Jun 22, 2018
  50. 4/7 grep.c: display column number of first matchTaylor Blau, Jun 22, 2018
  51. 5/7 builtin/grep.c: add '--column' option to 'git-grep(1)'Taylor Blau, Jun 22, 2018
  52. 7/7 contrib/git-jump/git-jump: jump to exact locationTaylor Blau, Jun 22, 2018
  53. 6/7 grep.c: add configuration variables to show matched optionTaylor Blau, Jun 22, 2018
  54. Jeff KingJun 25, 2018
  55. Taylor BlauJun 25, 2018
  56. Junio C HamanoJun 26, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.