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 gits...@pobox.com wrote: Jakub Narębski jna...@gmail.com writes: W dniu 2014-04-03 23:44, Junio C Hamano pisze: René Scharfe l@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(). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/10] log: --function-name pickaxe
W dniu 2014-04-03 23:44, Junio C Hamano pisze: René Scharfe l@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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/10] log: --function-name pickaxe
Jakub Narębski jna...@gmail.com writes: W dniu 2014-04-03 23:44, Junio C Hamano pisze: René Scharfe l@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(). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/10] log: --function-name pickaxe
Am 27.03.2014 19:50, schrieb David A. Dalrymple (and Bhushan G. Lodha): From: Bhushan G. Lodha David A. Dalrymple dad-...@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é -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/10] log: --function-name pickaxe
René Scharfe l@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. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 06/10] log: --function-name pickaxe
From: Bhushan G. Lodha David A. Dalrymple dad-...@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) davi...@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(-)
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)
+{
