{"thread":{"id":"5594","subject":"[PATCH 3/3] revision traversal: --author, --committer, and --grep.","startedAt":"2006-09-18T00:42:26Z","lastAt":"2006-09-18T17:07:51Z","messageCount":5,"participants":["Junio C Hamano","Jeff King","Linus Torvalds"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"27101","messageId":"7v4pv6yphp.fsf@assigned-by-dhcp.cox.net","threadId":"5594","inReplyTo":null,"subject":"[PATCH 3/3] revision traversal: --author, --committer, and --grep.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-09-18T00:42:26Z","receivedAt":"2006-09-18T00:42:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This adds three options to setup_revisions(), which lets you\nfilter resulting commits by the author name, the committer name\nand the log message with regexp.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n---\n Documentation/git-rev-list.txt |   11 ++++++\n revision.c                     |   76 +++++++++++++++++++++++++++++++++++++++-\n 2 files changed, 86 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt\nindex 28966ad..00a95e2 100644\n--- a/Documentation/git-rev-list.txt\n+++ b/Documentation/git-rev-list.txt\n@@ -20,6 +20,7 @@ SYNOPSIS\n \t     [ \\--stdin ]\n \t     [ \\--topo-order ]\n \t     [ \\--parents ]\n+\t     [ \\--(author|committer|grep)=<pattern> ]\n \t     [ [\\--objects | \\--objects-edge] [ \\--unpacked ] ]\n \t     [ \\--pretty | \\--header ]\n \t     [ \\--bisect ]\n@@ -154,6 +155,16 @@ limiting may be applied.\n \n \tLimit the commits output to specified time range.\n \n+--author='pattern', --committer='pattern'::\n+\n+\tLimit the commits output to ones with author/committer\n+\theader lines that match the specified pattern.\n+\n+--grep='pattern'::\n+\n+\tLimit the commits output to ones with log message that\n+\tmatches the specified pattern.\n+\n --remove-empty::\n \n \tStop when a given path disappears from the tree.\ndiff --git a/revision.c b/revision.c\nindex 8fda20d..19f7272 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -673,6 +673,40 @@ int handle_revision_arg(const char *arg,\n \treturn 0;\n }\n \n+static void add_header_grep(struct rev_info *revs, const char *field, const char *pattern)\n+{\n+\tchar *pat;\n+\tint patlen, fldlen;\n+\n+\tif (!revs->header_filter) {\n+\t\tstruct grep_opt *opt = xcalloc(1, sizeof(*opt));\n+\t\topt->status_only = 1;\n+\t\topt->pattern_tail = &(opt->pattern_list);\n+\t\topt->regflags = REG_NEWLINE;\n+\t\trevs->header_filter = opt;\n+\t}\n+\n+\tfldlen = strlen(field);\n+\tpatlen = strlen(pattern);\n+\tpat = xmalloc(patlen + fldlen + 3);\n+\tsprintf(pat, \"^%s %s\", field, pattern);\n+\tappend_grep_pattern(revs->header_filter, pat,\n+\t\t\t    \"command line\", 0, GREP_PATTERN);\n+}\n+\n+static void add_message_grep(struct rev_info *revs, const char *pattern)\n+{\n+\tif (!revs->message_filter) {\n+\t\tstruct grep_opt *opt = xcalloc(1, sizeof(*opt));\n+\t\topt->status_only = 1;\n+\t\topt->pattern_tail = &(opt->pattern_list);\n+\t\topt->regflags = REG_NEWLINE;\n+\t\trevs->message_filter = opt;\n+\t}\n+\tappend_grep_pattern(revs->message_filter, pattern,\n+\t\t\t    \"command line\", 0, GREP_PATTERN);\n+}\n+\n /*\n  * Parse revision information, filling in the \"rev_info\" structure,\n  * and removing the used arguments from the argument list.\n@@ -896,6 +930,18 @@ int setup_revisions(int argc, const char\n \t\t\t\trevs->relative_date = 1;\n \t\t\t\tcontinue;\n \t\t\t}\n+\t\t\tif (!strncmp(arg, \"--author=\", 9)) {\n+\t\t\t\tadd_header_grep(revs, \"author\", arg+9);\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t\tif (!strncmp(arg, \"--committer=\", 12)) {\n+\t\t\t\tadd_header_grep(revs, \"committer\", arg+12);\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t\tif (!strncmp(arg, \"--grep=\", 7)) {\n+\t\t\t\tadd_message_grep(revs, arg+7);\n+\t\t\t\tcontinue;\n+\t\t\t}\n \t\t\topts = diff_opt_parse(&revs->diffopt, argv+i, argc-i);\n \t\t\tif (opts > 0) {\n \t\t\t\trevs->diff = 1;\n@@ -956,6 +1002,11 @@ int setup_revisions(int argc, const char\n \tif (diff_setup_done(&revs->diffopt) < 0)\n \t\tdie(\"diff_setup_done failed\");\n \n+\tif (revs->header_filter)\n+\t\tcompile_grep_patterns(revs->header_filter);\n+\tif (revs->message_filter)\n+\t\tcompile_grep_patterns(revs->message_filter);\n+\n \treturn left;\n }\n \n@@ -1030,10 +1081,33 @@ static void mark_boundary_to_show(struct\n \n static int commit_match(struct commit *commit, struct rev_info *opt)\n {\n+\tchar *header, *message;\n+\tunsigned long header_len, message_len;\n+\n \tif (!opt->header_filter && !opt->message_filter)\n \t\treturn 1;\n \n-\t/* match it here */\n+\theader = commit->buffer;\n+\tmessage = strstr(header, \"\\n\\n\");\n+\tif (message) {\n+\t\tmessage += 2;\n+\t\theader_len = message - header - 1;\n+\t\tmessage_len = strlen(message);\n+\t}\n+\telse {\n+\t\theader_len = strlen(header);\n+\t\tmessage = header;\n+\t\tmessage_len = 0;\n+\t}\n+\n+\tif (opt->header_filter &&\n+\t    !grep_buffer(opt->header_filter, NULL, header, header_len))\n+\t\treturn 0;\n+\n+\tif (opt->message_filter &&\n+\t    !grep_buffer(opt->message_filter, NULL, message, message_len))\n+\t\treturn 0;\n+\n \treturn 1;\n }\n \n-- \n1.4.2.1.g414e5\n"},{"id":"27115","messageId":"20060918060552.GA2833@coredump.intra.peff.net","threadId":"5594","inReplyTo":"7v4pv6yphp.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 3/3] revision traversal: --author, --committer, and --grep.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2006-09-18T06:05:52Z","receivedAt":"2006-09-18T06:05:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Sep 17, 2006 at 05:42:26PM -0700, Junio C Hamano wrote:\n\n> This adds three options to setup_revisions(), which lets you\n> filter resulting commits by the author name, the committer name\n> and the log message with regexp.\n\nFirst of all, thanks for implementing this; I tried to use it the other\nday (remembering the discussion and patches a few weeks ago) and was\ndisappointed to find it absent.\n\nThat being said, I find the matching style completely unintuitive. :)\n\nTo find --author=foo, your strategy is to stringify the header and grep\nfor \"^author foo\". As a user, my expectation was that you would\nstringify the author field and grep for \"foo\".\n\nThe important difference is that your approach means that the user's\nregex is implicitly anchored at the beginning of the field. Thus,\nsearching by email address does not work with --author=junkio, but\nrather requires --author='.*junkio'.\n\nPossible fixes:\n  1. Match against \"^<field>.*<regex>\" (I haven't looked closely at the\n     builtin grep implementation, but presumably '.' as usual does not\n     include newline).\n  2. Find <field>, and then feed grep_buffer only the contents of that\n     line.\nThe second is what I feel that users will expect (at least what I\nexpected!), but is probably slightly less efficient (two greps instead\nof one, but I doubt the difference would be significant). However, I\ndon't think there is a way with the first approach to explicitly request\na beginning-of-string anchor (i.e., \"^Junio\" in the second approach).\n\nA few other thoughts:\n  1. Case sensitivity? For convenience sake, it seems reasonable to\n     match these fields without case sensitivity (what was the\n     capitalization of A Large Angry SCM again? von Brand or Von Brand?\n     etc). Should it be optional, and if so, how to specify it (a global\n     command line option is probably not desired, as you might want\n     case-sensitive --grep but case-insensitive --author). So we either\n     need a \"-i means the rest of the arguments are insensitive, +i\n     means they are sensitive\" option, or some syntax to specify it in\n     the regex (perl uses (?i)).\n  2. Is there any use to exposing the \"header_grep\" functionality with\n     --grep-header? Is there anything worth grepping for besides\n     author/committer? The general consensus on non-core headers in\n     commit objects seemed to be \"don't do it\".\n  3. An alias (--who=foo?) for --author=foo --committer=foo. I believe\n     this doesn't require boolean magic, since we default to OR.\n\nI'm happy to work on implementing any of the above if there's interest.\n\n-Peff\n"},{"id":"27121","messageId":"7vslipsm4x.fsf@assigned-by-dhcp.cox.net","threadId":"5594","inReplyTo":"20060918060552.GA2833@coredump.intra.peff.net","subject":"Re: [PATCH 3/3] revision traversal: --author, --committer, and --grep.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-09-18T06:51:26Z","receivedAt":"2006-09-18T06:51:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n>The important difference is that your approach means that the user's\n>regex is implicitly anchored at the beginning of the field. Thus,\n>searching by email address does not work with --author=junkio, but\n>rather requires --author='.*junkio'.\n\nI wanted to default it to left anchored, so this was somewhat\ndeliberate, but this is probably subject to taste.\n\nI actually wanted to do only --grep-header and --grep-message\ninternally with --author and --committer as simple shorthands,\nand that is why --author=<pattern> is just an internal synonym\nfor \"author <pattern>\".  I even considered doing without\ndistinction between header and body (meaning, a line in the\ncommit log message that happens to begin with \"author \" would\nmatch --author string), which is technically incorrect but would\nbe more efficient.  I do not think occasional false hits would\nmatter much in practice.\n\nNot making the match case insensitive was probably a mistake.\nLooking for \"--author=linus\" would be easier to type.\n\nOne thing that I know is broken is the match at the tail; I am\nactually thinking of doing something like this:\n\n\tsprintf(pat, \"^%s %s [0-9][0-9]* [+-][0-9][0-9][0-9][0-9]$\",\n\t\tfield, pattern);\n\nto avoid matching the timestamps.\n\n> A few other thoughts:\n>   1. Case sensitivity?\n\nI do not think much is lost if we make the match always case\ninsensitive for this application.  It might be reasonable to to\ndefault to case insensitive match but if there is a pattern that\nhas an uppercase use case sensitive match.\n\n>  2. Is there any use to exposing the \"header_grep\" functionality with\n>     --grep-header?\n\nI considered it but did not think of any.  We obviously would\nneed to revisit this if we do \"note \" header in the future, but\nnot for now.  On the other hand, it might be useful to allow\n\n\t--grep-header='^\\(author\\|committer\\) Junio'\n\n>   3. An alias (--who=foo?) for --author=foo --committer=foo. I believe\n>      this doesn't require boolean magic, since we default to OR.\n\nThis should be trivial to implement (two calls to\nadd_header_grep instead of one), but would it be useful?  I\ndunno.\n"},{"id":"27122","messageId":"20060918065242.GA13691@coredump.intra.peff.net","threadId":"5594","inReplyTo":"7v4pv6yphp.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] rev-list: fix segfault with --{author,committer,grep}","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2006-09-18T06:52:42Z","receivedAt":"2006-09-18T06:52:42Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We need to save the commit buffer if we're going to match against it.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin-rev-list.c |    4 +++-\n 1 files changed, 3 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-rev-list.c b/builtin-rev-list.c\nindex 1f3333d..dbfee75 100644\n--- a/builtin-rev-list.c\n+++ b/builtin-rev-list.c\n@@ -269,7 +269,9 @@ int cmd_rev_list(int argc, const char **\n \t    revs.diff)\n \t\tusage(rev_list_usage);\n \n-\tsave_commit_buffer = revs.verbose_header;\n+\tsave_commit_buffer = revs.verbose_header ||\n+\t\trevs.header_filter ||\n+\t\trevs.message_filter;\n \ttrack_object_refs = 0;\n \tif (bisect_list)\n \t\trevs.limited = 1;\n-- \n1.4.2.1.g9f1c-dirty\n"},{"id":"27135","messageId":"Pine.LNX.4.64.0609181001361.4388@g5.osdl.org","threadId":"5594","inReplyTo":"7vslipsm4x.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 3/3] revision traversal: --author, --committer, and --grep.","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-09-18T17:07:51Z","receivedAt":"2006-09-18T17:07:51Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sun, 17 Sep 2006, Junio C Hamano wrote:\n> \n> I wanted to default it to left anchored, so this was somewhat\n> deliberate, but this is probably subject to taste.\n\nI know that I'd prefer a rule where\n\n \"--author=^Junio\"\n\nwould result in the grep-pattern being \"^author Junio\", but without the \ninitial '^' it would be \"^author .*Junio\".\n\nSo something like this, perhaps? It allows the regular left anchoring \nsyntax ('^' at the start of a pattern), but defaults to the default grep \nbehaviour (\"anywhere in the line\").\n\n\t\tLinus\n\n---\ndiff --git a/revision.c b/revision.c\nindex 26dd418..bca1229 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -677,6 +677,7 @@ int handle_revision_arg(const char *arg,\n static void add_header_grep(struct rev_info *revs, const char *field, const char *pattern)\n {\n \tchar *pat;\n+\tconst char *prefix;\n \tint patlen, fldlen;\n \n \tif (!revs->header_filter) {\n@@ -689,8 +690,13 @@ static void add_header_grep(struct rev_i\n \n \tfldlen = strlen(field);\n \tpatlen = strlen(pattern);\n-\tpat = xmalloc(patlen + fldlen + 3);\n-\tsprintf(pat, \"^%s %s\", field, pattern);\n+\tpat = xmalloc(patlen + fldlen + 10);\n+\tprefix = \".*\";\n+\tif (*pattern == '^') {\n+\t\tprefix = \"\";\n+\t\tpattern++;\n+\t}\n+\tsprintf(pat, \"^%s %s%s\", field, prefix, pattern);\n \tappend_grep_pattern(revs->header_filter, pat,\n \t\t\t    \"command line\", 0, GREP_PATTERN);\n }\n"}]}