{"thread":{"id":"24415","subject":"gitk: add \"grep diff\" selection criterion (Re: find commit adding/removing could use changing option)","startedAt":"2010-07-16T17:28:06Z","lastAt":"2010-07-19T22:44:14Z","messageCount":2,"participants":["Jonathan Nieder","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"145699","messageId":"20100716172806.GA15491@burratino","threadId":"24415","inReplyTo":"D163BB49BCC4479AB3E4BA4F87826184@csmith","subject":"gitk: add \"grep diff\" selection criterion (Re: find commit adding/removing could use changing option)","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-16T17:28:06Z","receivedAt":"2010-07-16T17:28:06Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi Chris,\n\nChris Smith wrote:\n\n> One thing I find myself wishing for is the ability to find a line that has\n> been modified by using gitk. It seems to me that if I have the following in a\n> diff\n> \n> - def foo(this)\n> + def foo(this, that=False)\n> \n> that I can't find the line by searching for foo.\n\nAh, so you want a cousin to ‘log -S’ that /does/ search through\ndiffs.  See [1].\n\nAs that thread explains, it is doable from the commandline already,\nthough not in a nicely packaged form.  But what about from gitk?\n\nA possible hack would be to teach gitk to add arbitrary criteria to\nthe end of the ‘git diff-tree’ command line[2].  In this case, the\ncriterion would be ‘| search-diff $term’, where search-diff is a\nscript something like the following:\n\n #!/bin/sh\n while read -r commit\n do\n\tif\n\t\tgit diff-tree -p -c \"$commit\" | grep -q \"$@\"\n\tthen\n\t\tprintf '%s\\n' \"$commit\"\n\tfi\n done\n\nIf that proves useful, afterwards one could teach ‘diff-tree’ to do\nthe check itself, which would allow commands like\n\n git log --search-diff=foo\n\ntoo.\n\nThoughts?\nJonathan\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/122478\n[2] You can find the code one would need to touch for this by\nsearching for gdtargs and gdttype in the gitk script.\n"},{"id":"145827","messageId":"7viq4bnlc1.fsf@alter.siamese.dyndns.org","threadId":"24415","inReplyTo":"20100716172806.GA15491@burratino","subject":"Re: gitk: add \"grep diff\" selection criterion (Re: find commit adding/removing could use changing option)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-07-19T22:44:14Z","receivedAt":"2010-07-19T22:44:14Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> Ah, so you want a cousin to ‘log -S’ that /does/ search through\n> diffs.  See [1].\n> ...\n> If that proves useful, afterwards one could teach ‘diff-tree’ to do\n> the check itself...\n> [1] http://thread.gmane.org/gmane.comp.version-control.git/122478\n\nA better quote might have been:\n\n    http://thread.gmane.org/gmane.comp.version-control.git/112077/focus=112114\n\nAnd the attached should be a good starting point if anybody is interested.\nI didn't bother to check for leaks and premature free()s, add tests nor\ndocument it, though.\n\nNot yet anyway.\n\n-- >8 --\nSubject: [PATCH] git log: add -G<regexp> that greps in the patch text\n\nTeach \"-G<regexp>\" that is similar to \"-S<regexp> --pickaxe-regexp\" to the\n\"git diff\" family of commands.  This limits the diff queue to filepairs\nwhose patch text actually has an added or deleted line that matches the\ngiven regexp.  Unlike \"-S<regexp>\" changing other parts of the line that\nhas a substring that matches the given regexp IS counted as a change, as\nsuch a change would appear as one deletion followed by one addition in a\npatch text.\n\nUnlike -S (pickaxe) that is intended to be used to quickly detect a commit\nthat changes the number of occurrences of hits between the preimage and\nthe postimage to serve as a part of larger toolchain, this new feature is\nmeant to be used as the top-level Porcelain feature.\n\nThis implementation unfortunately has to run \"diff\" twice if you are\nrunning \"log\" family of commands to produce patches in the final output\n(e.g. \"git log -p\" or \"git format-patch\").  I think we _could_ cache the\nresult in-core if we wanted to, but that would require larger surgery to\nthe diffcore machinery (i.e. adding an extra pointer in the filepair\nstructure to keep a pointer to a strbuf around, stuff the textual diff to\nthe strbuf inside diffgrep_consume(), and make use of it in later stages\nwhen it is available) and it may not be worth it.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Makefile            |    1 +\n diff.c              |    6 ++-\n diff.h              |    1 +\n diffcore-log-grep.c |  145 +++++++++++++++++++++++++++++++++++++++++++++++++++\n diffcore.h          |    1 +\n 5 files changed, 153 insertions(+), 1 deletions(-)\n create mode 100644 diffcore-log-grep.c\n\ndiff --git a/Makefile b/Makefile\nindex bc3c570..4213f40 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -562,6 +562,7 @@ LIB_OBJS += date.o\n LIB_OBJS += decorate.o\n LIB_OBJS += diffcore-break.o\n LIB_OBJS += diffcore-delta.o\n+LIB_OBJS += diffcore-log-grep.o\n LIB_OBJS += diffcore-order.o\n LIB_OBJS += diffcore-pickaxe.o\n LIB_OBJS += diffcore-rename.o\ndiff --git a/diff.c b/diff.c\nindex 17873f3..f4bf1fd 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2904,7 +2904,7 @@ int diff_setup_done(struct diff_options *options)\n \t/*\n \t * Also pickaxe would not work very well if you do not say recursive\n \t */\n-\tif (options->pickaxe)\n+\tif (options->pickaxe || options->log_grep)\n \t\tDIFF_OPT_SET(options, RECURSIVE);\n \t/*\n \t * When patches are generated, submodules diffed against the work tree\n@@ -3184,6 +3184,8 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \t\toptions->rename_limit = strtoul(arg+2, NULL, 10);\n \telse if (!prefixcmp(arg, \"-S\"))\n \t\toptions->pickaxe = arg + 2;\n+\telse if (!prefixcmp(arg, \"-G\"))\n+\t\toptions->log_grep = arg + 2;\n \telse if (!strcmp(arg, \"--pickaxe-all\"))\n \t\toptions->pickaxe_opts = DIFF_PICKAXE_ALL;\n \telse if (!strcmp(arg, \"--pickaxe-regex\"))\n@@ -4075,6 +4077,8 @@ void diffcore_std(struct diff_options *options)\n \t\tdiffcore_merge_broken();\n \tif (options->pickaxe)\n \t\tdiffcore_pickaxe(options->pickaxe, options->pickaxe_opts);\n+\tif (options->log_grep)\n+\t\tdiffcore_log_grep(options->log_grep, options->pickaxe_opts);\n \tif (options->orderfile)\n \t\tdiffcore_order(options->orderfile);\n \tdiff_resolve_rename_copy();\ndiff --git a/diff.h b/diff.h\nindex 063d10a..ebd128b 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -96,6 +96,7 @@ struct diff_options {\n \tconst char *filter;\n \tconst char *orderfile;\n \tconst char *pickaxe;\n+\tconst char *log_grep;\n \tconst char *single_follow;\n \tconst char *a_prefix, *b_prefix;\n \tunsigned flags;\ndiff --git a/diffcore-log-grep.c b/diffcore-log-grep.c\nnew file mode 100644\nindex 0000000..501ae46\n--- /dev/null\n+++ b/diffcore-log-grep.c\n@@ -0,0 +1,145 @@\n+/*\n+ * Copyright (C) 2010 Junio C Hamano\n+ */\n+#include \"cache.h\"\n+#include \"diff.h\"\n+#include \"diffcore.h\"\n+#include \"xdiff-interface.h\"\n+\n+struct diffgrep_cb {\n+\tregex_t *regexp;\n+\tint hit;\n+};\n+\n+static void diffgrep_consume(void *priv, char *line, unsigned long len)\n+{\n+\tstruct diffgrep_cb *data = priv;\n+\tregmatch_t regmatch;\n+\tint hold;\n+\n+\tif (line[0] != '+' && line[0] != '-')\n+\t\treturn;\n+\tif (data->hit)\n+\t\t/*\n+\t\t * NEEDSWORK: we should have a way to terminate the\n+\t\t * caller early.\n+\t\t */\n+\t\treturn;\n+\t/* Yuck -- line ought to be \"const char *\"! */\n+\thold = line[len];\n+\tline[len] = '\\0';\n+\tdata->hit = !regexec(data->regexp, line + 1, 1, &regmatch, 0);\n+\tline[len] = hold;\n+}\n+\n+static void fill_one(struct diff_filespec *one,\n+\t\t     mmfile_t *mf, struct userdiff_driver **textconv)\n+{\n+\tif (DIFF_FILE_VALID(one)) {\n+\t\t*textconv = get_textconv(one);\n+\t\tmf->size = fill_textconv(*textconv, one, &mf->ptr);\n+\t} else {\n+\t\tmemset(mf, 0, sizeof(*mf));\n+\t}\n+}\n+\n+static int diff_grep(struct diff_filepair *p, regex_t *regexp)\n+{\n+\tregmatch_t regmatch;\n+\tstruct userdiff_driver *textconv_one = NULL;\n+\tstruct userdiff_driver *textconv_two = NULL;\n+\tmmfile_t mf1, mf2;\n+\tint hit;\n+\n+\tif (diff_unmodified_pair(p))\n+\t\treturn 0;\n+\n+\tfill_one(p->one, &mf1, &textconv_one);\n+\tfill_one(p->two, &mf2, &textconv_two);\n+\n+\tif (!mf1.ptr) {\n+\t\tif (!mf2.ptr)\n+\t\t\treturn 0; /* ignore unmerged */\n+\t\t/* created \"two\" -- does it have what we are looking for? */\n+\t\thit = !regexec(regexp, p->two->data, 1, &regmatch, 0);\n+\t} else if (!mf2.ptr) {\n+\t\t/* removed \"one\" -- did it have what we are looking for? */\n+\t\thit = !regexec(regexp, p->one->data, 1, &regmatch, 0);\n+\t} else {\n+\t\t/*\n+\t\t * We have both sides; need to run textual diff and see if a\n+\t\t * line that match the pattern appears in +/- line.\n+\t\t */\n+\t\tstruct diffgrep_cb ecbdata;\n+\t\txpparam_t xpp;\n+\t\txdemitconf_t xecfg;\n+\n+\t\tmemset(&xpp, 0, sizeof(xpp));\n+\t\tmemset(&xecfg, 0, sizeof(xecfg));\n+\t\tecbdata.regexp = regexp;\n+\t\tecbdata.hit = 0;\n+\t\txdi_diff_outf(&mf1, &mf2, diffgrep_consume, &ecbdata,\n+\t\t\t      &xpp, &xecfg);\n+\t\thit = ecbdata.hit;\n+\t}\n+\tif (textconv_one)\n+\t\tfree(mf1.ptr);\n+\tif (textconv_two)\n+\t\tfree(mf2.ptr);\n+\treturn hit;\n+}\n+\n+void diffcore_log_grep(const char *needle, int opts)\n+{\n+\tstruct diff_queue_struct *q = &diff_queued_diff;\n+\tint i, has_changes, err;\n+\tregex_t regex, *regexp = NULL;\n+\tstruct diff_queue_struct outq;\n+\toutq.queue = NULL;\n+\toutq.nr = outq.alloc = 0;\n+\n+\terr = regcomp(&regex, needle, REG_EXTENDED | REG_NEWLINE);\n+\tif (err) {\n+\t\tchar errbuf[1024];\n+\t\tregerror(err, &regex, errbuf, 1024);\n+\t\tregfree(&regex);\n+\t\tdie(\"invalid log-grep regex: %s\", errbuf);\n+\t}\n+\tregexp = &regex;\n+\n+\tif (opts & DIFF_PICKAXE_ALL) {\n+\t\t/* Showing the whole changeset if needle exists */\n+\t\tfor (i = has_changes = 0; !has_changes && i < q->nr; i++) {\n+\t\t\tstruct diff_filepair *p = q->queue[i];\n+\t\t\tif (diff_grep(p, regexp))\n+\t\t\t\thas_changes++;\n+\t\t}\n+\t\tif (has_changes)\n+\t\t\treturn; /* not munge the queue */\n+\n+\t\t/* otherwise we will clear the whole queue\n+\t\t * by copying the empty outq at the end of this\n+\t\t * function, but first clear the current entries\n+\t\t * in the queue.\n+\t\t */\n+\t\tfor (i = 0; i < q->nr; i++)\n+\t\t\tdiff_free_filepair(q->queue[i]);\n+\t} else {\n+\t\t/* Showing only the filepairs that has the needle */\n+\t\tfor (i = 0; i < q->nr; i++) {\n+\t\t\tstruct diff_filepair *p = q->queue[i];\n+\t\t\tif (diff_grep(p, regexp))\n+\t\t\t\tdiff_q(&outq, p);\n+\t\t\telse\n+\t\t\t\tdiff_free_filepair(p);\n+\t\t}\n+\t}\n+\n+\tif (opts & DIFF_PICKAXE_REGEX) {\n+\t\tregfree(&regex);\n+\t}\n+\n+\tfree(q->queue);\n+\t*q = outq;\n+\treturn;\n+}\ndiff --git a/diffcore.h b/diffcore.h\nindex 491bea0..e3f8f05 100644\n--- a/diffcore.h\n+++ b/diffcore.h\n@@ -110,6 +110,7 @@ extern void diffcore_break(int);\n extern void diffcore_rename(struct diff_options *);\n extern void diffcore_merge_broken(void);\n extern void diffcore_pickaxe(const char *needle, int opts);\n+extern void diffcore_log_grep(const char *needle, int opts);\n extern void diffcore_order(const char *orderfile);\n \n #define DIFF_DEBUG 0\n-- \n1.7.2.rc3.270.gb7c60\n"}]}