threads / rfc / 36306

patch, 10 parts[RFC] pickaxe for function names

Subject: [PATCH 00/10] [RFC] pickaxe for function names

## tl;dr

23 messages between Mar 27, 2014 and Apr 28, 2014. Diffs are folded; open one to read it.

replies: 22people: 8as markdown or json

David A. Dalrymple (and Bhushan G. Lodha)· Mar 27, 2014, 18:50 UTC · lore

This series introduces a --function-name=<pattern> option for git-log, intended to search for commits which touch a function matching a certain pattern (a feature we've seen requested and are interested in using ourselves).

This is our first attempt to patch git; we've tried to observe and follow the community standards, but we would greatly appreciate feedback. We've been working on this for a few weeks, and I just noticed that René Scharfe has done conflicing (and better) refactoring work in diffcore-pickaxe.c a few days ago. We'd be happy to rebase our changes and resolve the conflicts once René's patches are committed to master, but we thought we may as well ask for comments on the version we have working now.

Thanks for your time!
  .gitattributes: specify the language used
  diffcore-pickaxe.c: refactor regex compilation
  diffcore-pickaxe.c: Refactor pickaxe_fn signature
  diff.c/diff.h: expose userdiff_funcname
  diffcore-pickaxe.c: set up funcname pattern
  log: --function-name pickaxe
  xdiff: add XDL_EMIT_MOREFUNCNAMES to try harder
  xdiff: add XDL_EMIT_MOREHUNKHEADS to split hunks
  t4213: test --function-name option
  Documentation: Document --function-name usage
 .gitattributes                 |   2 +-
 Documentation/diff-options.txt |   9 +++
 Documentation/gitdiffcore.txt  |  17 ++++-
 builtin/log.c                  |   2 +-
 diff.c                         |  13 +++-
 diff.h                         |   3 +
 diffcore-pickaxe.c             | 162 +++++++++++++++++++++++++++++++++++-----------
 revision.c                     |   3 +-
 t/t4213-log-function-name.sh   |  73 +++++++++++++++++++++
 xdiff/xdiff.h                  |   2 +
 xdiff/xdiffi.c                 |   2 +-
 xdiff/xemit.c                  |  99 ++++++++++++++++++++++------
 xdiff/xemit.h                  |   4 +-
 13 files changed, 323 insertions(+), 68 deletions(-)
David A. Dalrymple (and Bhushan G. Lodha)· Mar 27, 2014, 18:50 UTC · re: David A. Dalrymple (and Bhushan G. Lodha) · lore

[PATCH 01/10] .gitattributes: specify the language used

From: "Bhushan G. Lodha & David A. Dalrymple" <dad-bgl@mit.edu>

Since git can intelligently emit diff hunk headers based on the programming language of each file, assuming that the language is specified in .gitattributes, it makes sense to specify our own language (cpp) in our own .gitattributes file.

Signed-off-by: David Dalrymple (on zayin) <davidad@alum.mit.edu>
---
 .gitattributes | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to .gitattributes +1 −1
diff --git a/.gitattributes b/.gitattributes
index 5e98806..320e33c 100644
--- a/.gitattributes
+++ b/.gitattributes
@@ -1,3 +1,3 @@
 * whitespace=!indent,trail,space
-*.[ch] whitespace=indent,trail,space
+*.[ch] whitespace=indent,trail,space diff=cpp
 *.sh whitespace=indent,trail,space
-- 
1.7.12.4 (Apple Git-37)
David A. Dalrymple (and Bhushan G. Lodha)· Mar 27, 2014, 18:50 UTC · re: David A. Dalrymple (and Bhushan G. Lodha) · lore

[PATCH 02/10] diffcore-pickaxe.c: refactor regex compilation

From: "Bhushan G. Lodha & David A. Dalrymple" <dad-bgl@mit.edu>

In this file, two functions use identical blocks of code to call the POSIX regex compiling function and handle a possible error. Here we factor that block into its own function, in anticipation of using the same code a third time.

Signed-off-by: David Dalrymple (on zayin) <davidad@alum.mit.edu>
---
 diffcore-pickaxe.c | 33 ++++++++++++++++-----------------
 1 file changed, 16 insertions(+), 17 deletions(-)
Show changes to diffcore-pickaxe.c +16 −17
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index 401eb72..0d36a3c 100644
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -12,6 +12,8 @@ typedef int (*pickaxe_fn)(mmfile_t *one, mmfile_t *two,
 			  struct diff_options *o,
 			  regex_t *regexp, kwset_t kws);
 
+static void compile_regex(regex_t *r, const char *s, int cflags);
+
 static int pickaxe_match(struct diff_filepair *p, struct diff_options *o,
 			 regex_t *regexp, kwset_t kws, pickaxe_fn fn);
 
@@ -110,20 +112,13 @@ static int diff_grep(mmfile_t *one, mmfile_t *two,
 
 static void diffcore_pickaxe_grep(struct diff_options *o)
 {
-	int err;
 	regex_t regex;
 	int cflags = REG_EXTENDED | REG_NEWLINE;
 
 	if (DIFF_OPT_TST(o, PICKAXE_IGNORE_CASE))
 		cflags |= REG_ICASE;
 
-	err = regcomp(&regex, o->pickaxe, cflags);
-	if (err) {
-		char errbuf[1024];
-		regerror(err, &regex, errbuf, 1024);
-		regfree(&regex);
-		die("invalid regex: %s", errbuf);
-	}
+	compile_regex(&regex, o->pickaxe, cflags);
 
 	pickaxe(&diff_queued_diff, o, &regex, NULL, diff_grep);
 
@@ -180,6 +175,18 @@ static int has_changes(mmfile_t *one, mmfile_t *two,
 	return one_contains != two_contains;
 }
 
+static void compile_regex(regex_t *r, const char *s, int cflags)
+{
+	int err;
+	err = regcomp(r, s, cflags);
+	if (err) {
+		char errbuf[1024];
+		regerror(err, r, errbuf, 1024);
+		regfree(r);
+		die("invalid regex: %s", errbuf);
+	}
+}
+
 static int pickaxe_match(struct diff_filepair *p, struct diff_options *o,
 			 regex_t *regexp, kwset_t kws, pickaxe_fn fn)
 {
@@ -236,15 +243,7 @@ static void diffcore_pickaxe_count(struct diff_options *o)
 	kwset_t kws = NULL;
 
 	if (opts & DIFF_PICKAXE_REGEX) {
-		int err;
-		err = regcomp(&regex, needle, REG_EXTENDED | REG_NEWLINE);
-		if (err) {
-			/* The POSIX.2 people are surely sick */
-			char errbuf[1024];
-			regerror(err, &regex, errbuf, 1024);
-			regfree(&regex);
-			die("invalid regex: %s", errbuf);
-		}
+		compile_regex(&regex, needle, REG_EXTENDED | REG_NEWLINE);
 		regexp = &regex;
 	} else {
 		kws = kwsalloc(DIFF_OPT_TST(o, PICKAXE_IGNORE_CASE)
-- 
1.7.12.4 (Apple Git-37)
David A. Dalrymple (and Bhushan G. Lodha)· Mar 27, 2014, 18:50 UTC · re: David A. Dalrymple (and Bhushan G. Lodha) · lore

[PATCH 03/10] diffcore-pickaxe.c: Refactor pickaxe_fn signature

From: "Bhushan G. Lodha & David A. Dalrymple" <dad-bgl@mit.edu>

This function type previously accepted separate regex_t and kwset_t parameters, which conceptually go together. Here we create a struct to encapsulate them, in anticipation of adding a third field that pickaxe_fn's may require.

This parallels the existing diffgrep_cb structure for passing possibly relevant values through to the callbacks invoked by xdi_diff_outf.

Signed-off-by: David Dalrymple (on zayin) <davidad@alum.mit.edu>
---
 diffcore-pickaxe.c | 50 ++++++++++++++++++++++++++++++--------------------
 1 file changed, 30 insertions(+), 20 deletions(-)
Show changes to diffcore-pickaxe.c +30 −20
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index 0d36a3c..7e65095 100644
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -8,17 +8,22 @@
 #include "xdiff-interface.h"
 #include "kwset.h"
 
+struct fn_options {
+	regex_t *regex;
+	kwset_t kws;
+};
+
 typedef int (*pickaxe_fn)(mmfile_t *one, mmfile_t *two,
 			  struct diff_options *o,
-			  regex_t *regexp, kwset_t kws);
+			  struct fn_options *fno);
 
 static void compile_regex(regex_t *r, const char *s, int cflags);
 
 static int pickaxe_match(struct diff_filepair *p, struct diff_options *o,
-			 regex_t *regexp, kwset_t kws, pickaxe_fn fn);
+			 pickaxe_fn fn, struct fn_options *fno);
 
 static void pickaxe(struct diff_queue_struct *q, struct diff_options *o,
-		    regex_t *regexp, kwset_t kws, pickaxe_fn fn)
+		    pickaxe_fn fn, struct fn_options *fno)
 {
 	int i;
 	struct diff_queue_struct outq;
@@ -29,7 +34,7 @@ static void pickaxe(struct diff_queue_struct *q, struct diff_options *o,
 		/* Showing the whole changeset if needle exists */
 		for (i = 0; i < q->nr; i++) {
 			struct diff_filepair *p = q->queue[i];
-			if (pickaxe_match(p, o, regexp, kws, fn))
+			if (pickaxe_match(p, o, fn, fno))
 				return; /* do not munge the queue */
 		}
 
@@ -44,7 +49,7 @@ static void pickaxe(struct diff_queue_struct *q, struct diff_options *o,
 		/* Showing only the filepairs that has the needle */
 		for (i = 0; i < q->nr; i++) {
 			struct diff_filepair *p = q->queue[i];
-			if (pickaxe_match(p, o, regexp, kws, fn))
+			if (pickaxe_match(p, o, fn, fno))
 				diff_q(&outq, p);
 			else
 				diff_free_filepair(p);
@@ -83,7 +88,7 @@ static void diffgrep_consume(void *priv, char *line, unsigned long len)
 
 static int diff_grep(mmfile_t *one, mmfile_t *two,
 		     struct diff_options *o,
-		     regex_t *regexp, kwset_t kws)
+		     struct fn_options *fno)
 {
 	regmatch_t regmatch;
 	struct diffgrep_cb ecbdata;
@@ -91,9 +96,9 @@ static int diff_grep(mmfile_t *one, mmfile_t *two,
 	xdemitconf_t xecfg;
 
 	if (!one)
-		return !regexec(regexp, two->ptr, 1, &regmatch, 0);
+		return !regexec(fno->regex, two->ptr, 1, &regmatch, 0);
 	if (!two)
-		return !regexec(regexp, one->ptr, 1, &regmatch, 0);
+		return !regexec(fno->regex, one->ptr, 1, &regmatch, 0);
 
 	/*
 	 * We have both sides; need to run textual diff and see if
@@ -101,7 +106,7 @@ static int diff_grep(mmfile_t *one, mmfile_t *two,
 	 */
 	memset(&xpp, 0, sizeof(xpp));
 	memset(&xecfg, 0, sizeof(xecfg));
-	ecbdata.regexp = regexp;
+	ecbdata.regexp = fno->regex;
 	ecbdata.hit = 0;
 	xecfg.ctxlen = o->context;
 	xecfg.interhunkctxlen = o->interhunkcontext;
@@ -113,6 +118,7 @@ static int diff_grep(mmfile_t *one, mmfile_t *two,
 static void diffcore_pickaxe_grep(struct diff_options *o)
 {
 	regex_t regex;
+	struct fn_options fno;
 	int cflags = REG_EXTENDED | REG_NEWLINE;
 
 	if (DIFF_OPT_TST(o, PICKAXE_IGNORE_CASE))
@@ -120,13 +126,14 @@ static void diffcore_pickaxe_grep(struct diff_options *o)
 
 	compile_regex(&regex, o->pickaxe, cflags);
 
-	pickaxe(&diff_queued_diff, o, &regex, NULL, diff_grep);
+	fno.regex = &regex;
+	pickaxe(&diff_queued_diff, o, diff_grep, &fno);
 
 	regfree(&regex);
 	return;
 }
 
-static unsigned int contains(mmfile_t *mf, regex_t *regexp, kwset_t kws)
+static unsigned int contains(mmfile_t *mf, struct fn_options *fno)
 {
 	unsigned int cnt;
 	unsigned long sz;
@@ -136,12 +143,12 @@ static unsigned int contains(mmfile_t *mf, regex_t *regexp, kwset_t kws)
 	data = mf->ptr;
 	cnt = 0;
 
-	if (regexp) {
+	if (fno->regex) {
 		regmatch_t regmatch;
 		int flags = 0;
 
 		assert(data[sz] == '\0');
-		while (*data && !regexec(regexp, data, 1, &regmatch, flags)) {
+		while (*data && !regexec(fno->regex, data, 1, &regmatch, flags)) {
 			flags |= REG_NOTBOL;
 			data += regmatch.rm_eo;
 			if (*data && regmatch.rm_so == regmatch.rm_eo)
@@ -152,7 +159,7 @@ static unsigned int contains(mmfile_t *mf, regex_t *regexp, kwset_t kws)
 	} else { /* Classic exact string match */
 		while (sz) {
 			struct kwsmatch kwsm;
-			size_t offset = kwsexec(kws, data, sz, &kwsm);
+			size_t offset = kwsexec(fno->kws, data, sz, &kwsm);
 			const char *found;
 			if (offset == -1)
 				break;
@@ -168,10 +175,10 @@ static unsigned int contains(mmfile_t *mf, regex_t *regexp, kwset_t kws)
 
 static int has_changes(mmfile_t *one, mmfile_t *two,
 		       struct diff_options *o,
-		       regex_t *regexp, kwset_t kws)
+		       struct fn_options *fno)
 {
-	unsigned int one_contains = one ? contains(one, regexp, kws) : 0;
-	unsigned int two_contains = two ? contains(two, regexp, kws) : 0;
+	unsigned int one_contains = one ? contains(one, fno) : 0;
+	unsigned int two_contains = two ? contains(two, fno) : 0;
 	return one_contains != two_contains;
 }
 
@@ -188,7 +195,7 @@ static void compile_regex(regex_t *r, const char *s, int cflags)
 }
 
 static int pickaxe_match(struct diff_filepair *p, struct diff_options *o,
-			 regex_t *regexp, kwset_t kws, pickaxe_fn fn)
+			 pickaxe_fn fn, struct fn_options *fno)
 {
 	struct userdiff_driver *textconv_one = NULL;
 	struct userdiff_driver *textconv_two = NULL;
@@ -222,7 +229,7 @@ static int pickaxe_match(struct diff_filepair *p, struct diff_options *o,
 
 	ret = fn(DIFF_FILE_VALID(p->one) ? &mf1 : NULL,
 		 DIFF_FILE_VALID(p->two) ? &mf2 : NULL,
-		 o, regexp, kws);
+		 o, fno);
 
 	if (textconv_one)
 		free(mf1.ptr);
@@ -236,6 +243,7 @@ static int pickaxe_match(struct diff_filepair *p, struct diff_options *o,
 
 static void diffcore_pickaxe_count(struct diff_options *o)
 {
+	struct fn_options fno;
 	const char *needle = o->pickaxe;
 	int opts = o->pickaxe_opts;
 	unsigned long len = strlen(needle);
@@ -252,7 +260,9 @@ static void diffcore_pickaxe_count(struct diff_options *o)
 		kwsprep(kws);
 	}
 
-	pickaxe(&diff_queued_diff, o, regexp, kws, has_changes);
+	fno.regex = regexp;
+	fno.kws = kws;
+	pickaxe(&diff_queued_diff, o, has_changes, &fno);
 
 	if (opts & DIFF_PICKAXE_REGEX)
 		regfree(&regex);
-- 
1.7.12.4 (Apple Git-37)
Jakub Narębski· Apr 4, 2014, 11:09 UTC · re: David A. Dalrymple (and Bhushan G. Lodha) · lore

Re: [PATCH 03/10] diffcore-pickaxe.c: Refactor pickaxe_fn signature

W dniu 2014-03-27 19:50, David A. Dalrymple (and Bhushan G. Lodha) pisze:
Show 9 quoted lines
> From: "Bhushan G. Lodha & David A. Dalrymple" <dad-bgl@mit.edu>
>
> This function type previously accepted separate regex_t and kwset_t
> parameters, which conceptually go together. Here we create a struct to
> encapsulate them, in anticipation of adding a third field that
> pickaxe_fn's may require.
>
> This parallels the existing diffgrep_cb structure for passing possibly
> relevant values through to the callbacks invoked by xdi_diff_outf.

If it parallels existing diffgrep_cb structure, why not name this equivalent in simular way, i.e. pickaxe_cb or pickaxe_options or pickaxe_cb_opts instead of generic name fn_options?

Show 17 quoted lines
> Signed-off-by: David Dalrymple (on zayin) <davidad@alum.mit.edu>
> ---
>   diffcore-pickaxe.c | 50 ++++++++++++++++++++++++++++++--------------------
>   1 file changed, 30 insertions(+), 20 deletions(-)
>
> diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
> index 0d36a3c..7e65095 100644
> --- a/diffcore-pickaxe.c
> +++ b/diffcore-pickaxe.c
> @@ -8,17 +8,22 @@
>   #include "xdiff-interface.h"
>   #include "kwset.h"
>
> +struct fn_options {
> +	regex_t *regex;
> +	kwset_t kws;
> +};
David A. Dalrymple (and Bhushan G. Lodha)· Mar 27, 2014, 18:50 UTC · re: David A. Dalrymple (and Bhushan G. Lodha) · lore

[PATCH 04/10] diff.c/diff.h: expose userdiff_funcname

From: "Bhushan G. Lodha & David A. Dalrymple" <dad-bgl@mit.edu>

The functionality of userdiff_funcname (determining the language in use for a given file and setting up patterns to match "function names" in that language) is useful outside of diff.c, so here we remove its static specifier and declare it in diff.h.

Signed-off-by: David Dalrymple (on zayin) <davidad@alum.mit.edu>
---
 diff.c | 2 +-
 diff.h | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)
Show changes to 2 files +3 −1

diff.c, diff.h

diff --git a/diff.c b/diff.c
index e343191..f978ee7 100644
--- a/diff.c
+++ b/diff.c
@@ -2203,7 +2203,7 @@ int diff_filespec_is_binary(struct diff_filespec *one)
 	return one->is_binary;
 }
 
-static const struct userdiff_funcname *diff_funcname_pattern(struct diff_filespec *one)
+const struct userdiff_funcname *diff_funcname_pattern(struct diff_filespec *one)
 {
 	diff_filespec_load_driver(one);
 	return one->driver->funcname.pattern ? &one->driver->funcname : NULL;
diff --git a/diff.h b/diff.h
index a24a767..9e96fc9 100644
--- a/diff.h
+++ b/diff.h
@@ -349,4 +349,6 @@ extern int print_stat_summary(FILE *fp, int files,
 			      int insertions, int deletions);
 extern void setup_diff_pager(struct diff_options *);
 
+const struct userdiff_funcname *diff_funcname_pattern(struct diff_filespec *);
+
 #endif /* DIFF_H */
-- 
1.7.12.4 (Apple Git-37)
David A. Dalrymple (and Bhushan G. Lodha)· Mar 27, 2014, 18:50 UTC · re: David A. Dalrymple (and Bhushan G. Lodha) · lore

[PATCH 05/10] diffcore-pickaxe.c: set up funcname pattern

From: "Bhushan G. Lodha & David A. Dalrymple" <dad-bgl@mit.edu>

We use userdiff_funcname to make the filetype-dependent function name pattern available to pickaxe functions.

Signed-off-by: David Dalrymple (on zayin) <davidad@alum.mit.edu>
---
 diffcore-pickaxe.c | 9 +++++++++
 1 file changed, 9 insertions(+)
Show changes to diffcore-pickaxe.c +9 −0
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index 7e65095..103fe6c 100644
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -7,10 +7,12 @@
 #include "diffcore.h"
 #include "xdiff-interface.h"
 #include "kwset.h"
+#include "userdiff.h"
 
 struct fn_options {
 	regex_t *regex;
 	kwset_t kws;
+	const struct userdiff_funcname *funcname_pattern;
 };
 
 typedef int (*pickaxe_fn)(mmfile_t *one, mmfile_t *two,
@@ -224,6 +226,13 @@ static int pickaxe_match(struct diff_filepair *p, struct diff_options *o,
 	if (textconv_one == textconv_two && diff_unmodified_pair(p))
 		return 0;
 
+	const struct userdiff_funcname *funcname_pattern;
+	funcname_pattern = diff_funcname_pattern(p->one);
+	if (!funcname_pattern)
+		funcname_pattern = diff_funcname_pattern(p->two);
+
+	fno->funcname_pattern = funcname_pattern;
+
 	mf1.size = fill_textconv(textconv_one, p->one, &mf1.ptr);
 	mf2.size = fill_textconv(textconv_two, p->two, &mf2.ptr);
 
-- 
1.7.12.4 (Apple Git-37)
David A. Dalrymple (and Bhushan G. Lodha)· Mar 27, 2014, 18:50 UTC · re: David A. Dalrymple (and Bhushan G. Lodha) · lore

[PATCH 06/10] log: --function-name pickaxe

From: "Bhushan G. Lodha & David A. Dalrymple" <dad-bgl@mit.edu>

This is similar to the pickaxe grep option (-G), but applies the provided regex only to diff hunk headers, thereby showing only those commits which affect a "function" with a definition line matching the pattern. These are "functions" in the same sense as with --function-context, i.e., they may be classes, structs, etc. depending on the programming-language-specific pattern specified by the "diff" attribute in .gitattributes.

builtin/log.c:
	as with pickaxe, set always_show_header when using --function-name
diff.c:
	parse option; as with pickaxe, always set the RECURSIVE option
	for --function-name
diff.h:
	include "funcname" field in struct diff_options
diffcore-pickaxe.c:
	implementation of --function-name filtering (diffcore_funcname), like
	the existing diffcore_pickaxe_grep and diffcore_pickaxe_count
revision.c:
	as with pickaxe, set revs->diff to always generate diffs when
	using --function-name
Signed-off-by: David Dalrymple (on zayin) <davidad@alum.mit.edu>
---
 builtin/log.c      |  2 +-
 diff.c             |  8 +++++--
 diff.h             |  1 +
 diffcore-pickaxe.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 revision.c         |  3 ++-
 5 files changed, 77 insertions(+), 6 deletions(-)
Show changes to 5 files +77 −6

builtin/log.c, diff.c, diff.h, diffcore-pickaxe.c, revision.c

diff --git a/builtin/log.c b/builtin/log.c
index b97373d..78694de 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -158,7 +158,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 	if (rev->show_notes)
 		init_display_notes(&rev->notes_opt);
 
-	if (rev->diffopt.pickaxe || rev->diffopt.filter)
+	if (rev->diffopt.pickaxe || rev->diffopt.filter || rev->diffopt.funcname)
 		rev->always_show_header = 0;
 	if (DIFF_OPT_TST(&rev->diffopt, FOLLOW_RENAMES)) {
 		rev->always_show_header = 0;
diff --git a/diff.c b/diff.c
index f978ee7..2f6dbc1 100644
--- a/diff.c
+++ b/diff.c
@@ -3298,7 +3298,7 @@ void diff_setup_done(struct diff_options *options)
 	/*
 	 * Also pickaxe would not work very well if you do not say recursive
 	 */
-	if (options->pickaxe)
+	if (options->pickaxe || options->funcname)
 		DIFF_OPT_SET(options, RECURSIVE);
 	/*
 	 * When patches are generated, submodules diffed against the work tree
@@ -3821,6 +3821,10 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		options->orderfile = optarg;
 		return argcount;
 	}
+	else if ((argcount = parse_long_opt("function-name", av, &optarg))) {
+		options->funcname = optarg;
+		return argcount;
+	}
 	else if ((argcount = parse_long_opt("diff-filter", av, &optarg))) {
 		int offending = parse_diff_filter_opt(optarg, options);
 		if (offending)
@@ -4768,7 +4772,7 @@ void diffcore_std(struct diff_options *options)
 		if (options->break_opt != -1)
 			diffcore_merge_broken();
 	}
-	if (options->pickaxe)
+	if (options->pickaxe || options->funcname)
 		diffcore_pickaxe(options);
 	if (options->orderfile)
 		diffcore_order(options->orderfile);
diff --git a/diff.h b/diff.h
index 9e96fc9..0fd5f1d 100644
--- a/diff.h
+++ b/diff.h
@@ -107,6 +107,7 @@ enum diff_words_type {
 struct diff_options {
 	const char *orderfile;
 	const char *pickaxe;
+	const char *funcname;
 	const char *single_follow;
 	const char *a_prefix, *b_prefix;
 	unsigned flags;
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index 103fe6c..259a8fa 100644
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -67,6 +67,12 @@ struct diffgrep_cb {
 	int hit;
 };
 
+struct funcname_cb {
+	struct userdiff_funcname *pattern;
+	regex_t *regex;
+	int hit;
+};
+
 static void diffgrep_consume(void *priv, char *line, unsigned long len)
 {
 	struct diffgrep_cb *data = priv;
@@ -88,6 +94,20 @@ static void diffgrep_consume(void *priv, char *line, unsigned long len)
 	line[len] = hold;
 }
 
+static void match_funcname(void *priv, char *line, unsigned long len)
+{
+	regmatch_t regmatch;
+	int hold;
+	struct funcname_cb *data = priv;
+	hold = line[len];
+	line[len] = '\0';
+
+	if (line[0] == '@' && line[1] == '@')
+		if (!regexec(data->regex, line, 1, &regmatch, 0))
+			data->hit = 1;
+	line[len] = hold;
+}
+
 static int diff_grep(mmfile_t *one, mmfile_t *two,
 		     struct diff_options *o,
 		     struct fn_options *fno)
@@ -117,6 +137,38 @@ static int diff_grep(mmfile_t *one, mmfile_t *two,
 	return ecbdata.hit;
 }
 
+static int diff_funcname_filter(mmfile_t *one, mmfile_t *two,
+				struct diff_options *o,
+				struct fn_options *fno)
+{
+	struct funcname_cb ecbdata;
+	xpparam_t xpp;
+	xdemitconf_t xecfg;
+
+	mmfile_t empty;
+	empty.ptr = "";
+	empty.size = 0;
+	if (!one)
+		one = &empty;
+	if (!two)
+		two = &empty;
+	memset(&xpp, 0, sizeof(xpp));
+	memset(&xecfg, 0, sizeof(xecfg));
+	ecbdata.regex = fno->regex;
+	ecbdata.hit = 0;
+	xecfg.ctxlen = o->context;
+
+	if (fno->funcname_pattern)
+		xdiff_set_find_func(&xecfg, fno->funcname_pattern->pattern,
+					    fno->funcname_pattern->cflags);
+	xecfg.interhunkctxlen = o->interhunkcontext;
+	if (!(one && two))
+		xecfg.flags = XDL_EMIT_FUNCCONTEXT;
+	xecfg.flags |= XDL_EMIT_FUNCNAMES;
+	xdi_diff_outf(one, two, match_funcname, &ecbdata, &xpp, &xecfg);
+	return ecbdata.hit;
+}
+
 static void diffcore_pickaxe_grep(struct diff_options *o)
 {
 	regex_t regex;
@@ -204,7 +256,7 @@ static int pickaxe_match(struct diff_filepair *p, struct diff_options *o,
 	mmfile_t mf1, mf2;
 	int ret;
 
-	if (!o->pickaxe[0])
+	if (o->pickaxe && !o->pickaxe[0])
 		return 0;
 
 	/* ignore unmerged */
@@ -280,11 +332,24 @@ static void diffcore_pickaxe_count(struct diff_options *o)
 	return;
 }
 
+static void diffcore_funcname(struct diff_options *o)
+{
+	struct fn_options fno;
+	regex_t regex;
+
+	fno.regex = &regex;
+	compile_regex(&regex, o->funcname, REG_EXTENDED | REG_NEWLINE);
+	pickaxe(&diff_queued_diff, o, diff_funcname_filter, &fno);
+	regfree(&regex);
+}
+
 void diffcore_pickaxe(struct diff_options *o)
 {
+	if (o->funcname)
+		diffcore_funcname(o);
 	/* Might want to warn when both S and G are on; I don't care... */
 	if (o->pickaxe_opts & DIFF_PICKAXE_KIND_G)
 		diffcore_pickaxe_grep(o);
-	else
+	else if (o-> pickaxe_opts & DIFF_PICKAXE_KIND_S)
 		diffcore_pickaxe_count(o);
 }
diff --git a/revision.c b/revision.c
index 8508550..d81d9b9 100644
--- a/revision.c
+++ b/revision.c
@@ -2211,8 +2211,9 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 	if (revs->diffopt.output_format & ~DIFF_FORMAT_NO_OUTPUT)
 		revs->diff = 1;
 
-	/* Pickaxe, diff-filter and rename following need diffs */
+	/* Pickaxe, funcname, diff-filter and rename following need diffs */
 	if (revs->diffopt.pickaxe ||
+	    revs->diffopt.funcname ||
 	    revs->diffopt.filter ||
 	    DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES))
 		revs->diff = 1;
-- 
1.7.12.4 (Apple Git-37)
René Scharfe· Apr 3, 2014, 21:25 UTC · re: David A. Dalrymple (and Bhushan G. Lodha) · lore

Re: [PATCH 06/10] log: --function-name pickaxe

Am 27.03.2014 19:50, schrieb David A. Dalrymple (and Bhushan G. Lodha):
Show 9 quoted lines
> From: "Bhushan G. Lodha & David A. Dalrymple" <dad-bgl@mit.edu>
>
> This is similar to the pickaxe grep option (-G), but applies the
> provided regex only to diff hunk headers, thereby showing only those
> commits which affect a "function" with a definition line matching the
> pattern. These are "functions" in the same sense as with
> --function-context, i.e., they may be classes, structs, etc. depending
> on the programming-language-specific pattern specified by the "diff"
> attribute in .gitattributes.

With that approach you depend on the hunk header and apparently need to add XDL_EMIT_MOREFUNCNAMES and XDL_EMIT_MOREHUNKHEADS to improve the results. This approach feels fragile.

Would it perhaps be more robust to not base the implementation on diff and instead to scan the raw file contents? You'd search both files for a matching function signature, then search for a non-matching one from there. The parts in between are function bodies and can be compared. If they match, you'd search for matching function starts again etc.

Or would it make sense to make use of the diff option FUNCCONTEXT (git diff -W) and look for the function signature in the diff body instead of in the hunk header? Such a diff contains whole functions, but a single hunk could contain multiple ones.

René
Junio C Hamano· Apr 3, 2014, 21:44 UTC · re: René Scharfe · lore

Re: [PATCH 06/10] log: --function-name pickaxe

René Scharfe <l.s.r@web.de> writes:
Show 6 quoted lines
> With that approach you depend on the hunk header and apparently need
> to add XDL_EMIT_MOREFUNCNAMES and XDL_EMIT_MOREHUNKHEADS to improve
> the results.  This approach feels fragile.
>
> Would it perhaps be more robust to not base the implementation on diff
> and instead to scan the raw file contents?
That is an interesting idea.

Perhaps this can be implemented as a new stage in the transformation pipeline, I wonder? There is currently no transformation that modifies the blob contents being compared, but I do not think there is anything fundamental that prevents one from being written. The new "limit to this function body" transformation would perhaps sit before the diffcore-rename and would transform all the blobs to empty, except for the part that is the body of the function the user is interested in.

Jakub Narębski· Apr 4, 2014, 11:15 UTC · re: Junio C Hamano · lore

Re: [PATCH 06/10] log: --function-name pickaxe

W dniu 2014-04-03 23:44, Junio C Hamano pisze:
Show 19 quoted lines
> René Scharfe <l.s.r@web.de> writes:
>
>> With that approach you depend on the hunk header and apparently need
>> to add XDL_EMIT_MOREFUNCNAMES and XDL_EMIT_MOREHUNKHEADS to improve
>> the results.  This approach feels fragile.
>>
>> Would it perhaps be more robust to not base the implementation on diff
>> and instead to scan the raw file contents?
>
> That is an interesting idea.
>
> Perhaps this can be implemented as a new stage in the transformation
> pipeline, I wonder?  There is currently no transformation that
> modifies the blob contents being compared, but I do not think there
> is anything fundamental that prevents one from being written.  The
> new "limit to this function body" transformation would perhaps sit
> before the diffcore-rename and would transform all the blobs to
> empty, except for the part that is the body of the function the user
> is interested in.
Well, there is 'texconv', e.g.
   .gitattributes
   *.jpg diff=jpg
   .git/config
   [diff "jpg"]
          textconv = exif
Doesn't it fit in said place in the transformation pipeline?
-- 
Jakub Narębski
Junio C Hamano· Apr 4, 2014, 18:46 UTC · re: Jakub Narębski · lore

Re: [PATCH 06/10] log: --function-name pickaxe

Jakub Narębski <jnareb@gmail.com> writes:
Show 29 quoted lines
> W dniu 2014-04-03 23:44, Junio C Hamano pisze:
>> René Scharfe <l.s.r@web.de> writes:
>>
>>> With that approach you depend on the hunk header and apparently need
>>> to add XDL_EMIT_MOREFUNCNAMES and XDL_EMIT_MOREHUNKHEADS to improve
>>> the results.  This approach feels fragile.
>>>
>>> Would it perhaps be more robust to not base the implementation on diff
>>> and instead to scan the raw file contents?
>>
>> That is an interesting idea.
>>
>> Perhaps this can be implemented as a new stage in the transformation
>> pipeline, I wonder?  There is currently no transformation that
>> modifies the blob contents being compared, but I do not think there
>> is anything fundamental that prevents one from being written.  The
>> new "limit to this function body" transformation would perhaps sit
>> before the diffcore-rename and would transform all the blobs to
>> empty, except for the part that is the body of the function the user
>> is interested in.
>
> Well, there is 'texconv', e.g.
>
>   .gitattributes
>   *.jpg diff=jpg
>
>   .git/config
>   [diff "jpg"]
>          textconv = exif
;-)  So you could define this textconv
    sed -n -e '/^int main(/,/^}/p'
to limit the output only to the definition of the function main().
> Doesn't it fit in said place in the transformation pipeline?

Not at all, unfortunately. The textconv conversion happens in the final output stage and comes way too late to influence the earlier stages like renames and pickaxe, which will still see the whole contents outside the definition of the function main().

Bhushan Lodha· Apr 28, 2014, 20:04 UTC · re: Junio C Hamano · lore

Re: [PATCH 06/10] log: --function-name pickaxe

I plan to work on this in few weeks. If anybody has more suggestion or want to discuss the implementation let me know

On Fri, Apr 4, 2014 at 2:46 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 47 quoted lines
> Jakub Narębski <jnareb@gmail.com> writes:
>
>> W dniu 2014-04-03 23:44, Junio C Hamano pisze:
>>> René Scharfe <l.s.r@web.de> writes:
>>>
>>>> With that approach you depend on the hunk header and apparently need
>>>> to add XDL_EMIT_MOREFUNCNAMES and XDL_EMIT_MOREHUNKHEADS to improve
>>>> the results.  This approach feels fragile.
>>>>
>>>> Would it perhaps be more robust to not base the implementation on diff
>>>> and instead to scan the raw file contents?
>>>
>>> That is an interesting idea.
>>>
>>> Perhaps this can be implemented as a new stage in the transformation
>>> pipeline, I wonder?  There is currently no transformation that
>>> modifies the blob contents being compared, but I do not think there
>>> is anything fundamental that prevents one from being written.  The
>>> new "limit to this function body" transformation would perhaps sit
>>> before the diffcore-rename and would transform all the blobs to
>>> empty, except for the part that is the body of the function the user
>>> is interested in.
>>
>> Well, there is 'texconv', e.g.
>>
>>   .gitattributes
>>   *.jpg diff=jpg
>>
>>   .git/config
>>   [diff "jpg"]
>>          textconv = exif
>
> ;-)  So you could define this textconv
>
>     sed -n -e '/^int main(/,/^}/p'
>
> to limit the output only to the definition of the function main().
>
>> Doesn't it fit in said place in the transformation pipeline?
>
> Not at all, unfortunately.  The textconv conversion happens in the
> final output stage and comes way too late to influence the earlier
> stages like renames and pickaxe, which will still see the whole
> contents outside the definition of the function main().
>
>
>
David A. Dalrymple (and Bhushan G. Lodha)· Mar 27, 2014, 18:50 UTC · re: David A. Dalrymple (and Bhushan G. Lodha) · lore

[PATCH 07/10] xdiff: add XDL_EMIT_MOREFUNCNAMES

From: "Bhushan G. Lodha & David A. Dalrymple" <dad-bgl@mit.edu>

For filtering commits by function name, it's useful to identify the function name in cases such as adding a new function to a file (where the default functionality will not emit a function name in the hunk header, because it isn't part of the context).

This adds a flag asking xdiff to be more aggressive in finding function names to emit, and turns the flag on when the --function-name option is in use.

Signed-off-by: David Dalrymple (on zayin) <davidad@alum.mit.edu>
---
 diff.c             |  2 ++
 diffcore-pickaxe.c |  2 +-
 xdiff/xdiff.h      |  1 +
 xdiff/xemit.c      | 39 +++++++++++++++++++++++----------------
 4 files changed, 27 insertions(+), 17 deletions(-)
Show changes to 4 files +27 −17

diff.c, diffcore-pickaxe.c, xdiff/xdiff.h, xdiff/xemit.c

diff --git a/diff.c b/diff.c
index 2f6dbc1..914b4a2 100644
--- a/diff.c
+++ b/diff.c
@@ -2380,6 +2380,8 @@ static void builtin_diff(const char *name_a,
 		xecfg.ctxlen = o->context;
 		xecfg.interhunkctxlen = o->interhunkcontext;
 		xecfg.flags = XDL_EMIT_FUNCNAMES;
+		if (o->funcname)
+			xecfg.flags |= XDL_EMIT_MOREFUNCNAMES;
 		if (DIFF_OPT_TST(o, FUNCCONTEXT))
 			xecfg.flags |= XDL_EMIT_FUNCCONTEXT;
 		if (pe)
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index 259a8fa..ab31c18 100644
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -164,7 +164,7 @@ static int diff_funcname_filter(mmfile_t *one, mmfile_t *two,
 	xecfg.interhunkctxlen = o->interhunkcontext;
 	if (!(one && two))
 		xecfg.flags = XDL_EMIT_FUNCCONTEXT;
-	xecfg.flags |= XDL_EMIT_FUNCNAMES;
+	xecfg.flags |= XDL_EMIT_FUNCNAMES | XDL_EMIT_MOREFUNCNAMES;
 	xdi_diff_outf(one, two, match_funcname, &ecbdata, &xpp, &xecfg);
 	return ecbdata.hit;
 }
diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h
index c033991..469bded 100644
--- a/xdiff/xdiff.h
+++ b/xdiff/xdiff.h
@@ -44,6 +44,7 @@
 #define XDL_EMIT_FUNCNAMES (1 << 0)
 #define XDL_EMIT_COMMON (1 << 1)
 #define XDL_EMIT_FUNCCONTEXT (1 << 2)
+#define XDL_EMIT_MOREFUNCNAMES (1 << 3)
 
 #define XDL_MMB_READONLY (1 << 0)
 
diff --git a/xdiff/xemit.c b/xdiff/xemit.c
index 4266ada..0ddb094 100644
--- a/xdiff/xemit.c
+++ b/xdiff/xemit.c
@@ -23,6 +23,10 @@
 #include "xinclude.h"
 
 
+struct func_line {
+	long len;
+	char buf[80];
+};
 
 
 static long xdl_get_rec(xdfile_t *xdf, long ri, char const **rec);
@@ -135,12 +139,7 @@ static int xdl_emit_common(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,
 	return 0;
 }
 
-struct func_line {
-	long len;
-	char buf[80];
-};
-
-static long get_func_line(xdfenv_t *xe, xdemitconf_t const *xecfg,
+static long get_func_line(xdfile_t *xdf, xdemitconf_t const *xecfg,
 			  struct func_line *func_line, long start, long limit)
 {
 	find_func_t ff = xecfg->find_func ? xecfg->find_func : def_ff;
@@ -150,9 +149,9 @@ static long get_func_line(xdfenv_t *xe, xdemitconf_t const *xecfg,
 	buf = func_line ? func_line->buf : dummy;
 	size = func_line ? sizeof(func_line->buf) : sizeof(dummy);
 
-	for (l = start; l != limit && 0 <= l && l < xe->xdf1.nrec; l += step) {
+	for (l = start; l != limit && 0 <= l && l < xdf->nrec; l += step) {
 		const char *rec;
-		long reclen = xdl_get_rec(&xe->xdf1, l, &rec);
+		long reclen = xdl_get_rec(xdf, l, &rec);
 		long len = ff(rec, reclen, buf, size, xecfg->find_func_priv);
 		if (len >= 0) {
 			if (func_line)
@@ -167,7 +166,7 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,
 		  xdemitconf_t const *xecfg) {
 	long s1, s2, e1, e2, lctx;
 	xdchange_t *xch, *xche;
-	long funclineprev = -1;
+	long funclineprev1 = -1, funclineprev2 = -1;
 	struct func_line func_line = { 0 };
 
 	if (xecfg->flags & XDL_EMIT_COMMON)
@@ -182,7 +181,7 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,
 		s2 = XDL_MAX(xch->i2 - xecfg->ctxlen, 0);
 
 		if (xecfg->flags & XDL_EMIT_FUNCCONTEXT) {
-			long fs1 = get_func_line(xe, xecfg, NULL, xch->i1, -1);
+			long fs1 = get_func_line(&xe->xdf1, xecfg, NULL, xch->i1, -1);
 			if (fs1 < 0)
 				fs1 = 0;
 			if (fs1 < s1) {
@@ -200,7 +199,7 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,
 		e2 = xche->i2 + xche->chg2 + lctx;
 
 		if (xecfg->flags & XDL_EMIT_FUNCCONTEXT) {
-			long fe1 = get_func_line(xe, xecfg, NULL,
+			long fe1 = get_func_line(&xe->xdf1, xecfg, NULL,
 						 xche->i1 + xche->chg1,
 						 xe->xdf1.nrec);
 			if (fe1 < 0)
@@ -218,7 +217,7 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,
 			if (xche->next) {
 				long l = xche->next->i1;
 				if (l <= e1 ||
-				    get_func_line(xe, xecfg, NULL, l, e1) < 0) {
+				    get_func_line(&xe->xdf1, xecfg, NULL, l, e1) < 0) {
 					xche = xche->next;
 					goto again;
 				}
@@ -229,10 +228,18 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,
 		 * Emit current hunk header.
 		 */
 
-		if (xecfg->flags & XDL_EMIT_FUNCNAMES) {
-			get_func_line(xe, xecfg, &func_line,
-				      s1 - 1, funclineprev);
-			funclineprev = s1 - 1;
+		if (xecfg->flags & XDL_EMIT_MOREFUNCNAMES) {
+			long fl_in_xch1 = get_func_line(&xe->xdf1, xecfg,
+					&func_line, xch->i1, xch->i1+xch->chg1);
+			if (fl_in_xch1 < 0) {
+				get_func_line(&xe->xdf2, xecfg, &func_line,
+					      xch->i2, funclineprev2);
+				funclineprev2 = xch->i2;
+			}
+		} else if (xecfg->flags & XDL_EMIT_FUNCNAMES) {
+			get_func_line(&xe->xdf1, xecfg, &func_line,
+				      s1 - 1, funclineprev1);
+			funclineprev1 = s1 - 1;
 		}
 		if (xdl_emit_hunk_hdr(s1 + 1, e1 - s1, s2 + 1, e2 - s2,
 				      func_line.buf, func_line.len, ecb) < 0)
-- 
1.7.12.4 (Apple Git-37)
David A. Dalrymple (and Bhushan G. Lodha)· Mar 27, 2014, 18:50 UTC · re: David A. Dalrymple (and Bhushan G. Lodha) · lore

[PATCH 08/10] xdiff: add XDL_EMIT_MOREHUNKHEADS

From: "Bhushan G. Lodha & David A. Dalrymple" <dad-bgl@mit.edu>

For filtering by function names, it's useful to split hunks whenever a function line is encountered, so that each function name being deleted or inserted gets its own hunk header (which then can be easily detected by the filter).

This adds a flag, XDL_EMIT_MOREHUNKHEADS, which triggers this nonstandard behavior, and enables it only in case the --function-name option is being used.

Signed-off-by: David Dalrymple (on zayin) <davidad@alum.mit.edu>
---
 diff.c             |  3 ++-
 diffcore-pickaxe.c |  3 ++-
 xdiff/xdiff.h      |  1 +
 xdiff/xdiffi.c     |  2 +-
 xdiff/xemit.c      | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 xdiff/xemit.h      |  4 +++-
 6 files changed, 67 insertions(+), 6 deletions(-)
Show changes to 6 files +67 −6

diff.c, diffcore-pickaxe.c, xdiff/xdiff.h, xdiff/xdiffi.c, xdiff/xemit.c, xdiff/xemit.h

diff --git a/diff.c b/diff.c
index 914b4a2..a86206c 100644
--- a/diff.c
+++ b/diff.c
@@ -2381,7 +2381,8 @@ static void builtin_diff(const char *name_a,
 		xecfg.interhunkctxlen = o->interhunkcontext;
 		xecfg.flags = XDL_EMIT_FUNCNAMES;
 		if (o->funcname)
-			xecfg.flags |= XDL_EMIT_MOREFUNCNAMES;
+			xecfg.flags |= XDL_EMIT_MOREFUNCNAMES
+				    | XDL_EMIT_MOREHUNKHEADS;
 		if (DIFF_OPT_TST(o, FUNCCONTEXT))
 			xecfg.flags |= XDL_EMIT_FUNCCONTEXT;
 		if (pe)
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index ab31c18..d9f4c30 100644
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -164,7 +164,8 @@ static int diff_funcname_filter(mmfile_t *one, mmfile_t *two,
 	xecfg.interhunkctxlen = o->interhunkcontext;
 	if (!(one && two))
 		xecfg.flags = XDL_EMIT_FUNCCONTEXT;
-	xecfg.flags |= XDL_EMIT_FUNCNAMES | XDL_EMIT_MOREFUNCNAMES;
+	xecfg.flags |= XDL_EMIT_FUNCNAMES | XDL_EMIT_MOREFUNCNAMES
+		    | XDL_EMIT_MOREHUNKHEADS;
 	xdi_diff_outf(one, two, match_funcname, &ecbdata, &xpp, &xecfg);
 	return ecbdata.hit;
 }
diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h
index 469bded..787c376 100644
--- a/xdiff/xdiff.h
+++ b/xdiff/xdiff.h
@@ -45,6 +45,7 @@
 #define XDL_EMIT_COMMON (1 << 1)
 #define XDL_EMIT_FUNCCONTEXT (1 << 2)
 #define XDL_EMIT_MOREFUNCNAMES (1 << 3)
+#define XDL_EMIT_MOREHUNKHEADS (1 << 4)
 
 #define XDL_MMB_READONLY (1 << 0)
 
diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c
index 2358a2d..c29804e 100644
--- a/xdiff/xdiffi.c
+++ b/xdiff/xdiffi.c
@@ -545,7 +545,7 @@ static int xdl_call_hunk_func(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,
 	xdchange_t *xch, *xche;
 
 	for (xch = xscr; xch; xch = xche->next) {
-		xche = xdl_get_hunk(&xch, xecfg);
+		xche = xdl_get_hunk(xe, &xch, xecfg);
 		if (!xch)
 			break;
 		if (xecfg->hunk_func(xch->i1, xche->i1 + xche->chg1 - xch->i1,
diff --git a/xdiff/xemit.c b/xdiff/xemit.c
index 0ddb094..f49eaaf 100644
--- a/xdiff/xemit.c
+++ b/xdiff/xemit.c
@@ -29,6 +29,9 @@ struct func_line {
 };
 
 
+static long get_func_line(xdfile_t *xdf, xdemitconf_t const *xecfg,
+			  struct func_line *func_line, long start, long limit);
+
 static long xdl_get_rec(xdfile_t *xdf, long ri, char const **rec);
 static int xdl_emit_record(xdfile_t *xdf, long ri, char const *pre, xdemitcb_t *ecb);
 
@@ -62,7 +65,7 @@ static int xdl_emit_record(xdfile_t *xdf, long ri, char const *pre, xdemitcb_t *
  * inside the differential hunk according to the specified configuration.
  * Also advance xscr if the first changes must be discarded.
  */
-xdchange_t *xdl_get_hunk(xdchange_t **xscr, xdemitconf_t const *xecfg)
+xdchange_t *xdl_get_hunk(xdfenv_t *xe, xdchange_t **xscr, xdemitconf_t const *xecfg)
 {
 	xdchange_t *xch, *xchp, *lxch;
 	long max_common = 2 * xecfg->ctxlen + xecfg->interhunkctxlen;
@@ -83,6 +86,59 @@ xdchange_t *xdl_get_hunk(xdchange_t **xscr, xdemitconf_t const *xecfg)
 
 	lxch = *xscr;
 
+	if (xecfg->flags & XDL_EMIT_MOREHUNKHEADS)
+		for (xch = *xscr; xch; xch=xch->next) {
+			/*
+			 * If a current change contains a func_line, end this
+			 * hunk immediately before and create a new hunk
+			 * starting from that line.
+			 */
+			long fl_in_xch1 = get_func_line(&xe->xdf1, xecfg, NULL,
+						xch->i1, xch->i1+xch->chg1);
+			long fl_in_xch2 = get_func_line(&xe->xdf2, xecfg, NULL,
+						xch->i2, xch->i2+xch->chg2);
+			if (fl_in_xch1 >= xch->i1 && fl_in_xch2 >= xch->i2) {
+				xdchange_t *new_next =
+					(xdchange_t *)xdl_malloc(sizeof(xdchange_t));
+				new_next->i1 = xch->i1+xch->chg1;
+				new_next->chg1 = 0;
+				new_next->i2 = xch->i2;
+				new_next->chg2 = xch->chg2;
+				new_next->ignore = xch->ignore;
+				new_next->next = xch->next;
+				xch->next = new_next;
+				xch->chg2 = 0;
+				return xch;
+			}
+			if (fl_in_xch1 > xch->i1) {
+				xdchange_t *new_next =
+					(xdchange_t *)xdl_malloc(sizeof(xdchange_t));
+				new_next->i1 = fl_in_xch1;
+				new_next->chg1 = (xch->i1+xch->chg1)-fl_in_xch1;
+				new_next->i2 = xch->i2;
+				new_next->chg2 = xch->chg2;
+				new_next->ignore = xch->ignore;
+				new_next->next = xch->next;
+				xch->next = new_next;
+				xch->chg1 = fl_in_xch1 - xch->i1;
+				xch->chg2 = 0;
+				return xch;
+			}
+			if (fl_in_xch2 > xch->i2) {
+				xdchange_t *new_next =
+					(xdchange_t *)xdl_malloc(sizeof(xdchange_t));
+				new_next->i2 = fl_in_xch2;
+				new_next->chg2 = (xch->i2+xch->chg2)-fl_in_xch2;
+				new_next->i1 = xch->i1+xch->chg1;
+				new_next->chg1 = 0;
+				new_next->ignore = xch->ignore;
+				new_next->next = xch->next;
+				xch->next = new_next;
+				xch->chg2 = fl_in_xch2 - xch->i2;
+				return xch;
+			}
+		}
+
 	for (xchp = *xscr, xch = xchp->next; xch; xchp = xch, xch = xch->next) {
 		long distance = xch->i1 - (xchp->i1 + xchp->chg1);
 		if (distance > max_common)
@@ -173,7 +229,7 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,
 		return xdl_emit_common(xe, xscr, ecb, xecfg);
 
 	for (xch = xscr; xch; xch = xche->next) {
-		xche = xdl_get_hunk(&xch, xecfg);
+		xche = xdl_get_hunk(xe, &xch, xecfg);
 		if (!xch)
 			break;
 
diff --git a/xdiff/xemit.h b/xdiff/xemit.h
index d297107..4d584f5 100644
--- a/xdiff/xemit.h
+++ b/xdiff/xemit.h
@@ -27,7 +27,9 @@
 typedef int (*emit_func_t)(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,
 			   xdemitconf_t const *xecfg);
 
-xdchange_t *xdl_get_hunk(xdchange_t **xscr, xdemitconf_t const *xecfg);
+xdchange_t *xdl_get_hunk(xdfenv_t *xe, xdchange_t **xscr,
+			 xdemitconf_t const *xecfg);
+
 int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,
 		  xdemitconf_t const *xecfg);
 
-- 
1.7.12.4 (Apple Git-37)
David A. Dalrymple (and Bhushan G. Lodha)· Mar 27, 2014, 18:50 UTC · re: David A. Dalrymple (and Bhushan G. Lodha) · lore

[PATCH 09/10] t4213: test --function-name option

From: "Bhushan G. Lodha & David A. Dalrymple" <dad-bgl@mit.edu>

This test builds a sample C file, adding and removing functions, and checks that the right commits are filtered by --function-name matching.

Signed-off-by: David Dalrymple (on zayin) <davidad@alum.mit.edu>
---
 t/t4213-log-function-name.sh | 73 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 73 insertions(+)
 create mode 100755 t/t4213-log-function-name.sh
Show changes to t/t4213-log-function-name.sh +73 −0
diff --git a/t/t4213-log-function-name.sh b/t/t4213-log-function-name.sh
new file mode 100755
index 0000000..1243ce5
--- /dev/null
+++ b/t/t4213-log-function-name.sh
@@ -0,0 +1,73 @@
+#!/bin/sh
+
+test_description='log --function-name'
+. ./test-lib.sh
+
+test_expect_success setup '
+	echo "* diff=cpp" > .gitattributes
+
+	>file &&
+	git add file &&
+	test_tick &&
+	git commit -m initial &&
+
+	printf "int main(){\n\treturn 0;\n}\n" >> file &&
+	test_tick &&
+	git commit -am second
+
+	printf "void newfunc(){\n\treturn;\n}\n" >> file &&
+	test_tick &&
+	git commit -am third
+
+	printf "void newfunc2(){\n\treturn;\n}\n" | cat - file > temp &&
+	mv temp file &&
+	test_tick &&
+	git commit -am fourth
+
+	printf "void newfunc3(){\n\treturn;\n}\n" | cat - file > temp &&
+	mv temp file &&
+	test_tick &&
+	git commit -am fifth
+
+	sed -i -e "s/void newfunc2/void newfunc4/" file &&
+	test_tick &&
+	git commit -am sixth
+'
+
+test_expect_success 'log --function-name=main' '
+	git log --function-name=main >actual &&
+	git log --grep=second >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'log --function-name "newfunc\W"' '
+	git log --function-name "newfunc\W" >actual &&
+	git log --grep=third >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'log --function-name "newfunc2"' '
+	git log --function-name newfunc2 >actual &&
+	git log -E --grep "sixth|fourth" >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'log --function-name "newfunc3"' '
+	git log --function-name newfunc3 >actual &&
+	git log --grep=fifth >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'log --function-name "newfunc4"' '
+	git log --function-name newfunc4 >actual &&
+	git log --grep=sixth >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'log --function-name "newfunc"' '
+	git log --function-name newfunc >actual &&
+	git log -E --grep "third|fourth|fifth|sixth" >expect &&
+	test_cmp expect actual
+'
+
+test_done
-- 
1.7.12.4 (Apple Git-37)
Johannes Sixt· Mar 28, 2014, 07:25 UTC · re: David A. Dalrymple (and Bhushan G. Lodha) · lore

Re: [PATCH 09/10] t4213: test --function-name option

Am 3/27/2014 19:50, schrieb David A. Dalrymple (and Bhushan G. Lodha):
> From: "Bhushan G. Lodha & David A. Dalrymple" <dad-bgl@mit.edu>
> 
> This test builds a sample C file, adding and removing functions, and
> checks that the right commits are filtered by --function-name matching.

This is probably the most important patch in your series as it documents the expected behavior. Unfortunately, I find its clarity very lacking. :(

This new feature uses the userdiff driver, IIUC. Does it do so in all respects? In particular, does it also evaluate the negative patterns? For example, when there is a label in the code, is it not mistaken as the beginning of a function? A test for this case would be very instructive.

Furthermore, consider a patch for a change at the very beginning of a function. Then the function name would appear in the pre-context of the hunk, but the hunk header would show the function before the one with the change. Would such a change confuse your implementation? I guess not. Again, a test case would remove any doubts.

Is it possible to search for a change that is before any functions? It would be useful to enumerate commits that change #include lines.

Show 29 quoted lines
> 
> Signed-off-by: David Dalrymple (on zayin) <davidad@alum.mit.edu>
> ---
>  t/t4213-log-function-name.sh | 73 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 73 insertions(+)
>  create mode 100755 t/t4213-log-function-name.sh
> 
> diff --git a/t/t4213-log-function-name.sh b/t/t4213-log-function-name.sh
> new file mode 100755
> index 0000000..1243ce5
> --- /dev/null
> +++ b/t/t4213-log-function-name.sh
> @@ -0,0 +1,73 @@
> +#!/bin/sh
> +
> +test_description='log --function-name'
> +. ./test-lib.sh
> +
> +test_expect_success setup '
> +	echo "* diff=cpp" > .gitattributes
> +
> +	>file &&
> +	git add file &&
> +	test_tick &&
> +	git commit -m initial &&
> +
> +	printf "int main(){\n\treturn 0;\n}\n" >> file &&
> +	test_tick &&
> +	git commit -am second
Broken && chain here and later as well. Please be careful.
> +
> +	printf "void newfunc(){\n\treturn;\n}\n" >> file &&
> +	test_tick &&
> +	git commit -am third
	git commit -am "append a function" &&
Show 5 quoted lines
> +
> +	printf "void newfunc2(){\n\treturn;\n}\n" | cat - file > temp &&
> +	mv temp file &&
> +	test_tick &&
> +	git commit -am fourth
	git commit -am "prepend a function" &&
etc. You get the picture.
Show 12 quoted lines
> +
> +	printf "void newfunc3(){\n\treturn;\n}\n" | cat - file > temp &&
> +	mv temp file &&
> +	test_tick &&
> +	git commit -am fifth
> +
> +	sed -i -e "s/void newfunc2/void newfunc4/" file &&
> +	test_tick &&
> +	git commit -am sixth
> +'
> +
> +test_expect_success 'log --function-name=main' '
test_expect_success 'log --function-name finds a function with a change' '
Show 6 quoted lines
> +	git log --function-name=main >actual &&
> +	git log --grep=second >expect &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'log --function-name "newfunc\W"' '
test_expect_success 'log --function-name with extended regexp' '
etc. You get the picture.
Show 31 quoted lines
> +	git log --function-name "newfunc\W" >actual &&
> +	git log --grep=third >expect &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'log --function-name "newfunc2"' '
> +	git log --function-name newfunc2 >actual &&
> +	git log -E --grep "sixth|fourth" >expect &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'log --function-name "newfunc3"' '
> +	git log --function-name newfunc3 >actual &&
> +	git log --grep=fifth >expect &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'log --function-name "newfunc4"' '
> +	git log --function-name newfunc4 >actual &&
> +	git log --grep=sixth >expect &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'log --function-name "newfunc"' '
> +	git log --function-name newfunc >actual &&
> +	git log -E --grep "third|fourth|fifth|sixth" >expect &&
> +	test_cmp expect actual
> +'
> +
> +test_done
> 
Eric Sunshine· Mar 28, 2014, 08:21 UTC · re: Johannes Sixt · lore

Re: [PATCH 09/10] t4213: test --function-name option

On Fri, Mar 28, 2014 at 3:25 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
Show 43 quoted lines
> Am 3/27/2014 19:50, schrieb David A. Dalrymple (and Bhushan G. Lodha):
>> From: "Bhushan G. Lodha & David A. Dalrymple" <dad-bgl@mit.edu>
>>
>> This test builds a sample C file, adding and removing functions, and
>> checks that the right commits are filtered by --function-name matching.
>
> This is probably the most important patch in your series as it documents
> the expected behavior. Unfortunately, I find its clarity very lacking. :(
>
> This new feature uses the userdiff driver, IIUC. Does it do so in all
> respects? In particular, does it also evaluate the negative patterns? For
> example, when there is a label in the code, is it not mistaken as the
> beginning of a function? A test for this case would be very instructive.
>
> Furthermore, consider a patch for a change at the very beginning of a
> function. Then the function name would appear in the pre-context of the
> hunk, but the hunk header would show the function before the one with the
> change. Would such a change confuse your implementation? I guess not.
> Again, a test case would remove any doubts.
>
> Is it possible to search for a change that is before any functions? It
> would be useful to enumerate commits that change #include lines.
>
>>
>> Signed-off-by: David Dalrymple (on zayin) <davidad@alum.mit.edu>
>> ---
>>  t/t4213-log-function-name.sh | 73 ++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 73 insertions(+)
>>  create mode 100755 t/t4213-log-function-name.sh
>>
>> diff --git a/t/t4213-log-function-name.sh b/t/t4213-log-function-name.sh
>> new file mode 100755
>> index 0000000..1243ce5
>> --- /dev/null
>> +++ b/t/t4213-log-function-name.sh
>> @@ -0,0 +1,73 @@
>> +#!/bin/sh
>> +
>> +test_description='log --function-name'
>> +. ./test-lib.sh
>> +
>> +test_expect_success setup '
>> +     echo "* diff=cpp" > .gitattributes
Broken &&-chain here, as well.
Show 90 quoted lines
>> +
>> +     >file &&
>> +     git add file &&
>> +     test_tick &&
>> +     git commit -m initial &&
>> +
>> +     printf "int main(){\n\treturn 0;\n}\n" >> file &&
>> +     test_tick &&
>> +     git commit -am second
>
> Broken && chain here and later as well. Please be careful.
>
>> +
>> +     printf "void newfunc(){\n\treturn;\n}\n" >> file &&
>> +     test_tick &&
>> +     git commit -am third
>
>         git commit -am "append a function" &&
>
>> +
>> +     printf "void newfunc2(){\n\treturn;\n}\n" | cat - file > temp &&
>> +     mv temp file &&
>> +     test_tick &&
>> +     git commit -am fourth
>
>         git commit -am "prepend a function" &&
>
> etc. You get the picture.
>
>> +
>> +     printf "void newfunc3(){\n\treturn;\n}\n" | cat - file > temp &&
>> +     mv temp file &&
>> +     test_tick &&
>> +     git commit -am fifth
>> +
>> +     sed -i -e "s/void newfunc2/void newfunc4/" file &&
>> +     test_tick &&
>> +     git commit -am sixth
>> +'
>> +
>> +test_expect_success 'log --function-name=main' '
>
> test_expect_success 'log --function-name finds a function with a change' '
>
>> +     git log --function-name=main >actual &&
>> +     git log --grep=second >expect &&
>> +     test_cmp expect actual
>> +'
>> +
>> +test_expect_success 'log --function-name "newfunc\W"' '
>
> test_expect_success 'log --function-name with extended regexp' '
>
> etc. You get the picture.
>
>> +     git log --function-name "newfunc\W" >actual &&
>> +     git log --grep=third >expect &&
>> +     test_cmp expect actual
>> +'
>> +
>> +test_expect_success 'log --function-name "newfunc2"' '
>> +     git log --function-name newfunc2 >actual &&
>> +     git log -E --grep "sixth|fourth" >expect &&
>> +     test_cmp expect actual
>> +'
>> +
>> +test_expect_success 'log --function-name "newfunc3"' '
>> +     git log --function-name newfunc3 >actual &&
>> +     git log --grep=fifth >expect &&
>> +     test_cmp expect actual
>> +'
>> +
>> +test_expect_success 'log --function-name "newfunc4"' '
>> +     git log --function-name newfunc4 >actual &&
>> +     git log --grep=sixth >expect &&
>> +     test_cmp expect actual
>> +'
>> +
>> +test_expect_success 'log --function-name "newfunc"' '
>> +     git log --function-name newfunc >actual &&
>> +     git log -E --grep "third|fourth|fifth|sixth" >expect &&
>> +     test_cmp expect actual
>> +'
>> +
>> +test_done
>>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jakub Narębski· Apr 4, 2014, 11:21 UTC · re: David A. Dalrymple (and Bhushan G. Lodha) · lore

Re: [PATCH 09/10] t4213: test --function-name option

W dniu 2014-03-27 19:50, David A. Dalrymple (and Bhushan G. Lodha) pisze:
Show 11 quoted lines
> +test_expect_success setup '
> +	echo "* diff=cpp" > .gitattributes
> +
> +	>file &&
> +	git add file &&
> +	test_tick &&
> +	git commit -m initial &&
> +
> +	printf "int main(){\n\treturn 0;\n}\n" >> file &&
> +	test_tick &&
> +	git commit -am second

Wouldn't it be more readable to use "cat \-EOF" or separate files instead of printf-s with embedded newlines "\n" and tabs "\t"?

-- 
Jakub Narębski
David A. Dalrymple (and Bhushan G. Lodha)· Mar 27, 2014, 18:50 UTC · re: David A. Dalrymple (and Bhushan G. Lodha) · lore

[PATCH 10/10] Documentation: Document --function-name usage

From: "Bhushan Lodha & David A. Dalrymple" <dad-bgl@mit.edu>
Signed-off-by: David Dalrymple (on zayin) <davidad@alum.mit.edu>
---
 Documentation/diff-options.txt |  9 +++++++++
 Documentation/gitdiffcore.txt  | 17 ++++++++++++++---
 2 files changed, 23 insertions(+), 3 deletions(-)
Show changes to 2 files +23 −3

Documentation/diff-options.txt, Documentation/gitdiffcore.txt

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 9b37b2a..a778dff 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -427,6 +427,15 @@ information.
 --pickaxe-regex::
 	Treat the <string> given to `-S` as an extended POSIX regular
 	expression to match.
+
+--function-name<regex>::
+	Look for differences whose patch text contains hunk headers that match
+	<regex>. This can be useful to locate changes to a particular function
+	or other semantic element in a program, since hunk headers are intended
+	to indicate the "function context" (in the sense of
+	`--function-context`) in which the particular change occurs. The
+	function context is determined by the diff driver corresponding to the
+	file in question; see linkgit:gitattributes[7] for details.
 endif::git-format-patch[]
 
 -O<orderfile>::
diff --git a/Documentation/gitdiffcore.txt b/Documentation/gitdiffcore.txt
index c8b3e51..b8477ce 100644
--- a/Documentation/gitdiffcore.txt
+++ b/Documentation/gitdiffcore.txt
@@ -222,10 +222,11 @@ version prefixed with '+'.
 diffcore-pickaxe: For Detecting Addition/Deletion of Specified String
 ---------------------------------------------------------------------
 
-This transformation limits the set of filepairs to those that change
+This transformation limits the set of filepairs to those that involve
 specified strings between the preimage and the postimage in a certain
-way.  -S<block of text> and -G<regular expression> options are used to
-specify different ways these strings are sought.
+way.  -S<block of text>, -G<regular expression>, and
+--function-name<regular expression> options are used to specify
+different ways these strings are sought.
 
 "-S<block of text>" detects filepairs whose preimage and postimage
 have different number of occurrences of the specified block of text.
@@ -251,6 +252,16 @@ criterion in a changeset, the entire changeset is kept.  This behavior
 is designed to make reviewing changes in the context of the whole
 changeset easier.
 
+"--function-name<regular expression>" detects filepairs whose textual
+diff contains a hunk header that matches the given regular expression.
+The hunk header is generated via the diff driver specified in
+`.gitattributes`, and is intended to reflect the "function context"
+(in the sense of `--function-context`) in which the change occurs,
+with programming-language-dependent heuristics. As of now, the
+programming language is not auto-detected in any way. Also note that
+hunks whose headers do not match the regular expression are not
+currently filtered out; this is only a filepair filter.
+
 diffcore-order: For Sorting the Output Based on Filenames
 ---------------------------------------------------------
 
-- 
1.7.12.4 (Apple Git-37)
Eric Sunshine· Mar 28, 2014, 00:30 UTC · re: David A. Dalrymple (and Bhushan G. Lodha) · lore

Re: [PATCH 10/10] Documentation: Document --function-name usage

On Thu, Mar 27, 2014 at 2:50 PM, David A. Dalrymple (and Bhushan G. Lodha) <dad-bgl@mit.edu> wrote:

Show 18 quoted lines
> From: "Bhushan Lodha & David A. Dalrymple" <dad-bgl@mit.edu>
>
> Signed-off-by: David Dalrymple (on zayin) <davidad@alum.mit.edu>
> ---
>  Documentation/diff-options.txt |  9 +++++++++
>  Documentation/gitdiffcore.txt  | 17 ++++++++++++++---
>  2 files changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
> index 9b37b2a..a778dff 100644
> --- a/Documentation/diff-options.txt
> +++ b/Documentation/diff-options.txt
> @@ -427,6 +427,15 @@ information.
>  --pickaxe-regex::
>         Treat the <string> given to `-S` as an extended POSIX regular
>         expression to match.
> +
> +--function-name<regex>::

Missing '=' before <regex>. Ditto for the several additional instances in this patch.

Show 53 quoted lines
> +       Look for differences whose patch text contains hunk headers that match
> +       <regex>. This can be useful to locate changes to a particular function
> +       or other semantic element in a program, since hunk headers are intended
> +       to indicate the "function context" (in the sense of
> +       `--function-context`) in which the particular change occurs. The
> +       function context is determined by the diff driver corresponding to the
> +       file in question; see linkgit:gitattributes[7] for details.
>  endif::git-format-patch[]
>
>  -O<orderfile>::
> diff --git a/Documentation/gitdiffcore.txt b/Documentation/gitdiffcore.txt
> index c8b3e51..b8477ce 100644
> --- a/Documentation/gitdiffcore.txt
> +++ b/Documentation/gitdiffcore.txt
> @@ -222,10 +222,11 @@ version prefixed with '+'.
>  diffcore-pickaxe: For Detecting Addition/Deletion of Specified String
>  ---------------------------------------------------------------------
>
> -This transformation limits the set of filepairs to those that change
> +This transformation limits the set of filepairs to those that involve
>  specified strings between the preimage and the postimage in a certain
> -way.  -S<block of text> and -G<regular expression> options are used to
> -specify different ways these strings are sought.
> +way.  -S<block of text>, -G<regular expression>, and
> +--function-name<regular expression> options are used to specify
> +different ways these strings are sought.
>
>  "-S<block of text>" detects filepairs whose preimage and postimage
>  have different number of occurrences of the specified block of text.
> @@ -251,6 +252,16 @@ criterion in a changeset, the entire changeset is kept.  This behavior
>  is designed to make reviewing changes in the context of the whole
>  changeset easier.
>
> +"--function-name<regular expression>" detects filepairs whose textual
> +diff contains a hunk header that matches the given regular expression.
> +The hunk header is generated via the diff driver specified in
> +`.gitattributes`, and is intended to reflect the "function context"
> +(in the sense of `--function-context`) in which the change occurs,
> +with programming-language-dependent heuristics. As of now, the
> +programming language is not auto-detected in any way. Also note that
> +hunks whose headers do not match the regular expression are not
> +currently filtered out; this is only a filepair filter.
> +
>  diffcore-order: For Sorting the Output Based on Filenames
>  ---------------------------------------------------------
>
> --
> 1.7.12.4 (Apple Git-37)
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jeff King· Mar 27, 2014, 19:03 UTC · re: David A. Dalrymple (and Bhushan G. Lodha) · lore

Re: [PATCH 00/10] [RFC] pickaxe for function names

On Thu, Mar 27, 2014 at 02:50:46PM -0400, David A. Dalrymple (and Bhushan G. Lodha) wrote:
> This series introduces a --function-name=<pattern> option for git-log, intended
> to search for commits which touch a function matching a certain pattern (a
> feature we've seen requested and are interested in using ourselves).
How does your feature compare with the line-level history viewer? E.g.:
  git log -L:myfunc:foo.c

I guess by being part of pickaxe, it can be used to generally select commits (whereas "-L" is about drilling down a particular set of lines, so something like --full-diff would not work).

But "-L" can do many things that pickaxe can't. It is not just about finding lines touched within a pattern, but uses the pattern to determine an initial set of lines, and then recursively "blames" those lines going back through history. So how you specify the pattern is more flexible (you can give any line range, for example), and it should be able to cross boundaries like function renames.

-Peff

← back to recent threads