Subject: [PATCH v6 0/7] Teach '--column' to 'git-grep(1)'

From: Taylor Blau <[email-masked]>
Date: Fri, 11 May 2018 20:10:57 -0700

Hi,

Attached is my sixth re-roll of a series to add '--column' to 'git
grep'.

The main change since v5 is supporting --column with queries containing
--and, --or, or --not. Previously, I had chosen to die() in this case
since there isn't always a good answer to "what is the first column of
<complicated expression>?" but have gone back on this for two reasons:

  1. It is important not to regress calls to git-jump/contrib/git-jump
  that contain --and, --or, or --not.

  2. It is not that hard to detect the absence of column data in scripts.
     Likewise, git-jump will happily accept lines with or without
     columnar information, and Vim will accept it as-is.

So, let's support --column and only die() when also given
--invert-match. When we don't have a good answer, print nothing.

Thanks,
Taylor

Taylor Blau (7):
  Documentation/config.txt: camel-case lineNumber for consistency
  grep.c: expose matched 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 match column in addition to line

 Documentation/config.txt   |  7 ++++++-
 Documentation/git-grep.txt | 10 +++++++++-
 builtin/grep.c             |  4 ++++
 contrib/git-jump/README    | 12 ++++++++++--
 contrib/git-jump/git-jump  |  2 +-
 grep.c                     | 40 +++++++++++++++++++++++++++++---------
 grep.h                     |  2 ++
 t/t7810-grep.sh            | 39 +++++++++++++++++++++++++++++++++++++
 8 files changed, 102 insertions(+), 14 deletions(-)

Inter-diff (since v5):

diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index dc8f76ce99..c48a578cb1 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -173,8 +173,9 @@ providing this option will cause it to die.
 	Prefix the line number to matching lines.

 --column::
-	Prefix the 1-indexed byte-offset of the first match on non-context lines. This
-	option is incompatible with '--invert-match', and extended expressions.
+	Prefix the 1-indexed byte-offset of the first match from the start of the
+	matching line. This option is incompatible with '--invert-match', and
+	ignored with expressions using '--and', '--or', '--not'.

 -l::
 --files-with-matches::
diff --git a/grep.c b/grep.c
index 5d904810ad..5ba1b05526 100644
--- a/grep.c
+++ b/grep.c
@@ -1001,9 +1001,6 @@ static void compile_grep_patterns_real(struct grep_opt *opt)
 	else if (!opt->extended && !opt->debug)
 		return;

-	if (opt->columnnum && opt->extended)
-		die(_("--column and extended expressions cannot be combined"));
-
 	p = opt->pattern_list;
 	if (p)
 		opt->pattern_expression = compile_pattern_expr(&p);
@@ -1411,9 +1408,10 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 	/*
 	 * Treat 'cno' as the 1-indexed offset from the start of a non-context
 	 * line to its first match. Otherwise, 'cno' is 0 indicating that we are
-	 * being called with a context line.
+	 * being called with a context line, or we are --extended, and cannot
+	 * always show an answer.
 	 */
-	if (opt->columnnum && cno) {
+	if (opt->columnnum && sign == ':' && !opt->extended) {
 		char buf[32];
 		xsnprintf(buf, sizeof(buf), "%d", cno);
 		output_color(opt, buf, strlen(buf), opt->color_columnno);
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index aa56b21ed9..491b2e044a 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -110,6 +110,18 @@ do
 		test_cmp expected actual
 	'

+	test_expect_success "grep -w $L (with --column, -C)" '
+		{
+			echo ${HC}file:5:foo mmap bar
+			echo ${HC}file-foo_mmap bar
+			echo ${HC}file:14:foo_mmap bar mmap
+			echo ${HC}file:5:foo mmap bar_mmap
+			echo ${HC}file:14:foo_mmap bar mmap baz
+		} >expected &&
+		git grep --column -w -C1 -e mmap $H >actual &&
+		test_cmp expected actual
+	'
+
 	test_expect_success "grep -w $L (with --line-number, --column)" '
 		{
 			echo ${HC}file:1:5:foo mmap bar
@@ -1617,9 +1629,4 @@ test_expect_success 'grep does not allow --column, --invert-match' '
 	test_i18ngrep "\-\-column and \-\-invert-match cannot be combined" err
 '

-test_expect_success 'grep does not allow --column, extended' '
-	test_must_fail git grep --column --not -e pat 2>err &&
-	test_i18ngrep "\-\-column and extended expressions cannot be combined" err
-'
-
 test_done

--
2.17.0