[PATCH 0/6] grep: add option -p/--show-function, similar to diff's

[PATCH 0/6] grep: add option -p/--show-function, similar to diff's

[René Scharfe]

This patch series retrofits git grep with functionality similar to the diff
option -p, which shows the function definitions for diff hunks.  git grep is
enhanced to show them as context lines before matches, if called with -p.  It
can be combined with the existing context options (-A, -B, -C).

The first patch is a cleanup.  The userdiff API is enhanced to allow freeing
of allocated memory, before it is used outside of git diff code, later in the
series:

  [PATCH 1/6] userdiff: add xdiff_clear_find_func()

The next three patches massage the context handling code to make it easier to
extend:

  [PATCH 2/6] grep: move context hunk mark handling into show_line()
  [PATCH 3/6] grep: print context hunk marks between files
  [PATCH 4/6] grep: handle pre context lines on demand

Finally, handling for the option -p is added and then enhanced to support
userdiff configuration settings:

  [PATCH 5/6] grep: add option -p/--show-function
  [PATCH 6/6] grep -p: support user defined regular expressions


 Documentation/git-grep.txt |    8 ++
 builtin-grep.c             |   22 ++++++-
 diff.c                     |    1 +
 grep.c                     |  154 ++++++++++++++++++++++++++++++++------------
 grep.h                     |    4 +
 t/t7002-grep.sh            |   71 ++++++++++++++++++++-
 xdiff-interface.c          |   15 ++++
 xdiff-interface.h          |    1 +
 8 files changed, 232 insertions(+), 44 deletions(-)

[PATCH 1/6] userdiff: add xdiff_clear_find_func()

[René Scharfe]

xdiff_set_find_func() is used to set user defined regular expressions
for finding function signatures.  Add xdiff_clear_find_func(), which
frees the memory allocated by the former, making the API complete.

Also, use the new function in diff.c (the only call site of
xdiff_set_find_func()) to clean up after ourselves.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 diff.c            |    1 +
 xdiff-interface.c |   15 +++++++++++++++
 xdiff-interface.h |    1 +
 3 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/diff.c b/diff.c
index 43835d7..892921c 100644
--- a/diff.c
+++ b/diff.c
@@ -1603,6 +1603,7 @@ static void builtin_diff(const char *name_a,
 			free(mf1.ptr);
 		if (textconv_two)
 			free(mf2.ptr);
+		xdiff_clear_find_func(&xecfg);
 	}
 
  free_ab_and_return:
diff --git a/xdiff-interface.c b/xdiff-interface.c
index b9b0db8..01f14fb 100644
--- a/xdiff-interface.c
+++ b/xdiff-interface.c
@@ -309,6 +309,21 @@ void xdiff_set_find_func(xdemitconf_t *xecfg, const char *value, int cflags)
 	}
 }
 
+void xdiff_clear_find_func(xdemitconf_t *xecfg)
+{
+	if (xecfg->find_func) {
+		int i;
+		struct ff_regs *regs = xecfg->find_func_priv;
+
+		for (i = 0; i < regs->nr; i++)
+			regfree(&regs->array[i].re);
+		free(regs->array);
+		free(regs);
+		xecfg->find_func = NULL;
+		xecfg->find_func_priv = NULL;
+	}
+}
+
 int git_xmerge_style = -1;
 
 int git_xmerge_config(const char *var, const char *value, void *cb)
diff --git a/xdiff-interface.h b/xdiff-interface.h
index 7352b9a..55572c3 100644
--- a/xdiff-interface.h
+++ b/xdiff-interface.h
@@ -21,6 +21,7 @@ int read_mmfile(mmfile_t *ptr, const char *filename);
 int buffer_is_binary(const char *ptr, unsigned long size);
 
 extern void xdiff_set_find_func(xdemitconf_t *xecfg, const char *line, int cflags);
+extern void xdiff_clear_find_func(xdemitconf_t *xecfg);
 extern int git_xmerge_config(const char *var, const char *value, void *cb);
 extern int git_xmerge_style;
 
-- 
1.6.3.3

[PATCH 2/6] grep: move context hunk mark handling into show_line()

[René Scharfe]

Move last_shown into struct grep_opt, to make it available in
show_line(), and then make the function handle the printing of hunk
marks for context lines in a central place.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 grep.c |   26 +++++++++++---------------
 grep.h |    1 +
 2 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/grep.c b/grep.c
index 92a47c7..6ee80f7 100644
--- a/grep.c
+++ b/grep.c
@@ -490,6 +490,12 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 {
 	int rest = eol - bol;
 
+	if (opt->pre_context || opt->post_context) {
+		if (opt->last_shown && lno > opt->last_shown + 1)
+			fputs("--\n", stdout);
+	}
+	opt->last_shown = lno;
+
 	if (opt->null_following_name)
 		sign = '\0';
 	if (opt->pathname)
@@ -531,12 +537,12 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
 		char *eol;
 	} *prev = NULL, *pcl;
 	unsigned last_hit = 0;
-	unsigned last_shown = 0;
 	int binary_match_only = 0;
-	const char *hunk_mark = "";
 	unsigned count = 0;
 	enum grep_context ctx = GREP_CONTEXT_HEAD;
 
+	opt->last_shown = 0;
+
 	if (buffer_is_binary(buf, size)) {
 		switch (opt->binary) {
 		case GREP_BINARY_DEFAULT:
@@ -552,8 +558,6 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
 
 	if (opt->pre_context)
 		prev = xcalloc(opt->pre_context, sizeof(*prev));
-	if (opt->pre_context || opt->post_context)
-		hunk_mark = "--\n";
 
 	while (left) {
 		char *eol, ch;
@@ -607,33 +611,25 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
 					from = lno - opt->pre_context;
 				else
 					from = 1;
-				if (from <= last_shown)
-					from = last_shown + 1;
-				if (last_shown && from != last_shown + 1)
-					fputs(hunk_mark, stdout);
+				if (from <= opt->last_shown)
+					from = opt->last_shown + 1;
 				while (from < lno) {
 					pcl = &prev[lno-from-1];
 					show_line(opt, pcl->bol, pcl->eol,
 						  name, from, '-');
 					from++;
 				}
-				last_shown = lno-1;
 			}
-			if (last_shown && lno != last_shown + 1)
-				fputs(hunk_mark, stdout);
 			if (!opt->count)
 				show_line(opt, bol, eol, name, lno, ':');
-			last_shown = last_hit = lno;
+			last_hit = lno;
 		}
 		else if (last_hit &&
 			 lno <= last_hit + opt->post_context) {
 			/* If the last hit is within the post context,
 			 * we need to show this line.
 			 */
-			if (last_shown && lno != last_shown + 1)
-				fputs(hunk_mark, stdout);
 			show_line(opt, bol, eol, name, lno, '-');
-			last_shown = lno;
 		}
 		if (opt->pre_context) {
 			memmove(prev+1, prev,
diff --git a/grep.h b/grep.h
index 464e272..0883214 100644
--- a/grep.h
+++ b/grep.h
@@ -84,6 +84,7 @@ struct grep_opt {
 	int regflags;
 	unsigned pre_context;
 	unsigned post_context;
+	unsigned last_shown;
 };
 
 extern void append_grep_pattern(struct grep_opt *opt, const char *pat, const char *origin, int no, enum grep_pat_token t);
-- 
1.6.3.3

Re: [PATCH 2/6] grep: move context hunk mark handling into show_line()

[Junio C Hamano]

René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:

> Move last_shown into struct grep_opt, to make it available in
> show_line(), and then make the function handle the printing of hunk
> marks for context lines in a central place.

Makes sense.  But now "are we showing from a separate block of text" check
is in show_line() and relieves the caller of show_line() from doing so,...

>  		else if (last_hit &&
>  			 lno <= last_hit + opt->post_context) {
>  			/* If the last hit is within the post context,
>  			 * we need to show this line.
>  			 */
> -			if (last_shown && lno != last_shown + 1)
> -				fputs(hunk_mark, stdout);
>  			show_line(opt, bol, eol, name, lno, '-');
> -			last_shown = lno;

... the comment in this context should go, no?

Re: [PATCH 2/6] grep: move context hunk mark handling into show_line()

[René Scharfe]

Junio C Hamano schrieb:
> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> 
>> Move last_shown into struct grep_opt, to make it available in
>> show_line(), and then make the function handle the printing of hunk
>> marks for context lines in a central place.
> 
> Makes sense.  But now "are we showing from a separate block of text" check
> is in show_line() and relieves the caller of show_line() from doing so,...
> 
>>  		else if (last_hit &&
>>  			 lno <= last_hit + opt->post_context) {
>>  			/* If the last hit is within the post context,
>>  			 * we need to show this line.
>>  			 */
>> -			if (last_shown && lno != last_shown + 1)
>> -				fputs(hunk_mark, stdout);
>>  			show_line(opt, bol, eol, name, lno, '-');
>> -			last_shown = lno;
> 
> ... the comment in this context should go, no?

Hmm, I didn't read it to refer to the hunk_mark, but rather to the line
 (from bol to eol).  And it's still true that show_line needs to be
called because we're not more than post_context lines away from the
previous hit, i.e. this line is a post context line.

Re: [PATCH 2/6] grep: move context hunk mark handling into show_line()

[Junio C Hamano]

René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:

> Hmm, I didn't read it to refer to the hunk_mark, but rather to the line
> (from bol to eol).  And it's still true that show_line needs to be
> called because we're not more than post_context lines away from the
> previous hit, i.e. this line is a post context line.

You are right; it was my thinko.

[PATCH 3/6] grep: print context hunk marks between files

[René Scharfe]

Print a hunk mark before matches from a new file are shown, in addition
to the current behaviour of printing them if lines have been skipped.

The result is easier to read, as (presumably unrelated) matches from
different files are separated by a hunk mark.  GNU grep does the same.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 builtin-grep.c  |   11 +++++++++++
 grep.c          |    7 ++++++-
 grep.h          |    1 +
 t/t7002-grep.sh |   22 ++++++++++++++++++++++
 4 files changed, 40 insertions(+), 1 deletions(-)

diff --git a/builtin-grep.c b/builtin-grep.c
index 73fc922..48998af 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -278,6 +278,17 @@ static int flush_grep(struct grep_opt *opt,
 		argc -= 2;
 	}
 
+	if (opt->pre_context || opt->post_context) {
+		/*
+		 * grep handles hunk marks between files, but we need to
+		 * do that ourselves between multiple calls.
+		 */
+		if (opt->show_hunk_mark)
+			write_or_die(1, "--\n", 3);
+		else
+			opt->show_hunk_mark = 1;
+	}
+
 	status = exec_grep(argc, argv);
 
 	if (kept_0) {
diff --git a/grep.c b/grep.c
index 6ee80f7..4bca759 100644
--- a/grep.c
+++ b/grep.c
@@ -491,7 +491,12 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 	int rest = eol - bol;
 
 	if (opt->pre_context || opt->post_context) {
-		if (opt->last_shown && lno > opt->last_shown + 1)
+		if (opt->last_shown == 0) {
+			if (opt->show_hunk_mark)
+				fputs("--\n", stdout);
+			else
+				opt->show_hunk_mark = 1;
+		} else if (lno > opt->last_shown + 1)
 			fputs("--\n", stdout);
 	}
 	opt->last_shown = lno;
diff --git a/grep.h b/grep.h
index 0883214..730ffd6 100644
--- a/grep.h
+++ b/grep.h
@@ -85,6 +85,7 @@ struct grep_opt {
 	unsigned pre_context;
 	unsigned post_context;
 	unsigned last_shown;
+	int show_hunk_mark;
 };
 
 extern void append_grep_pattern(struct grep_opt *opt, const char *pat, const char *origin, int no, enum grep_pat_token t);
diff --git a/t/t7002-grep.sh b/t/t7002-grep.sh
index 7868af8..155bfdb 100755
--- a/t/t7002-grep.sh
+++ b/t/t7002-grep.sh
@@ -155,6 +155,28 @@ test_expect_success 'grep -e A --and --not -e B' '
 	test_cmp expected actual
 '
 
+cat >expected <<EOF
+y:y yy
+--
+z:zzz
+EOF
+
+# Create 1024 file names that sort between "y" and "z" to make sure
+# the two files are handled by different calls to an external grep.
+# This depends on MAXARGS in builtin-grep.c being 1024 or less.
+c32="0 1 2 3 4 5 6 7 8 9 a b c d e f g h i j k l m n o p q r s t u v"
+test_expect_success 'grep -C1, hunk mark between files' '
+	for a in $c32; do for b in $c32; do : >y-$a$b; done; done &&
+	git add y-?? &&
+	git grep -C1 "^[yz]" >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'grep -C1 --no-ext-grep, hunk mark between files' '
+	git grep -C1 --no-ext-grep "^[yz]" >actual &&
+	test_cmp expected actual
+'
+
 test_expect_success 'log grep setup' '
 	echo a >>file &&
 	test_tick &&
-- 
1.6.3.3

[PATCH 4/6] grep: handle pre context lines on demand

[René Scharfe]

Factor out pre context line handling into the new function
show_pre_context() and change the algorithm to rewind by looking for
newline characters and roll forward again, instead of maintaining an
array of line beginnings and ends.

This is slower for hits, but the cost for non-matching lines becomes
zero.  Normally, there are far more non-matching lines, so the time
spent in total decreases.

Before this patch (current Linux kernel repo, best of five runs):

	$ time git grep --no-ext-grep -B1 memset >/dev/null

	real	0m2.134s
	user	0m1.932s
	sys	0m0.196s

	$ time git grep --no-ext-grep -B1000 memset >/dev/null

	real	0m12.059s
	user	0m11.837s
	sys	0m0.224s

The same with this patch:

	$ time git grep --no-ext-grep -B1 memset >/dev/null

	real	0m2.117s
	user	0m1.892s
	sys	0m0.228s

	$ time git grep --no-ext-grep -B1000 memset >/dev/null

	real	0m2.986s
	user	0m2.696s
	sys	0m0.288s

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 grep.c |   61 ++++++++++++++++++++++++++++++++-----------------------------
 1 files changed, 32 insertions(+), 29 deletions(-)

diff --git a/grep.c b/grep.c
index 4bca759..9b9d2e3 100644
--- a/grep.c
+++ b/grep.c
@@ -531,16 +531,42 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 	printf("%.*s\n", rest, bol);
 }
 
+static void show_pre_context(struct grep_opt *opt, const char *name, char *buf,
+			     char *bol, unsigned lno)
+{
+	unsigned cur = lno, from = 1;
+
+	if (opt->pre_context < lno)
+		from = lno - opt->pre_context;
+	if (from <= opt->last_shown)
+		from = opt->last_shown + 1;
+
+	/* Rewind. */
+	while (bol > buf && cur > from) {
+		bol--;
+		while (bol > buf && bol[-1] != '\n')
+			bol--;
+		cur--;
+	}
+
+	/* Back forward. */
+	while (cur < lno) {
+		char *eol = bol;
+
+		while (*eol != '\n')
+			eol++;
+		show_line(opt, bol, eol, name, cur, '-');
+		bol = eol + 1;
+		cur++;
+	}
+}
+
 static int grep_buffer_1(struct grep_opt *opt, const char *name,
 			 char *buf, unsigned long size, int collect_hits)
 {
 	char *bol = buf;
 	unsigned long left = size;
 	unsigned lno = 1;
-	struct pre_context_line {
-		char *bol;
-		char *eol;
-	} *prev = NULL, *pcl;
 	unsigned last_hit = 0;
 	int binary_match_only = 0;
 	unsigned count = 0;
@@ -561,9 +587,6 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
 		}
 	}
 
-	if (opt->pre_context)
-		prev = xcalloc(opt->pre_context, sizeof(*prev));
-
 	while (left) {
 		char *eol, ch;
 		int hit;
@@ -610,21 +633,8 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
 			 * the context which is nonsense, but the user
 			 * deserves to get that ;-).
 			 */
-			if (opt->pre_context) {
-				unsigned from;
-				if (opt->pre_context < lno)
-					from = lno - opt->pre_context;
-				else
-					from = 1;
-				if (from <= opt->last_shown)
-					from = opt->last_shown + 1;
-				while (from < lno) {
-					pcl = &prev[lno-from-1];
-					show_line(opt, pcl->bol, pcl->eol,
-						  name, from, '-');
-					from++;
-				}
-			}
+			if (opt->pre_context)
+				show_pre_context(opt, name, buf, bol, lno);
 			if (!opt->count)
 				show_line(opt, bol, eol, name, lno, ':');
 			last_hit = lno;
@@ -636,12 +646,6 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
 			 */
 			show_line(opt, bol, eol, name, lno, '-');
 		}
-		if (opt->pre_context) {
-			memmove(prev+1, prev,
-				(opt->pre_context-1) * sizeof(*prev));
-			prev->bol = bol;
-			prev->eol = eol;
-		}
 
 	next_line:
 		bol = eol + 1;
@@ -651,7 +655,6 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
 		lno++;
 	}
 
-	free(prev);
 	if (collect_hits)
 		return 0;
 
-- 
1.6.3.3

[PATCH 5/6] grep: add option -p/--show-function

[René Scharfe]

The new option -p instructs git grep to print the previous function
definition as a context line, similar to diff -p.  Such context lines
are marked with an equal sign instead of a dash.  This option
complements the existing context options -A, -B, -C.

Function definitions are detected using the same heuristic that diff
uses.  User defined regular expressions are not supported, yet.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 Documentation/git-grep.txt |    5 +++
 builtin-grep.c             |    8 +++--
 grep.c                     |   61 ++++++++++++++++++++++++++++++++++++++------
 grep.h                     |    1 +
 t/t7002-grep.sh            |   36 ++++++++++++++++++++++++-
 5 files changed, 98 insertions(+), 13 deletions(-)

diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index fccb82d..b3bb283 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -122,6 +122,11 @@ OPTIONS
 -<num>::
 	A shortcut for specifying -C<num>.
 
+-p::
+--show-function::
+	Show the preceding line that contains the function name of
+	the match, unless the matching line is a function name itself.
+
 -f <file>::
 	Read patterns from <file>, one per line.
 
diff --git a/builtin-grep.c b/builtin-grep.c
index 48998af..037452e 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -278,13 +278,13 @@ static int flush_grep(struct grep_opt *opt,
 		argc -= 2;
 	}
 
-	if (opt->pre_context || opt->post_context) {
+	if (opt->pre_context || opt->post_context || opt->funcname) {
 		/*
 		 * grep handles hunk marks between files, but we need to
 		 * do that ourselves between multiple calls.
 		 */
 		if (opt->show_hunk_mark)
-			write_or_die(1, "--\n", 3);
+			write_or_die(1, opt->funcname ? "==\n" : "--\n", 3);
 		else
 			opt->show_hunk_mark = 1;
 	}
@@ -721,6 +721,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			"show <n> context lines after matches"),
 		OPT_NUMBER_CALLBACK(&opt, "shortcut for -C NUM",
 			context_callback),
+		OPT_BOOLEAN('p', "show-function", &opt.funcname,
+			"show a line with the function name before matches"),
 		OPT_GROUP(""),
 		OPT_CALLBACK('f', NULL, &opt, "file",
 			"read patterns from file", file_callback),
@@ -789,7 +791,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		argc--;
 	}
 
-	if (opt.color && !opt.color_external)
+	if ((opt.color && !opt.color_external) || opt.funcname)
 		external_grep_allowed = 0;
 	if (!opt.pattern_list)
 		die("no pattern given.");
diff --git a/grep.c b/grep.c
index 9b9d2e3..3a5c138 100644
--- a/grep.c
+++ b/grep.c
@@ -490,14 +490,18 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 {
 	int rest = eol - bol;
 
-	if (opt->pre_context || opt->post_context) {
+	if (opt->pre_context || opt->post_context || opt->funcname) {
 		if (opt->last_shown == 0) {
 			if (opt->show_hunk_mark)
-				fputs("--\n", stdout);
+				fputs(opt->funcname ? "==\n" : "--\n", stdout);
 			else
 				opt->show_hunk_mark = 1;
-		} else if (lno > opt->last_shown + 1)
-			fputs("--\n", stdout);
+		} else if (lno > opt->last_shown + 1) {
+			if (opt->pre_context || opt->post_context)
+				fputs((sign == '=') ? "==\n" : "--\n", stdout);
+			else if (sign == '=')
+				fputs("==\n", stdout);
+		}
 	}
 	opt->last_shown = lno;
 
@@ -531,10 +535,40 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 	printf("%.*s\n", rest, bol);
 }
 
+static int match_funcname(char *bol, char *eol)
+{
+	if (bol == eol)
+		return 0;
+	if (isalpha(*bol) || *bol == '_' || *bol == '$')
+		return 1;
+	return 0;
+}
+
+static void show_funcname_line(struct grep_opt *opt, const char *name,
+			       char *buf, char *bol, unsigned lno)
+{
+	while (bol > buf) {
+		char *eol = --bol;
+
+		while (bol > buf && bol[-1] != '\n')
+			bol--;
+		lno--;
+
+		if (lno <= opt->last_shown)
+			break;
+
+		if (match_funcname(bol, eol)) {
+			show_line(opt, bol, eol, name, lno, '=');
+			break;
+		}
+	}
+}
+
 static void show_pre_context(struct grep_opt *opt, const char *name, char *buf,
 			     char *bol, unsigned lno)
 {
-	unsigned cur = lno, from = 1;
+	unsigned cur = lno, from = 1, funcname_lno = 0;
+	int funcname_needed = opt->funcname;
 
 	if (opt->pre_context < lno)
 		from = lno - opt->pre_context;
@@ -543,19 +577,28 @@ static void show_pre_context(struct grep_opt *opt, const char *name, char *buf,
 
 	/* Rewind. */
 	while (bol > buf && cur > from) {
-		bol--;
+		char *eol = --bol;
+
 		while (bol > buf && bol[-1] != '\n')
 			bol--;
 		cur--;
+		if (funcname_needed && match_funcname(bol, eol)) {
+			funcname_lno = cur;
+			funcname_needed = 0;
+		}
 	}
 
+	/* We need to look even further back to find a function signature. */
+	if (opt->funcname && funcname_needed)
+		show_funcname_line(opt, name, buf, bol, cur);
+
 	/* Back forward. */
 	while (cur < lno) {
-		char *eol = bol;
+		char *eol = bol, sign = (cur == funcname_lno) ? '=' : '-';
 
 		while (*eol != '\n')
 			eol++;
-		show_line(opt, bol, eol, name, cur, '-');
+		show_line(opt, bol, eol, name, cur, sign);
 		bol = eol + 1;
 		cur++;
 	}
@@ -635,6 +678,8 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
 			 */
 			if (opt->pre_context)
 				show_pre_context(opt, name, buf, bol, lno);
+			else if (opt->funcname)
+				show_funcname_line(opt, name, buf, bol, lno);
 			if (!opt->count)
 				show_line(opt, bol, eol, name, lno, ':');
 			last_hit = lno;
diff --git a/grep.h b/grep.h
index 730ffd6..3f75e3a 100644
--- a/grep.h
+++ b/grep.h
@@ -79,6 +79,7 @@ struct grep_opt {
 	int pathname;
 	int null_following_name;
 	int color;
+	int funcname;
 	char color_match[COLOR_MAXLEN];
 	const char *color_external;
 	int regflags;
diff --git a/t/t7002-grep.sh b/t/t7002-grep.sh
index 155bfdb..ef59ab9 100755
--- a/t/t7002-grep.sh
+++ b/t/t7002-grep.sh
@@ -8,6 +8,15 @@ test_description='git grep various.
 
 . ./test-lib.sh
 
+cat >hello.c <<EOF
+#include <stdio.h>
+int main(int argc, const char **argv)
+{
+	printf("Hello world.\n");
+	return 0;
+}
+EOF
+
 test_expect_success setup '
 	{
 		echo foo mmap bar
@@ -22,7 +31,7 @@ test_expect_success setup '
 	echo zzz > z &&
 	mkdir t &&
 	echo test >t/t &&
-	git add file w x y z t/t &&
+	git add file w x y z t/t hello.c &&
 	test_tick &&
 	git commit -m initial
 '
@@ -229,9 +238,32 @@ test_expect_success 'log grep (6)' '
 test_expect_success 'grep with CE_VALID file' '
 	git update-index --assume-unchanged t/t &&
 	rm t/t &&
-	test "$(git grep --no-ext-grep t)" = "t/t:test" &&
+	test "$(git grep --no-ext-grep test)" = "t/t:test" &&
 	git update-index --no-assume-unchanged t/t &&
 	git checkout t/t
 '
 
+cat >expected <<EOF
+hello.c=int main(int argc, const char **argv)
+hello.c:	return 0;
+EOF
+
+test_expect_success 'grep -p' '
+	git grep -p return >actual &&
+	test_cmp expected actual
+'
+
+cat >expected <<EOF
+hello.c-#include <stdio.h>
+hello.c=int main(int argc, const char **argv)
+hello.c-{
+hello.c-	printf("Hello world.\n");
+hello.c:	return 0;
+EOF
+
+test_expect_success 'grep -p -B5' '
+	git grep -p -B5 return >actual &&
+	test_cmp expected actual
+'
+
 test_done
-- 
1.6.3.3

Re: [PATCH 5/6] grep: add option -p/--show-function

[Junio C Hamano]

René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:

> The new option -p instructs git grep to print the previous function
> definition as a context line, similar to diff -p.  Such context lines
> are marked with an equal sign instead of a dash.  This option
> complements the existing context options -A, -B, -C.

I have to say that I am a bit confused with these '==' markers.

    $ ./git-grep -p -n -e pre_context -- '*.c'
    builtin-grep.c=238=static int flush_grep(struct grep_opt *opt,
    builtin-grep.c:288:	if (opt->pre_context || opt->post_context ||...
    ==
    builtin-grep.c=328=static int external_grep(struct grep_opt *opt, co...
    builtin-grep.c:363:	if (opt->post_context || opt->pre_context) {
    builtin-grep.c:364:		if (opt->post_context != opt->pre_co...
    builtin-grep.c:365:			if (opt->pre_context) {
    builtin-grep.c:368:						"%u"...
    ==
    builtin-grep.c=586=static int context_callback(const struct option *...

Am I correct to say that '==' tells us that it has been showing the hits
in the previous function shown on the =$lineno= line, but now it will show
hits from a different function?

When used with -[ABC], it gets even confusing.

    $ ./git-grep -C1 -p -n -e pre_context -- '*.c'
    builtin-grep.c=238=static int flush_grep(struct grep_opt *opt,
    --
    builtin-grep.c-287-
    builtin-grep.c:288:	if (opt->pre_context || opt->post_context ||...
    builtin-grep.c-289-		/*
    ==
    builtin-grep.c=328=static int external_grep(struct grep_opt *opt, co...
    --
    builtin-grep.c-362-		push_arg("-c");
    builtin-grep.c:363:	if (opt->post_context || opt->pre_context) {
    builtin-grep.c:364:		if (opt->post_context != opt->pre_co...
    builtin-grep.c:365:			if (opt->pre_context) {
    builtin-grep.c-366-				push_arg("-B");
    builtin-grep.c-367-				len += snprintf(argp...
    builtin-grep.c:368:						"%u"...
    builtin-grep.c-369-				if (sizeof(randarg) ...
    ==
    builtin-grep.c=586=static int context_callback(const struct option *...
    --

What do "--" lines tell us in these cases?  For example, does the first
one tells us that the -p output shown, which was from line 238, is outside
of the context of the hit it will show at line 288?

I am in no way complaining.  My eyes are just not yet used to the output
format and are having hard time getting used to them and/or appreciating
how the separation these dividing lines give us is helpful.

Re: [PATCH 5/6] grep: add option -p/--show-function

[René Scharfe]

Junio C Hamano schrieb:
> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> 
>> The new option -p instructs git grep to print the previous function
>> definition as a context line, similar to diff -p.  Such context lines
>> are marked with an equal sign instead of a dash.  This option
>> complements the existing context options -A, -B, -C.
> 
> I have to say that I am a bit confused with these '==' markers.

You may be glad to hear that initial versions of this patch followed
even stranger rules for showing these markers. ;-)

>     $ ./git-grep -p -n -e pre_context -- '*.c'
>     builtin-grep.c=238=static int flush_grep(struct grep_opt *opt,
>     builtin-grep.c:288:	if (opt->pre_context || opt->post_context ||...
>     ==
>     builtin-grep.c=328=static int external_grep(struct grep_opt *opt, co...
>     builtin-grep.c:363:	if (opt->post_context || opt->pre_context) {
>     builtin-grep.c:364:		if (opt->post_context != opt->pre_co...
>     builtin-grep.c:365:			if (opt->pre_context) {
>     builtin-grep.c:368:						"%u"...
>     ==
>     builtin-grep.c=586=static int context_callback(const struct option *...
> 
> Am I correct to say that '==' tells us that it has been showing the hits
> in the previous function shown on the =$lineno= line, but now it will show
> hits from a different function?

With -p, the previous function of a hit is marked with an "=" (or
"=$lineno=" with -n).  "==" is printed immediately before such a line,
to separate it from other functions, yes.

"--" could be printed between lines 238 and 288, between lines 328 and
363 (after the function lines), and between lines 365 and 368, but that
would bloat the output for no gain, IMHO.  It would mean "here we've
skipped over a few lines", which is correct, but it's also common and
expected.  So I left out print of "--" markers completely if -p is used
without -[ABC].

> When used with -[ABC], it gets even confusing.
> 
>     $ ./git-grep -C1 -p -n -e pre_context -- '*.c'
>     builtin-grep.c=238=static int flush_grep(struct grep_opt *opt,
>     --
>     builtin-grep.c-287-
>     builtin-grep.c:288:	if (opt->pre_context || opt->post_context ||...
>     builtin-grep.c-289-		/*
>     ==
>     builtin-grep.c=328=static int external_grep(struct grep_opt *opt, co...
>     --
>     builtin-grep.c-362-		push_arg("-c");
>     builtin-grep.c:363:	if (opt->post_context || opt->pre_context) {
>     builtin-grep.c:364:		if (opt->post_context != opt->pre_co...
>     builtin-grep.c:365:			if (opt->pre_context) {
>     builtin-grep.c-366-				push_arg("-B");
>     builtin-grep.c-367-				len += snprintf(argp...
>     builtin-grep.c:368:						"%u"...
>     builtin-grep.c-369-				if (sizeof(randarg) ...
>     ==
>     builtin-grep.c=586=static int context_callback(const struct option *...
>     --
> 
> What do "--" lines tell us in these cases?  For example, does the first
> one tells us that the -p output shown, which was from line 238, is outside
> of the context of the hit it will show at line 288?
>
> I am in no way complaining.  My eyes are just not yet used to the output
> format and are having hard time getting used to them and/or appreciating
> how the separation these dividing lines give us is helpful.

Here's another example, which is potentially even more confusing:

	$ git grep -p -n markers grep.c
	grep.c=748=static void clr_hit_marker(struct grep_expr *x)
	grep.c:750:	/* All-hit markers are meaningful only at...
	==
	grep.c=762=static int chk_hit_marker(struct grep_expr *x)
	grep.c:764:	/* Top level nodes have hit markers.  See...
	==
	grep.c=774=int grep_buffer(struct grep_opt *opt, const ch...
	grep.c:784:	 * We first clear hit markers from them.


	$ git grep -p -n -B3 markers grep.c
	grep.c-747-
	grep.c=748=static void clr_hit_marker(struct grep_expr *x)
	grep.c-749-{
	grep.c:750:	/* All-hit markers are meaningful only at...
	--
	grep.c-761-
	grep.c=762=static int chk_hit_marker(struct grep_expr *x)
	grep.c-763-{
	grep.c:764:	/* Top level nodes have hit markers.  See...
	==
	grep.c=774=int grep_buffer(struct grep_opt *opt, const ch...
	--
	grep.c-781-		return grep_buffer_1(opt, name, b...
	grep.c-782-
	grep.c-783-	/* Otherwise the toplevel "or" terms hit ...
	grep.c:784:	 * We first clear hit markers from them.

The "==" after line 750 is changed to a "--" with -B3 or more.


I'm not so sure about the usefulness of these markers myself, but the
rules to generate them are quite simple.  The "==" markers exist only
because I wanted the output of -p alone to be as compact as possible,
but without changing the meaning of the "--" markers.

So the meaning of the "--" markers is the same as before: "lines have
been skipped".  "==" means the same, but also "the next line is a
function line".  And -p alone only shows these specialized markers, for
brevity.

Hmm, even shorter would be to leave out these "==" things entirely:

	# mock-up
	$ git grep -p -n markers grep.c
	grep.c=746=static void clr_hit_marker(struct grep_expr *x)
	grep.c:748:     /* All-hit markers are meaningful only at...
	grep.c=760=static int chk_hit_marker(struct grep_expr *x)
	grep.c:762:     /* Top level nodes have hit markers.  See...
	grep.c=772=int grep_buffer(struct grep_opt *opt, const ch...
	grep.c:782:      * We first clear hit markers from them.

Is that still readable?

Re: [PATCH 5/6] grep: add option -p/--show-function

[Junio C Hamano]

René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:

> Hmm, even shorter would be to leave out these "==" things entirely:
>
> 	# mock-up
> 	$ git grep -p -n markers grep.c
> 	grep.c=746=static void clr_hit_marker(struct grep_expr *x)
> 	grep.c:748:     /* All-hit markers are meaningful only at...
> 	grep.c=760=static int chk_hit_marker(struct grep_expr *x)
> 	grep.c:762:     /* Top level nodes have hit markers.  See...
> 	grep.c=772=int grep_buffer(struct grep_opt *opt, const ch...
> 	grep.c:782:      * We first clear hit markers from them.
>
> Is that still readable?

Actually the above matches what I expected to see when I read your [0/6].

Is the use of '=' your invention, or does somebody's grep have this
function header feature already and use '=' the same way?

Re: [PATCH 5/6] grep: add option -p/--show-function

[René Scharfe]

Junio C Hamano schrieb:
> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> 
>> Hmm, even shorter would be to leave out these "==" things entirely:
>>
>> 	# mock-up
>> 	$ git grep -p -n markers grep.c
>> 	grep.c=746=static void clr_hit_marker(struct grep_expr *x)
>> 	grep.c:748:     /* All-hit markers are meaningful only at...
>> 	grep.c=760=static int chk_hit_marker(struct grep_expr *x)
>> 	grep.c:762:     /* Top level nodes have hit markers.  See...
>> 	grep.c=772=int grep_buffer(struct grep_opt *opt, const ch...
>> 	grep.c:782:      * We first clear hit markers from them.
>>
>> Is that still readable?
> 
> Actually the above matches what I expected to see when I read your [0/6].

OK. :)

> Is the use of '=' your invention, or does somebody's grep have this
> function header feature already and use '=' the same way?

I didn't find a prior implementation, only this announcement:

   http://thread.gmane.org/gmane.comp.gnu.grep.bugs/141

Initially, I used '@' (similar to diff), but that looked strange.  The
equal sign seemed to fit well ('-' means context, '=' means even more
context! ;-), but I haven't put a lot of thought or research into it.

Here's patch 5 again, with all traces of "==" markers removed.  Patch 6
still applies, albeit with a slight offset.

--- snip! ---
The new option -p instructs git grep to print the previous function
definition as a context line, similar to diff -p.  Such context lines
are marked with an equal sign instead of a dash.  This option
complements the existing context options -A, -B, -C.

Function definitions are detected using the same heuristic that diff
uses.  User defined regular expressions are not supported, yet.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 Documentation/git-grep.txt |    5 ++++
 builtin-grep.c             |    4 ++-
 grep.c                     |   49 ++++++++++++++++++++++++++++++++++++++++---
 grep.h                     |    1 +
 t/t7002-grep.sh            |   36 ++++++++++++++++++++++++++++++-
 5 files changed, 88 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index fccb82d..b3bb283 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -122,6 +122,11 @@ OPTIONS
 -<num>::
 	A shortcut for specifying -C<num>.
 
+-p::
+--show-function::
+	Show the preceding line that contains the function name of
+	the match, unless the matching line is a function name itself.
+
 -f <file>::
 	Read patterns from <file>, one per line.
 
diff --git a/builtin-grep.c b/builtin-grep.c
index 48998af..44fd485 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -721,6 +721,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			"show <n> context lines after matches"),
 		OPT_NUMBER_CALLBACK(&opt, "shortcut for -C NUM",
 			context_callback),
+		OPT_BOOLEAN('p', "show-function", &opt.funcname,
+			"show a line with the function name before matches"),
 		OPT_GROUP(""),
 		OPT_CALLBACK('f', NULL, &opt, "file",
 			"read patterns from file", file_callback),
@@ -789,7 +791,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		argc--;
 	}
 
-	if (opt.color && !opt.color_external)
+	if ((opt.color && !opt.color_external) || opt.funcname)
 		external_grep_allowed = 0;
 	if (!opt.pattern_list)
 		die("no pattern given.");
diff --git a/grep.c b/grep.c
index 9b9d2e3..364c7c3 100644
--- a/grep.c
+++ b/grep.c
@@ -531,10 +531,40 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 	printf("%.*s\n", rest, bol);
 }
 
+static int match_funcname(char *bol, char *eol)
+{
+	if (bol == eol)
+		return 0;
+	if (isalpha(*bol) || *bol == '_' || *bol == '$')
+		return 1;
+	return 0;
+}
+
+static void show_funcname_line(struct grep_opt *opt, const char *name,
+			       char *buf, char *bol, unsigned lno)
+{
+	while (bol > buf) {
+		char *eol = --bol;
+
+		while (bol > buf && bol[-1] != '\n')
+			bol--;
+		lno--;
+
+		if (lno <= opt->last_shown)
+			break;
+
+		if (match_funcname(bol, eol)) {
+			show_line(opt, bol, eol, name, lno, '=');
+			break;
+		}
+	}
+}
+
 static void show_pre_context(struct grep_opt *opt, const char *name, char *buf,
 			     char *bol, unsigned lno)
 {
-	unsigned cur = lno, from = 1;
+	unsigned cur = lno, from = 1, funcname_lno = 0;
+	int funcname_needed = opt->funcname;
 
 	if (opt->pre_context < lno)
 		from = lno - opt->pre_context;
@@ -543,19 +573,28 @@ static void show_pre_context(struct grep_opt *opt, const char *name, char *buf,
 
 	/* Rewind. */
 	while (bol > buf && cur > from) {
-		bol--;
+		char *eol = --bol;
+
 		while (bol > buf && bol[-1] != '\n')
 			bol--;
 		cur--;
+		if (funcname_needed && match_funcname(bol, eol)) {
+			funcname_lno = cur;
+			funcname_needed = 0;
+		}
 	}
 
+	/* We need to look even further back to find a function signature. */
+	if (opt->funcname && funcname_needed)
+		show_funcname_line(opt, name, buf, bol, cur);
+
 	/* Back forward. */
 	while (cur < lno) {
-		char *eol = bol;
+		char *eol = bol, sign = (cur == funcname_lno) ? '=' : '-';
 
 		while (*eol != '\n')
 			eol++;
-		show_line(opt, bol, eol, name, cur, '-');
+		show_line(opt, bol, eol, name, cur, sign);
 		bol = eol + 1;
 		cur++;
 	}
@@ -635,6 +674,8 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
 			 */
 			if (opt->pre_context)
 				show_pre_context(opt, name, buf, bol, lno);
+			else if (opt->funcname)
+				show_funcname_line(opt, name, buf, bol, lno);
 			if (!opt->count)
 				show_line(opt, bol, eol, name, lno, ':');
 			last_hit = lno;
diff --git a/grep.h b/grep.h
index 730ffd6..3f75e3a 100644
--- a/grep.h
+++ b/grep.h
@@ -79,6 +79,7 @@ struct grep_opt {
 	int pathname;
 	int null_following_name;
 	int color;
+	int funcname;
 	char color_match[COLOR_MAXLEN];
 	const char *color_external;
 	int regflags;
diff --git a/t/t7002-grep.sh b/t/t7002-grep.sh
index 155bfdb..ef59ab9 100755
--- a/t/t7002-grep.sh
+++ b/t/t7002-grep.sh
@@ -8,6 +8,15 @@ test_description='git grep various.
 
 . ./test-lib.sh
 
+cat >hello.c <<EOF
+#include <stdio.h>
+int main(int argc, const char **argv)
+{
+	printf("Hello world.\n");
+	return 0;
+}
+EOF
+
 test_expect_success setup '
 	{
 		echo foo mmap bar
@@ -22,7 +31,7 @@ test_expect_success setup '
 	echo zzz > z &&
 	mkdir t &&
 	echo test >t/t &&
-	git add file w x y z t/t &&
+	git add file w x y z t/t hello.c &&
 	test_tick &&
 	git commit -m initial
 '
@@ -229,9 +238,32 @@ test_expect_success 'log grep (6)' '
 test_expect_success 'grep with CE_VALID file' '
 	git update-index --assume-unchanged t/t &&
 	rm t/t &&
-	test "$(git grep --no-ext-grep t)" = "t/t:test" &&
+	test "$(git grep --no-ext-grep test)" = "t/t:test" &&
 	git update-index --no-assume-unchanged t/t &&
 	git checkout t/t
 '
 
+cat >expected <<EOF
+hello.c=int main(int argc, const char **argv)
+hello.c:	return 0;
+EOF
+
+test_expect_success 'grep -p' '
+	git grep -p return >actual &&
+	test_cmp expected actual
+'
+
+cat >expected <<EOF
+hello.c-#include <stdio.h>
+hello.c=int main(int argc, const char **argv)
+hello.c-{
+hello.c-	printf("Hello world.\n");
+hello.c:	return 0;
+EOF
+
+test_expect_success 'grep -p -B5' '
+	git grep -p -B5 return >actual &&
+	test_cmp expected actual
+'
+
 test_done
-- 
1.6.3.3

Re: [PATCH 5/6] grep: add option -p/--show-function

[René Scharfe]

And here is the interdiff to remove the code to print == markers.

I think it makes sense to start out without them.  We can add them
back later (or something else, like colourization) if the simpler
output turns out to be difficult to read.

Comments?  Opinions?


 builtin-grep.c |    4 ++--
 grep.c         |   12 ++++--------
 2 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/builtin-grep.c b/builtin-grep.c
index 9343cc5..ff8e51b 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -285,13 +285,13 @@ static int flush_grep(struct grep_opt *opt,
 		argc -= 2;
 	}
 
-	if (opt->pre_context || opt->post_context || opt->funcname) {
+	if (opt->pre_context || opt->post_context) {
 		/*
 		 * grep handles hunk marks between files, but we need to
 		 * do that ourselves between multiple calls.
 		 */
 		if (opt->show_hunk_mark)
-			write_or_die(1, opt->funcname ? "==\n" : "--\n", 3);
+			write_or_die(1, "--\n", 3);
 		else
 			opt->show_hunk_mark = 1;
 	}
diff --git a/grep.c b/grep.c
index c47785a..5d162da 100644
--- a/grep.c
+++ b/grep.c
@@ -491,18 +491,14 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 {
 	int rest = eol - bol;
 
-	if (opt->pre_context || opt->post_context || opt->funcname) {
+	if (opt->pre_context || opt->post_context) {
 		if (opt->last_shown == 0) {
 			if (opt->show_hunk_mark)
-				fputs(opt->funcname ? "==\n" : "--\n", stdout);
+				fputs("--\n", stdout);
 			else
 				opt->show_hunk_mark = 1;
-		} else if (lno > opt->last_shown + 1) {
-			if (opt->pre_context || opt->post_context)
-				fputs((sign == '=') ? "==\n" : "--\n", stdout);
-			else if (sign == '=')
-				fputs("==\n", stdout);
-		}
+		} else if (lno > opt->last_shown + 1)
+			fputs("--\n", stdout);
 	}
 	opt->last_shown = lno;
 

[PATCH 6/6] grep -p: support user defined regular expressions

[René Scharfe]

Respect the userdiff attributes and config settings when looking for
lines with function definitions in git grep -p.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 Documentation/git-grep.txt |    3 +++
 builtin-grep.c             |    7 +++++++
 grep.c                     |   29 ++++++++++++++++++++++++++---
 grep.h                     |    1 +
 t/t7002-grep.sh            |   13 +++++++++++++
 5 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index b3bb283..b753c9d 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -126,6 +126,9 @@ OPTIONS
 --show-function::
 	Show the preceding line that contains the function name of
 	the match, unless the matching line is a function name itself.
+	The name is determined in the same way as 'git diff' works out
+	patch hunk headers (see 'Defining a custom hunk-header' in
+	linkgit:gitattributes[5]).
 
 -f <file>::
 	Read patterns from <file>, one per line.
diff --git a/builtin-grep.c b/builtin-grep.c
index 037452e..9343cc5 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -11,6 +11,7 @@
 #include "tree-walk.h"
 #include "builtin.h"
 #include "parse-options.h"
+#include "userdiff.h"
 #include "grep.h"
 
 #ifndef NO_EXTERNAL_GREP
@@ -30,6 +31,12 @@ static int grep_config(const char *var, const char *value, void *cb)
 {
 	struct grep_opt *opt = cb;
 
+	switch (userdiff_config(var, value)) {
+	case 0: break;
+	case -1: return -1;
+	default: return 0;
+	}
+
 	if (!strcmp(var, "color.grep")) {
 		opt->color = git_config_colorbool(var, value, -1);
 		return 0;
diff --git a/grep.c b/grep.c
index 3a5c138..c47785a 100644
--- a/grep.c
+++ b/grep.c
@@ -1,5 +1,6 @@
 #include "cache.h"
 #include "grep.h"
+#include "userdiff.h"
 #include "xdiff-interface.h"
 
 void append_header_grep_pattern(struct grep_opt *opt, enum grep_header_field field, const char *pat)
@@ -535,8 +536,15 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 	printf("%.*s\n", rest, bol);
 }
 
-static int match_funcname(char *bol, char *eol)
+static int match_funcname(struct grep_opt *opt, char *bol, char *eol)
 {
+	xdemitconf_t *xecfg = opt->priv;
+	if (xecfg && xecfg->find_func) {
+		char buf[1];
+		return xecfg->find_func(bol, eol - bol, buf, 1,
+					xecfg->find_func_priv) >= 0;
+	}
+
 	if (bol == eol)
 		return 0;
 	if (isalpha(*bol) || *bol == '_' || *bol == '$')
@@ -557,7 +565,7 @@ static void show_funcname_line(struct grep_opt *opt, const char *name,
 		if (lno <= opt->last_shown)
 			break;
 
-		if (match_funcname(bol, eol)) {
+		if (match_funcname(opt, bol, eol)) {
 			show_line(opt, bol, eol, name, lno, '=');
 			break;
 		}
@@ -582,7 +590,7 @@ static void show_pre_context(struct grep_opt *opt, const char *name, char *buf,
 		while (bol > buf && bol[-1] != '\n')
 			bol--;
 		cur--;
-		if (funcname_needed && match_funcname(bol, eol)) {
+		if (funcname_needed && match_funcname(opt, bol, eol)) {
 			funcname_lno = cur;
 			funcname_needed = 0;
 		}
@@ -614,6 +622,7 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
 	int binary_match_only = 0;
 	unsigned count = 0;
 	enum grep_context ctx = GREP_CONTEXT_HEAD;
+	xdemitconf_t xecfg;
 
 	opt->last_shown = 0;
 
@@ -630,6 +639,17 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
 		}
 	}
 
+	memset(&xecfg, 0, sizeof(xecfg));
+	if (opt->funcname && !opt->unmatch_name_only && !opt->status_only &&
+	    !opt->name_only && !binary_match_only && !collect_hits) {
+		struct userdiff_driver *drv = userdiff_find_by_path(name);
+		if (drv && drv->funcname.pattern) {
+			const struct userdiff_funcname *pe = &drv->funcname;
+			xdiff_set_find_func(&xecfg, pe->pattern, pe->cflags);
+			opt->priv = &xecfg;
+		}
+	}
+
 	while (left) {
 		char *eol, ch;
 		int hit;
@@ -711,6 +731,9 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
 		return 1;
 	}
 
+	xdiff_clear_find_func(&xecfg);
+	opt->priv = NULL;
+
 	/* NEEDSWORK:
 	 * The real "grep -c foo *.c" gives many "bar.c:0" lines,
 	 * which feels mostly useless but sometimes useful.  Maybe
diff --git a/grep.h b/grep.h
index 3f75e3a..f00db0e 100644
--- a/grep.h
+++ b/grep.h
@@ -87,6 +87,7 @@ struct grep_opt {
 	unsigned post_context;
 	unsigned last_shown;
 	int show_hunk_mark;
+	void *priv;
 };
 
 extern void append_grep_pattern(struct grep_opt *opt, const char *pat, const char *origin, int no, enum grep_pat_token t);
diff --git a/t/t7002-grep.sh b/t/t7002-grep.sh
index ef59ab9..b13aa7e 100755
--- a/t/t7002-grep.sh
+++ b/t/t7002-grep.sh
@@ -244,11 +244,24 @@ test_expect_success 'grep with CE_VALID file' '
 '
 
 cat >expected <<EOF
+hello.c=#include <stdio.h>
+hello.c:	return 0;
+EOF
+
+test_expect_success 'grep -p with userdiff' '
+	git config diff.custom.funcname "^#" &&
+	echo "hello.c diff=custom" >.gitattributes &&
+	git grep -p return >actual &&
+	test_cmp expected actual
+'
+
+cat >expected <<EOF
 hello.c=int main(int argc, const char **argv)
 hello.c:	return 0;
 EOF
 
 test_expect_success 'grep -p' '
+	rm -f .gitattributes &&
 	git grep -p return >actual &&
 	test_cmp expected actual
 '
-- 
1.6.3.3