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):
Show changes to 2 files +66 −25
grep.c, t/t7810-grep.sh
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
Taylor Blau
