{"thread":{"id":"31654","subject":"[PATCH] revision: add --reflog-message= to grep reflog messages","startedAt":"2012-09-26T12:12:45Z","lastAt":"2012-09-30T03:45:30Z","messageCount":25,"participants":["Nguyễn Thái Ngọc Duy","Junio C Hamano","Nguyen Thai Ngoc Duy","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"199944","messageId":"1348661565-30484-1-git-send-email-pclouds@gmail.com","threadId":"31654","inReplyTo":null,"subject":"[PATCH] revision: add --reflog-message= to grep reflog messages","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-09-26T12:12:45Z","receivedAt":"2012-09-26T12:12:45Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Both \"git log\" and \"git reflog show\" recognize this option.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n Itch: how to show reflogs for checkout operation only?\n\n Instead of ignoring when -g is not given, we might want to imply -g.\n\n Still itch: grep highlight! For all applicable areas: commit headers\n including reflog messages, commit body, diff.\n\n Documentation/rev-list-options.txt | 5 +++++\n revision.c | 30 ++++++++++++++++++++++++++++++\n revision.h | 1 +\n 3 files changed, 36 insertions(+)\n\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex 1fc2a18..aeaa58c 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -51,6 +51,11 @@ endif::git-rev-list[]\n \tcommits whose author matches any of the given patterns are\n \tchosen (similarly for multiple `--committer=`).\n \n+--reflog-message=::\n+\tLimit the commits output to ones with reflog messages that\n+\tmatch the specified pattern (regular expression). Ignored unless\n+\t--walk-reflogs is given.\n+\n --grep=::\n \n \tLimit the commits output to ones with log message that\ndiff --git a/revision.c b/revision.c\nindex ae12e11..ee55bb2 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1053,6 +1053,11 @@ void init_revisions(struct rev_info *revs, const char *prefix)\n \trevs->grep_filter.header_tail = &(revs->grep_filter.header_list);\n \trevs->grep_filter.regflags = REG_NEWLINE;\n \n+\trevs->reflog_filter.status_only = 1;\n+\trevs->reflog_filter.pattern_tail = &(revs->reflog_filter.pattern_list);\n+\trevs->reflog_filter.header_tail = &(revs->reflog_filter.header_list);\n+\trevs->reflog_filter.regflags = REG_NEWLINE;\n+\n \tdiff_setup(&revs->diffopt);\n \tif (prefix && !revs->diffopt.prefix) {\n \t\trevs->diffopt.prefix = prefix;\n@@ -1298,6 +1303,12 @@ static void add_message_grep(struct rev_info *revs, const char *pattern)\n \tadd_grep(revs, pattern, GREP_PATTERN_BODY);\n }\n \n+static void add_reflog_grep(struct rev_info *revs, const char *ptn)\n+{\n+\tappend_grep_pattern(&revs->reflog_filter, ptn,\n+\t\t\t \"command line\", 0, GREP_PATTERN);\n+}\n+\n static int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,\n \t\t\t int *unkc, const char **unkv)\n {\n@@ -1600,15 +1611,23 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\treturn argcount;\n \t} else if (!strcmp(arg, \"--grep-debug\")) {\n \t\trevs->grep_filter.debug = 1;\n+\t} else if ((argcount = parse_long_opt(\"reflog-message\",\n+\t\t\t\t\t argv, &optarg))) {\n+\t\tadd_reflog_grep(revs, optarg);\n+\t\treturn argcount;\n \t} else if (!strcmp(arg, \"--extended-regexp\") || !strcmp(arg, \"-E\")) {\n \t\trevs->grep_filter.regflags |= REG_EXTENDED;\n+\t\trevs->reflog_filter.regflags |= REG_EXTENDED;\n \t} else if (!strcmp(arg, \"--regexp-ignore-case\") || !strcmp(arg, \"-i\")) {\n \t\trevs->grep_filter.regflags |= REG_ICASE;\n+\t\trevs->reflog_filter.regflags |= REG_ICASE;\n \t\tDIFF_OPT_SET(&revs->diffopt, PICKAXE_IGNORE_CASE);\n \t} else if (!strcmp(arg, \"--fixed-strings\") || !strcmp(arg, \"-F\")) {\n \t\trevs->grep_filter.fixed = 1;\n+\t\trevs->reflog_filter.fixed = 1;\n \t} else if (!strcmp(arg, \"--all-match\")) {\n \t\trevs->grep_filter.all_match = 1;\n+\t\trevs->reflog_filter.all_match = 1;\n \t} else if ((argcount = parse_long_opt(\"encoding\", argv, &optarg))) {\n \t\tif (strcmp(optarg, \"none\"))\n \t\t\tgit_log_output_encoding = xstrdup(optarg);\n@@ -1891,6 +1910,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \tdiff_setup_done(&revs->diffopt);\n \n \tcompile_grep_patterns(&revs->grep_filter);\n+\tcompile_grep_patterns(&revs->reflog_filter);\n \n \tif (revs->reverse && revs->reflog_info)\n \t\tdie(\"cannot combine --reverse with --walk-reflogs\");\n@@ -2242,6 +2262,16 @@ enum commit_action get_commit_action(struct rev_info *revs, struct commit *commi\n \t\t ((revs->max_parents >= 0) && (n > revs->max_parents)))\n \t\t\treturn commit_ignore;\n \t}\n+\tif (revs->reflog_info &&\n+\t revs->reflog_filter.pattern_list) {\n+\t\tstruct strbuf sb = STRBUF_INIT;\n+\t\tint ignore;\n+\t\tget_reflog_message(&sb, revs->reflog_info);\n+\t\tignore = !grep_buffer(&revs->reflog_filter, sb.buf, sb.len);\n+\t\tstrbuf_release(&sb);\n+\t\tif (ignore)\n+\t\t\treturn commit_ignore;\n+\t}\n \tif (!commit_match(commit, revs))\n \t\treturn commit_ignore;\n \tif (revs->prune && revs->dense) {\ndiff --git a/revision.h b/revision.h\nindex a95bd0b..0ebe34b 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -145,6 +145,7 @@ struct rev_info {\n \n \t/* Filter by commit log message */\n \tstruct grep_opt\tgrep_filter;\n+\tstruct grep_opt\treflog_filter;\n \n \t/* Display history graph */\n \tstruct git_graph *graph;\n-- \n1.7.12.1.406.g6ab07c4\n"},{"id":"199947","messageId":"7vpq58ubbx.fsf@alter.siamese.dyndns.org","threadId":"31654","inReplyTo":"1348661565-30484-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] revision: add --reflog-message= to grep reflog messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-26T14:07:30Z","receivedAt":"2012-09-26T14:07:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> Both \"git log\" and \"git reflog show\" recognize this option.\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n\nHow well does it interact with --grep and/or --all-match?\n"},{"id":"199949","messageId":"CACsJy8D=OPtbuygjSFx+Wcwo=hgq2RsumgeNzpNk-4QGH1CVdg@mail.gmail.com","threadId":"31654","inReplyTo":"7vpq58ubbx.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] revision: add --reflog-message= to grep reflog messages","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-09-26T14:15:43Z","receivedAt":"2012-09-26T14:15:43Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Sep 26, 2012 at 9:07 PM, Junio C Hamano wrote:\n> Nguyễn Thái Ngọc Duy writes:\n>\n>> Both \"git log\" and \"git reflog show\" recognize this option.\n>>\n>> Signed-off-by: Nguyễn Thái Ngọc Duy \n>> ---\n>\n> How well does it interact with --grep and/or --all-match?\n\nGood point. It currently works like and operator. But people might\nexpect to combine them in different ways.\n-- \nDuy\n"},{"id":"199962","messageId":"7va9wctwg4.fsf@alter.siamese.dyndns.org","threadId":"31654","inReplyTo":"CACsJy8D=OPtbuygjSFx+Wcwo=hgq2RsumgeNzpNk-4QGH1CVdg@mail.gmail.com","subject":"Re: [PATCH] revision: add --reflog-message= to grep reflog messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-26T19:28:59Z","receivedAt":"2012-09-26T19:28:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyen Thai Ngoc Duy writes:\n\n> On Wed, Sep 26, 2012 at 9:07 PM, Junio C Hamano wrote:\n>> Nguyễn Thái Ngọc Duy writes:\n>>\n>>> Both \"git log\" and \"git reflog show\" recognize this option.\n>>>\n>>> Signed-off-by: Nguyễn Thái Ngọc Duy \n>>> ---\n>>\n>> How well does it interact with --grep and/or --all-match?\n>\n> Good point. It currently works like and operator. But people might\n> expect to combine them in different ways.\n\nThe current commit_match() runs grep_buffer() on commit->buffer. It\nprobably makes sense to instead notice from opt that we are running\nlog with \"-g\", prepare a temporary strbuf and add in the reflog\nmessage to the string in commit->buffer, and run grep_buffer() on\nthat temporary strbuf on it.\n\nI personally think it is sufficient ot just reuse --grep on\nconcatenation of commit->buffer with \"Reflog message: checkout:\nmoving from as/check-ignore to pu\".\n\nIf you really want to go fancier, you could add --grep-reflog option\nthat behaves like the existing --author and --committer options to\nadd \"header match\" elements to the grep expression, splice a fake\n\"reflog \" header to the string copied from commit->buffer, e.g.\nprepare something like this in your temporary strbuf:\n\n tree b4429f218782165faf101ccb0f4ba1cdd6d1d349\n parent de5cd03876e546d6d264ab28a01daa978f3eae78\n parent b378e5a25658e07e6d0c0f4db79e87cb21de5489\n author Junio C Hamano 1348616180 -0700\n committer Junio C Hamano 1348616180 -0700\n reflog checkout: moving from as/check-ignore to pu\n\n Merge branch 'jk/lua-hackery' into pu\n\n * jk/lua-hackery:\n Minimum compilation fixup\n Makefile: make \"lua\" a bit more configurable\n add a \"lua\" pretty format\n add basic lua infrastructure\n pretty: make some commit-parsing helpers more public\n\nthat way, you can take advantage of the existing logic used for the\nauthor/committer match that matches only in the commit object\nheader.\n\nAgain, I personally doubt the fancier option is worth it, but the\nstarting point may look something like this.\n\n revision.c | 16 ++++++++++++++--\n 1 file changed, 14 insertions(+), 2 deletions(-)\n\ndiff --git c/revision.c w/revision.c\nindex ae12e11..b0f4d5b 100644\n--- c/revision.c\n+++ w/revision.c\n@@ -2212,8 +2212,20 @@ static int commit_match(struct commit *commit, struct rev_info *opt)\n {\n \tif (!opt->grep_filter.pattern_list && !opt->grep_filter.header_list)\n \t\treturn 1;\n-\treturn grep_buffer(&opt->grep_filter,\n-\t\t\t commit->buffer, strlen(commit->buffer));\n+\n+\tif (opt->reflog_info) {\n+\t\tint retval;\n+\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\tstrbuf_addf(&buf, \"reflog %s\\n\", opt->reflog_info->message);\n+\t\tstrbuf_addstr(&buf, commit->buffer);\n+\t\tretval = grep_buffer(&opt->grep_filter,\n+\t\t\t\t buf.buf, buf.len);\n+\t\tstrbuf_release(&buf);\n+\t\treturn retval;\n+\t} else {\n+\t\treturn grep_buffer(&opt->grep_filter,\n+\t\t\t\t commit->buffer, strlen(commit->buffer));\n+\t}\n }\n \n static inline int want_ancestry(struct rev_info *revs)\n"},{"id":"200004","messageId":"1348745786-27197-1-git-send-email-pclouds@gmail.com","threadId":"31654","inReplyTo":"7va9wctwg4.fsf@alter.siamese.dyndns.org","subject":"[PATCH] revision: add --reflog-message to grep reflog messages","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-09-27T11:36:26Z","receivedAt":"2012-09-27T11:36:26Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n On Thu, Sep 27, 2012 at 2:28 AM, Junio C Hamano wrote:\n > The current commit_match() runs grep_buffer() on commit->buffer. It\n > probably makes sense to instead notice from opt that we are running\n > log with \"-g\", prepare a temporary strbuf and add in the reflog\n > message to the string in commit->buffer, and run grep_buffer() on\n > that temporary strbuf on it.\n\n Yeah. I was starting to think that way too, otherwise combining grep\n options would be a mess. I was hoping by injecting a fake reflog\n header, we could simplify reflog exceptions in pretty.c. But it\n was not that easy.\n\n > I personally think it is sufficient ot just reuse --grep on\n > concatenation of commit->buffer with \"Reflog message: checkout:\n > moving from as/check-ignore to pu\".\n\n --grep only reads the commit body. So either we append reflog\n message to commit body, or we put it in the header and add a new\n option for it. I don't like appending things to the commit body as\n --grep may hit reflog message while users do not mean so.\n\n > If you really want to go fancier, you could add --grep-reflog option\n > that behaves like the existing --author and --committer options to\n > add \"header match\" elements to the grep expression, splice a fake\n > \"reflog \" header to the string copied from commit->buffer\n\n Inserting at the beginning of the commit like your demo patch works\n just fine and is simpler. I'm tempted to take the undocumented option\n name --reflog for this purpose. But it's probably not worth the risk.\n\n Documentation/rev-list-options.txt | 5 +++++\n grep.c | 1 +\n grep.h | 5 +++--\n revision.c | 19 +++++++++++++++++--\n t/t7810-grep.sh | 6 ++++++\n 5 files changed, 32 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex 1fc2a18..aeaa58c 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -51,6 +51,11 @@ endif::git-rev-list[]\n \tcommits whose author matches any of the given patterns are\n \tchosen (similarly for multiple `--committer=`).\n \n+--reflog-message=::\n+\tLimit the commits output to ones with reflog entries that\n+\tmatch the specified pattern (regular expression). Ignored unless\n+\t--walk-reflogs is given.\n+\n --grep=::\n \n \tLimit the commits output to ones with log message that\ndiff --git a/grep.c b/grep.c\nindex 898be6e..72ac1bf 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -697,6 +697,7 @@ static struct {\n } header_field[] = {\n \t{ \"author \", 7 },\n \t{ \"committer \", 10 },\n+\t{ \"reflog \", 7 },\n };\n \n static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,\ndiff --git a/grep.h b/grep.h\nindex 8a28a67..1416ad7 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -29,9 +29,10 @@ enum grep_context {\n \n enum grep_header_field {\n \tGREP_HEADER_AUTHOR = 0,\n-\tGREP_HEADER_COMMITTER\n+\tGREP_HEADER_COMMITTER,\n+\tGREP_HEADER_REFLOG,\n+\tGREP_HEADER_FIELD_MAX\n };\n-#define GREP_HEADER_FIELD_MAX (GREP_HEADER_COMMITTER + 1)\n \n struct grep_pat {\n \tstruct grep_pat *next;\ndiff --git a/revision.c b/revision.c\nindex ae12e11..837051c 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1595,6 +1595,9 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t} else if ((argcount = parse_long_opt(\"committer\", argv, &optarg))) {\n \t\tadd_header_grep(revs, GREP_HEADER_COMMITTER, optarg);\n \t\treturn argcount;\n+\t} else if ((argcount = parse_long_opt(\"reflog-message\", argv, &optarg))) {\n+\t\tadd_header_grep(revs, GREP_HEADER_REFLOG, optarg);\n+\t\treturn argcount;\n \t} else if ((argcount = parse_long_opt(\"grep\", argv, &optarg))) {\n \t\tadd_message_grep(revs, optarg);\n \t\treturn argcount;\n@@ -2212,8 +2215,20 @@ static int commit_match(struct commit *commit, struct rev_info *opt)\n {\n \tif (!opt->grep_filter.pattern_list && !opt->grep_filter.header_list)\n \t\treturn 1;\n-\treturn grep_buffer(&opt->grep_filter,\n-\t\t\t commit->buffer, strlen(commit->buffer));\n+\tif (opt->reflog_info) {\n+\t\tint retval;\n+\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\tstrbuf_addstr(&buf, \"reflog \");\n+\t\tget_reflog_message(&buf, opt->reflog_info);\n+\t\tstrbuf_addch(&buf, '\\n');\n+\t\tstrbuf_addstr(&buf, commit->buffer);\n+\t\tretval = grep_buffer(&opt->grep_filter, buf.buf, buf.len);\n+\t\tstrbuf_release(&buf);\n+\t\treturn retval;\n+\t} else {\n+\t\treturn grep_buffer(&opt->grep_filter,\n+\t\t\t\t commit->buffer, strlen(commit->buffer));\n+\t}\n }\n \n static inline int want_ancestry(struct rev_info *revs)\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex 91db352..a0f519e 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -546,6 +546,12 @@ test_expect_success 'log grep (6)' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'log grep (7)' '\n+\tgit log -g --reflog-message=\"commit: third\" --pretty=tformat:%s >actual &&\n+\techo third >expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'log with multiple --grep uses union' '\n \tgit log --grep=i --grep=r --format=%s >actual &&\n \t{\n-- \n1.7.12.1.406.g6ab07c4\n"},{"id":"200018","messageId":"7v4nmjs88n.fsf@alter.siamese.dyndns.org","threadId":"31654","inReplyTo":"1348745786-27197-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] revision: add --reflog-message to grep reflog messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-27T17:09:28Z","receivedAt":"2012-09-27T17:09:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n\nPlase explain yourself in the space above.\n\n> diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\n> index 1fc2a18..aeaa58c 100644\n> --- a/Documentation/rev-list-options.txt\n> +++ b/Documentation/rev-list-options.txt\n> @@ -51,6 +51,11 @@ endif::git-rev-list[]\n> \tcommits whose author matches any of the given patterns are\n> \tchosen (similarly for multiple `--committer=`).\n> \n> +--reflog-message=::\n> +\tLimit the commits output to ones with reflog entries that\n> +\tmatch the specified pattern (regular expression). Ignored unless\n> +\t--walk-reflogs is given.\n> +\n\nI am debating myself if it is sane for this option to have no hint\nthat it is about \"limiting\" in its name. \"--author/--committer\"\ndon't and it is clear from the context of the command that they are\nnot about setting author/committer, so \"--reflog-message\" may be\ninterpreted the same, perhaps.\n\nThe entry in the context above talks about multiple occurrence of\nthat option. Shouldn't this new one also say what happens when it is\ngiven twice?\n\n> diff --git a/grep.c b/grep.c\n> index 898be6e..72ac1bf 100644\n> --- a/grep.c\n> +++ b/grep.c\n> @@ -697,6 +697,7 @@ static struct {\n> } header_field[] = {\n> \t{ \"author \", 7 },\n> \t{ \"committer \", 10 },\n> +\t{ \"reflog \", 7 },\n> };\n> \n> static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,\n> diff --git a/grep.h b/grep.h\n> index 8a28a67..1416ad7 100644\n> --- a/grep.h\n> +++ b/grep.h\n> @@ -29,9 +29,10 @@ enum grep_context {\n> \n> enum grep_header_field {\n> \tGREP_HEADER_AUTHOR = 0,\n> -\tGREP_HEADER_COMMITTER\n> +\tGREP_HEADER_COMMITTER,\n> +\tGREP_HEADER_REFLOG,\n> +\tGREP_HEADER_FIELD_MAX\n> };\n> -#define GREP_HEADER_FIELD_MAX (GREP_HEADER_COMMITTER + 1)\n\nPlease add comment to ensure that FIELD_MAX stays at the end; if you\nensure that, the result is much better than the original \"we know\ncommitter is at the end so add one\".\n\nI think I wrote prep_header_patterns() and compile_grep_patterns()\ncarefully enough not to assume the headers are only the author and\ncommitter names, so the various combinations i.e. all-match,\nauthor(s), committer(s), grep(s), and reflog-message(s), should work\nout of the box, but have you actually tested them?\n\nI do not know offhand the matching side is prepared to take random\ngarbage fields. IIRC, we strip the trailing timestamp from committer\nand author header lines when we match, and a new code needs to be\nadded to control when that stripping should / should not kick in\ndepending on the header.\n\n> diff --git a/revision.c b/revision.c\n> index ae12e11..837051c 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -1595,6 +1595,9 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n> \t} else if ((argcount = parse_long_opt(\"committer\", argv, &optarg))) {\n> \t\tadd_header_grep(revs, GREP_HEADER_COMMITTER, optarg);\n> \t\treturn argcount;\n> +\t} else if ((argcount = parse_long_opt(\"reflog-message\", argv, &optarg))) {\n> +\t\tadd_header_grep(revs, GREP_HEADER_REFLOG, optarg);\n> +\t\treturn argcount;\n> \t} else if ((argcount = parse_long_opt(\"grep\", argv, &optarg))) {\n> \t\tadd_message_grep(revs, optarg);\n> \t\treturn argcount;\n> @@ -2212,8 +2215,20 @@ static int commit_match(struct commit *commit, struct rev_info *opt)\n> {\n> \tif (!opt->grep_filter.pattern_list && !opt->grep_filter.header_list)\n> \t\treturn 1;\n> -\treturn grep_buffer(&opt->grep_filter,\n> -\t\t\t commit->buffer, strlen(commit->buffer));\n> +\tif (opt->reflog_info) {\n> +\t\tint retval;\n> +\t\tstruct strbuf buf = STRBUF_INIT;\n> +\t\tstrbuf_addstr(&buf, \"reflog \");\n> +\t\tget_reflog_message(&buf, opt->reflog_info);\n> +\t\tstrbuf_addch(&buf, '\\n');\n> +\t\tstrbuf_addstr(&buf, commit->buffer);\n> +\t\tretval = grep_buffer(&opt->grep_filter, buf.buf, buf.len);\n> +\t\tstrbuf_release(&buf);\n> +\t\treturn retval;\n> +\t} else {\n> +\t\treturn grep_buffer(&opt->grep_filter,\n> +\t\t\t\t commit->buffer, strlen(commit->buffer));\n> +\t}\n> }\n\nThis part looks familiar and smells sane ;-)\n"},{"id":"200022","messageId":"20120927172844.GC1547@sigill.intra.peff.net","threadId":"31654","inReplyTo":"7v4nmjs88n.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] revision: add --reflog-message to grep reflog messages","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-09-27T17:28:44Z","receivedAt":"2012-09-27T17:28:44Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 27, 2012 at 10:09:28AM -0700, Junio C Hamano wrote:\n\n> > +--reflog-message=::\n> > +\tLimit the commits output to ones with reflog entries that\n> > +\tmatch the specified pattern (regular expression). Ignored unless\n> > +\t--walk-reflogs is given.\n> > +\n> \n> I am debating myself if it is sane for this option to have no hint\n> that it is about \"limiting\" in its name. \"--author/--committer\"\n> don't and it is clear from the context of the command that they are\n> not about setting author/committer, so \"--reflog-message\" may be\n> interpreted the same, perhaps.\n\nI also found the name confusing on first-read. While \"--author\" is an\nexample in one direction, the fact that \"--grep\" is not called \"--body\"\nis a counter-example.\n\nI'd much rather see it as \"--grep-reflog\" or something. You could also\ndo \"--grep-reflog-message\", which would match a later\n\"--grep-reflog-author\", but I am not sure anybody would want the latter,\nand it makes the current name a lot longer.\n\nI actually think just checking the reflog when we call \"--grep\" would\nthe most common workflow, and requires no extra work from the user. My\nonly hesitation is that if somebody _does_ want to distinguish, there's\nno escape hatch. Of course, the reflog walker is already full of such\nweird conflations (e.g., the rewriting of parent and date information).\n\n-Peff\n"},{"id":"200031","messageId":"7vbogrqqjp.fsf@alter.siamese.dyndns.org","threadId":"31654","inReplyTo":"20120927172844.GC1547@sigill.intra.peff.net","subject":"Re: [PATCH] revision: add --reflog-message to grep reflog messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-27T18:16:58Z","receivedAt":"2012-09-27T18:16:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I actually think just checking the reflog when we call \"--grep\" would\n> the most common workflow, and requires no extra work from the user. My\n> only hesitation is that if somebody _does_ want to distinguish, there's\n> no escape hatch. Of course, the reflog walker is already full of such\n> weird conflations (e.g., the rewriting of parent and date information).\n\nYes, that reasoning more-or-less matches the reason why I said we do\nnot necessarily want to go \"fancier\". But \"log -g\" output already\nmakes it fairly clear that the additional information is not part of\nthe regular log and is a some header-like thingy, so I actually am\nOK with --reflog-grep and --grep being different.\n\nThe output from \"log --show-notes\", on the other hand, is even more\nconflated and a casual user would view it as part of the message, so\nI would imagine that if we ever do the extention to cover notes\ndata, the normal \"--grep\" should apply to it.\n"},{"id":"200068","messageId":"1348815682-18696-1-git-send-email-pclouds@gmail.com","threadId":"31654","inReplyTo":"1348745786-27197-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 1/3] grep: generalize header grep code to accept arbitrary headers","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-09-28T07:01:20Z","receivedAt":"2012-09-28T07:01:20Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n On Fri, Sep 28, 2012 at 12:09 AM, Junio C Hamano wrote:\n >> enum grep_header_field {\n >> GREP_HEADER_AUTHOR = 0,\n >> - GREP_HEADER_COMMITTER\n >> + GREP_HEADER_COMMITTER,\n >> + GREP_HEADER_REFLOG,\n >> + GREP_HEADER_FIELD_MAX\n >> };\n >> -#define GREP_HEADER_FIELD_MAX (GREP_HEADER_COMMITTER + 1)\n >\n > Please add comment to ensure that FIELD_MAX stays at the end; if you\n > ensure that, the result is much better than the original \"we know\n > committer is at the end so add one\".\n\n It's probably even better to remove the enum. Say one day I got drunk\n and decided to add --grep-encoding. This patch makes sure that I could\n submit such a patch even in drunk state.\n\n grep.c | 76 +++++++++++++++++++++++++++++++++-----------------------------\n grep.h | 11 +++------\n revision.c | 8 +++----\n 3 files changed, 47 insertions(+), 48 deletions(-)\n\ndiff --git a/grep.c b/grep.c\nindex 898be6e..0c72262 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -10,7 +10,7 @@ static int grep_source_is_binary(struct grep_source *gs);\n static struct grep_pat *create_grep_pat(const char *pat, size_t patlen,\n \t\t\t\t\tconst char *origin, int no,\n \t\t\t\t\tenum grep_pat_token t,\n-\t\t\t\t\tenum grep_header_field field)\n+\t\t\t\t\tconst char *header, size_t header_len)\n {\n \tstruct grep_pat *p = xcalloc(1, sizeof(*p));\n \tp->pattern = xmemdupz(pat, patlen);\n@@ -18,7 +18,8 @@ static struct grep_pat *create_grep_pat(const char *pat, size_t patlen,\n \tp->origin = origin;\n \tp->no = no;\n \tp->token = t;\n-\tp->field = field;\n+\tp->header = header;\n+\tp->header_len = header_len;\n \treturn p;\n }\n \n@@ -45,7 +46,8 @@ static void do_append_grep_pat(struct grep_pat ***tail, struct grep_pat *p)\n \t\t\tif (!nl)\n \t\t\t\tbreak;\n \t\t\tnew_pat = create_grep_pat(nl + 1, len - 1, p->origin,\n-\t\t\t\t\t\t p->no, p->token, p->field);\n+\t\t\t\t\t\t p->no, p->token,\n+\t\t\t\t\t\t p->header, p->header_len);\n \t\t\tnew_pat->next = p->next;\n \t\t\tif (!p->next)\n \t\t\t\t*tail = &new_pat->next;\n@@ -59,11 +61,13 @@ static void do_append_grep_pat(struct grep_pat ***tail, struct grep_pat *p)\n \t}\n }\n \n+/* header must not be freed while grep is running */\n void append_header_grep_pattern(struct grep_opt *opt,\n-\t\t\t\tenum grep_header_field field, const char *pat)\n+\t\t\t\tconst char *header, const char *pat)\n {\n \tstruct grep_pat *p = create_grep_pat(pat, strlen(pat), \"header\", 0,\n-\t\t\t\t\t GREP_PATTERN_HEAD, field);\n+\t\t\t\t\t GREP_PATTERN_HEAD,\n+\t\t\t\t\t header, strlen(header));\n \tdo_append_grep_pat(&opt->header_tail, p);\n }\n \n@@ -76,7 +80,7 @@ void append_grep_pattern(struct grep_opt *opt, const char *pat,\n void append_grep_pat(struct grep_opt *opt, const char *pat, size_t patlen,\n \t\t const char *origin, int no, enum grep_pat_token t)\n {\n-\tstruct grep_pat *p = create_grep_pat(pat, patlen, origin, no, t, 0);\n+\tstruct grep_pat *p = create_grep_pat(pat, patlen, origin, no, t, NULL, 0);\n \tdo_append_grep_pat(&opt->pattern_tail, p);\n }\n \n@@ -92,7 +96,7 @@ struct grep_opt *grep_opt_dup(const struct grep_opt *opt)\n \tfor(pat = opt->pattern_list; pat != NULL; pat = pat->next)\n \t{\n \t\tif(pat->token == GREP_PATTERN_HEAD)\n-\t\t\tappend_header_grep_pattern(ret, pat->field,\n+\t\t\tappend_header_grep_pattern(ret, pat->header,\n \t\t\t\t\t\t pat->pattern);\n \t\telse\n \t\t\tappend_grep_pat(ret, pat->pattern, pat->patternlen,\n@@ -359,7 +363,7 @@ static void dump_grep_pat(struct grep_pat *p)\n \tswitch (p->token) {\n \tdefault: break;\n \tcase GREP_PATTERN_HEAD:\n-\t\tfprintf(stderr, \"\", p->field); break;\n+\t\tfprintf(stderr, \"\", p->header); break;\n \tcase GREP_PATTERN_BODY:\n \t\tfprintf(stderr, \"\"); break;\n \t}\n@@ -436,9 +440,10 @@ static struct grep_expr *grep_or_expr(struct grep_expr *left, struct grep_expr *\n static struct grep_expr *prep_header_patterns(struct grep_opt *opt)\n {\n \tstruct grep_pat *p;\n-\tstruct grep_expr *header_expr;\n-\tstruct grep_expr *(header_group[GREP_HEADER_FIELD_MAX]);\n-\tenum grep_header_field fld;\n+\tstruct grep_expr *header_expr = NULL;\n+\tstruct grep_expr **header_group;\n+\tstruct string_list header = STRING_LIST_INIT_NODUP;\n+\tint fld;\n \n \tif (!opt->header_list)\n \t\treturn NULL;\n@@ -446,37 +451,45 @@ static struct grep_expr *prep_header_patterns(struct grep_opt *opt)\n \tfor (p = opt->header_list; p; p = p->next) {\n \t\tif (p->token != GREP_PATTERN_HEAD)\n \t\t\tdie(\"bug: a non-header pattern in grep header list.\");\n-\t\tif (p->field < 0 || GREP_HEADER_FIELD_MAX <= p->field)\n-\t\t\tdie(\"bug: unknown header field %d\", p->field);\n+\t\tif (!p->header || !p->header_len)\n+\t\t\tdie(\"bug: unknown header field\");\n \t\tcompile_regexp(p, opt);\n+\t\tstring_list_append(&header, p->header);\n \t}\n \n-\tfor (fld = 0; fld < GREP_HEADER_FIELD_MAX; fld++)\n-\t\theader_group[fld] = NULL;\n+\tsort_string_list(&header);\n+\tstring_list_remove_duplicates(&header, 0);\n+\theader_group = xmalloc(sizeof(*header_group) * header.nr);\n+\tmemset(header_group, 0, sizeof(*header_group) * header.nr);\n \n \tfor (p = opt->header_list; p; p = p->next) {\n \t\tstruct grep_expr *h;\n \t\tstruct grep_pat *pp = p;\n+\t\tstruct string_list_item *item;\n \n \t\th = compile_pattern_atom(&pp);\n \t\tif (!h || pp != p->next)\n \t\t\tdie(\"bug: malformed header expr\");\n-\t\tif (!header_group[p->field]) {\n-\t\t\theader_group[p->field] = h;\n+\t\titem = string_list_lookup(&header, p->header);\n+\t\tif (!item)\n+\t\t\tdie(\"bug: malformed header expr\");\n+\t\tfld = item - header.items;\n+\t\tif (!header_group[fld]) {\n+\t\t\theader_group[fld] = h;\n \t\t\tcontinue;\n \t\t}\n-\t\theader_group[p->field] = grep_or_expr(h, header_group[p->field]);\n+\t\theader_group[fld] = grep_or_expr(h, header_group[fld]);\n \t}\n \n-\theader_expr = NULL;\n-\n-\tfor (fld = 0; fld < GREP_HEADER_FIELD_MAX; fld++) {\n-\t\tif (!header_group[fld])\n-\t\t\tcontinue;\n+\tfor (fld = 0; fld < header.nr; fld++) {\n \t\tif (!header_expr)\n \t\t\theader_expr = grep_true_expr();\n \t\theader_expr = grep_or_expr(header_group[fld], header_expr);\n \t}\n+\n+\tstring_list_clear(&header, 0);\n+\tfree(header_group);\n+\n \treturn header_expr;\n }\n \n@@ -691,14 +704,6 @@ static int strip_timestamp(char *bol, char **eol_p)\n \treturn 0;\n }\n \n-static struct {\n-\tconst char *field;\n-\tsize_t len;\n-} header_field[] = {\n-\t{ \"author \", 7 },\n-\t{ \"committer \", 10 },\n-};\n-\n static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,\n \t\t\t enum grep_context ctx,\n \t\t\t regmatch_t *pmatch, int eflags)\n@@ -714,12 +719,11 @@ static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,\n \tif (p->token == GREP_PATTERN_HEAD) {\n \t\tconst char *field;\n \t\tsize_t len;\n-\t\tassert(p->field < ARRAY_SIZE(header_field));\n-\t\tfield = header_field[p->field].field;\n-\t\tlen = header_field[p->field].len;\n-\t\tif (strncmp(bol, field, len))\n+\t\tfield = p->header;\n+\t\tlen = p->header_len;\n+\t\tif (strncmp(bol, field, len) || bol[len] != ' ')\n \t\t\treturn 0;\n-\t\tbol += len;\n+\t\tbol += len + 1;\n \t\tsaved_ch = strip_timestamp(bol, &eol);\n \t}\n \ndiff --git a/grep.h b/grep.h\nindex 8a28a67..bc00f2a 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -27,12 +27,6 @@ enum grep_context {\n \tGREP_CONTEXT_BODY\n };\n \n-enum grep_header_field {\n-\tGREP_HEADER_AUTHOR = 0,\n-\tGREP_HEADER_COMMITTER\n-};\n-#define GREP_HEADER_FIELD_MAX (GREP_HEADER_COMMITTER + 1)\n-\n struct grep_pat {\n \tstruct grep_pat *next;\n \tconst char *origin;\n@@ -40,7 +34,8 @@ struct grep_pat {\n \tenum grep_pat_token token;\n \tchar *pattern;\n \tsize_t patternlen;\n-\tenum grep_header_field field;\n+\tconst char *header;\n+\tsize_t header_len;\n \tregex_t regexp;\n \tpcre *pcre_regexp;\n \tpcre_extra *pcre_extra_info;\n@@ -136,7 +131,7 @@ struct grep_opt {\n \n extern void append_grep_pat(struct grep_opt *opt, const char *pat, size_t patlen, const char *origin, int no, enum grep_pat_token t);\n extern void append_grep_pattern(struct grep_opt *opt, const char *pat, const char *origin, int no, enum grep_pat_token t);\n-extern void append_header_grep_pattern(struct grep_opt *, enum grep_header_field, const char *);\n+extern void append_header_grep_pattern(struct grep_opt *, const char *, const char *);\n extern void compile_grep_patterns(struct grep_opt *opt);\n extern void free_grep_patterns(struct grep_opt *opt);\n extern int grep_buffer(struct grep_opt *opt, char *buf, unsigned long size);\ndiff --git a/revision.c b/revision.c\nindex ae12e11..f7cf385 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1288,9 +1288,9 @@ static void add_grep(struct rev_info *revs, const char *ptn, enum grep_pat_token\n \tappend_grep_pattern(&revs->grep_filter, ptn, \"command line\", 0, what);\n }\n \n-static void add_header_grep(struct rev_info *revs, enum grep_header_field field, const char *pattern)\n+static void add_header_grep(struct rev_info *revs, const char *header, const char *pattern)\n {\n-\tappend_header_grep_pattern(&revs->grep_filter, field, pattern);\n+\tappend_header_grep_pattern(&revs->grep_filter, header, pattern);\n }\n \n static void add_message_grep(struct rev_info *revs, const char *pattern)\n@@ -1590,10 +1590,10 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t * Grepping the commit log\n \t */\n \telse if ((argcount = parse_long_opt(\"author\", argv, &optarg))) {\n-\t\tadd_header_grep(revs, GREP_HEADER_AUTHOR, optarg);\n+\t\tadd_header_grep(revs, \"author\", optarg);\n \t\treturn argcount;\n \t} else if ((argcount = parse_long_opt(\"committer\", argv, &optarg))) {\n-\t\tadd_header_grep(revs, GREP_HEADER_COMMITTER, optarg);\n+\t\tadd_header_grep(revs, \"committer\", optarg);\n \t\treturn argcount;\n \t} else if ((argcount = parse_long_opt(\"grep\", argv, &optarg))) {\n \t\tadd_message_grep(revs, optarg);\n-- \n1.7.12.1.405.gb727dc9\n"},{"id":"200069","messageId":"1348815682-18696-2-git-send-email-pclouds@gmail.com","threadId":"31654","inReplyTo":"1348815682-18696-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 2/3] revision: add --grep-reflog to filter commits by reflog messages","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-09-28T07:01:21Z","receivedAt":"2012-09-28T07:01:21Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Similar to --author/--committer which filters commits by author and\ncommitter header fields. --grep-reflog adds a fake \"reflog\" header to\ncommit and a grep filter to search on that line.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n On Fri, Sep 28, 2012 at 12:09 AM, Junio C Hamano \n wrote:\n > I am debating myself if it is sane for this option to have no hint\n > that it is about \"limiting\" in its name. \"--author/--committer\"\n > don't and it is clear from the context of the command that they are\n > not about setting author/committer, so \"--reflog-message\" may be\n > interpreted the same, perhaps.\n >\n > The entry in the context above talks about multiple occurrence of\n > that option. Shouldn't this new one also say what happens when it\n > is\n > given twice?\n\n Fixed.\n\n > I think I wrote prep_header_patterns() and compile_grep_patterns()\n > carefully enough not to assume the headers are only the author and\n > committer names, so the various combinations i.e. all-match,\n > author(s), committer(s), grep(s), and reflog-message(s), should\n > work\n > out of the box, but have you actually tested them?\n\n I did not. I do now, tests are also added.\n\n On Fri, Sep 28, 2012 at 12:28 AM, Jeff King wrote:\n > I also found the name confusing on first-read. While \"--author\" is\n > an\n > example in one direction, the fact that \"--grep\" is not called\n > \"--body\"\n > is a counter-example.\n >\n > I'd much rather see it as \"--grep-reflog\" or something. You could\n > also\n > do \"--grep-reflog-message\", which would match a later\n > \"--grep-reflog-author\", but I am not sure anybody would want the\n > latter,\n > and it makes the current name a lot longer.\n\n Changed it to --grep-reflog. I was tempted to add --grep-* but I\n can't error out if user gives an invalid header. So --grep-reflog\n only for now.\n\n Documentation/rev-list-options.txt | 8 ++++++++\n revision.c | 20 ++++++++++++++++++--\n t/t7810-grep.sh | 26 ++++++++++++++++++++++++++\n 3 files changed, 52 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex 1fc2a18..aa7cd9d 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -51,6 +51,14 @@ endif::git-rev-list[]\n \tcommits whose author matches any of the given patterns are\n \tchosen (similarly for multiple `--committer=`).\n \n+--grep-reflog=::\n+\n+\tLimit the commits output to ones with reflog entries that\n+\tmatch the specified pattern (regular expression). With\n+\tmore than one `--grep-reflog`, commits whose reflog message\n+\tmatches any of the given patterns are chosen. Ignored unless\n+\t`--walk-reflogs` is given.\n+\n --grep=::\n \n \tLimit the commits output to ones with log message that\ndiff --git a/revision.c b/revision.c\nindex f7cf385..cfa0e2e 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1595,6 +1595,9 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t} else if ((argcount = parse_long_opt(\"committer\", argv, &optarg))) {\n \t\tadd_header_grep(revs, \"committer\", optarg);\n \t\treturn argcount;\n+\t} else if ((argcount = parse_long_opt(\"grep-reflog\", argv, &optarg))) {\n+\t\tadd_header_grep(revs, \"reflog\", optarg);\n+\t\treturn argcount;\n \t} else if ((argcount = parse_long_opt(\"grep\", argv, &optarg))) {\n \t\tadd_message_grep(revs, optarg);\n \t\treturn argcount;\n@@ -2210,10 +2213,23 @@ static int rewrite_parents(struct rev_info *revs, struct commit *commit)\n \n static int commit_match(struct commit *commit, struct rev_info *opt)\n {\n+\tint retval;\n+\tstruct strbuf buf = STRBUF_INIT;\n \tif (!opt->grep_filter.pattern_list && !opt->grep_filter.header_list)\n \t\treturn 1;\n-\treturn grep_buffer(&opt->grep_filter,\n-\t\t\t commit->buffer, strlen(commit->buffer));\n+\tif (opt->reflog_info) {\n+\t\tstrbuf_addstr(&buf, \"reflog \");\n+\t\tget_reflog_message(&buf, opt->reflog_info);\n+\t\tstrbuf_addch(&buf, '\\n');\n+\t\tstrbuf_addstr(&buf, commit->buffer);\n+\t}\n+\tif (buf.len)\n+\t\tretval = grep_buffer(&opt->grep_filter, buf.buf, buf.len);\n+\telse\n+\t\tretval = grep_buffer(&opt->grep_filter,\n+\t\t\t\t commit->buffer, strlen(commit->buffer));\n+\tstrbuf_release(&buf);\n+\treturn retval;\n }\n \n static inline int want_ancestry(struct rev_info *revs)\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex 91db352..f42a605 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -546,6 +546,32 @@ test_expect_success 'log grep (6)' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'log grep (7)' '\n+\tgit log -g --grep-reflog=\"commit: third\" --pretty=tformat:%s >actual &&\n+\techo third >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'log grep (8)' '\n+\tgit log -g --grep-reflog=\"commit: third\" --grep-reflog=\"commit: second\" --pretty=tformat:%s >actual &&\n+\t{\n+\t\techo third && echo second\n+\t} >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'log grep (9)' '\n+\tgit log -g --grep-reflog=\"commit: third\" --author=\"Thor\" --pretty=tformat:%s >actual &&\n+\techo third >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'log grep (9)' '\n+\tgit log -g --grep-reflog=\"commit: third\" --author=\"non-existant\" --pretty=tformat:%s >actual &&\n+\t: >expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'log with multiple --grep uses union' '\n \tgit log --grep=i --grep=r --format=%s >actual &&\n \t{\n-- \n1.7.12.1.405.gb727dc9\n"},{"id":"200070","messageId":"1348815682-18696-3-git-send-email-pclouds@gmail.com","threadId":"31654","inReplyTo":"1348815682-18696-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 3/3] revision: make --grep search in notes too if shown","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-09-28T07:01:22Z","receivedAt":"2012-09-28T07:01:22Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Notes are shown after commit body. From user perspective it looks\npretty much like commit body and they may assume --grep would search\nin that part too. Make it so.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n On Fri, Sep 28, 2012 at 1:16 AM, Junio C Hamano \n wrote:\n > The output from \"log --show-notes\", on the other hand, is even more\n > conflated and a casual user would view it as part of the message,\n > so\n > I would imagine that if we ever do the extention to cover notes\n > data, the normal \"--grep\" should apply to it.\n\n Something like this?\n\n revision.c | 6 ++++++\n 1 file changed, 6 insertions(+)\n\ndiff --git a/revision.c b/revision.c\nindex cfa0e2e..febb4d7 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2223,6 +2223,12 @@ static int commit_match(struct commit *commit, struct rev_info *opt)\n \t\tstrbuf_addch(&buf, '\\n');\n \t\tstrbuf_addstr(&buf, commit->buffer);\n \t}\n+\tif (opt->show_notes) {\n+\t\tif (!buf.len)\n+\t\t\tstrbuf_addstr(&buf, commit->buffer);\n+\t\tformat_display_notes(commit->object.sha1, &buf,\n+\t\t\t\t get_log_output_encoding(), 0);\n+\t}\n \tif (buf.len)\n \t\tretval = grep_buffer(&opt->grep_filter, buf.buf, buf.len);\n \telse\n-- \n1.7.12.1.405.gb727dc9\n"},{"id":"200115","messageId":"7vr4pmm3qz.fsf@alter.siamese.dyndns.org","threadId":"31654","inReplyTo":"1348815682-18696-2-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 2/3] revision: add --grep-reflog to filter commits by reflog messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-28T17:55:16Z","receivedAt":"2012-09-28T17:55:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> Similar to --author/--committer which filters commits by author and\n> committer header fields. --grep-reflog adds a fake \"reflog\" header to\n> commit and a grep filter to search on that line.\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> On Fri, Sep 28, 2012 at 12:09 AM, Junio C Hamano \n> wrote:\n> > I am debating myself if it is sane for this option to have no hint\n> > that it is about \"limiting\" in its name. \"--author/--committer\"\n> > don't and it is clear from the context of the command that they are\n> > not about setting author/committer, so \"--reflog-message\" may be\n> > interpreted the same, perhaps.\n> >\n> > The entry in the context above talks about multiple occurrence of\n> > that option. Shouldn't this new one also say what happens when it\n> > is\n> > given twice?\n>\n> Fixed.\n>\n> > I think I wrote prep_header_patterns() and compile_grep_patterns()\n> > carefully enough not to assume the headers are only the author and\n> > committer names, so the various combinations i.e. all-match,\n> > author(s), committer(s), grep(s), and reflog-message(s), should\n> > work\n> > out of the box, but have you actually tested them?\n>\n> I did not. I do now, tests are also added.\n\nYou seem to have skipped a lot more important comment from the\nmessage that came immediately after the above part. Did you take a\nlook at grep.c::match_one_pattern() that calls strip_timestamp() for\nany and all GREP_PATTERN_HEAD items?\n\nThis is why I think that the [1/3] in this series shows a total lack\nof design taste. Each element in \"headers\", unlike \"body\", is a\nstructured field and its payload has meaning depending on what field\nname is associated with it. That is why we do not give a way to\nusers to willy-nilly add ad-hoc header fields.\n\nThe other side of this coin is that Git ought to be able to take\nadvantage of the meaning of each header. Stripping the timestamp\npart when doing a textual match against --author/--committer is one\nexample. The code in match_one_pattern() to deal with the header\npatterns need to be _extended_ to take into account the special\ntreatment each header, includign this new \"reflog \" fake header,\nwants when doing a textual match, and when we add any other match\nagainst header (either real or fake), we should be able to add\nspecific matching logic tailored to handle the particular meaning of\nthe header would have.\n\nFor that to happen, the code _must_ know what kind of headers we\nwould support; discarding the existing enum is going in a wrong\ndirection.\n\nWhen we introduce \"git log --header=frotz:xyzzy\" option that looks\nfor a generic \"frotz \" header and tries to see if \"xyzzy\" textually\nappears in the field, we may want to add a generic \"this is not a\nheader that we have special treatment for\" enum to the mix. But for\nknown kinds of headers, we would need a list of what each header is\nand what semantics it wants.\n\nSo please reconsider undoing [1/3], and rerolling [2/3] that depends\non it.\n"},{"id":"200116","messageId":"7vmx0am3pz.fsf@alter.siamese.dyndns.org","threadId":"31654","inReplyTo":"1348815682-18696-3-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 3/3] revision: make --grep search in notes too if shown","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-28T17:55:52Z","receivedAt":"2012-09-28T17:55:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> Notes are shown after commit body. From user perspective it looks\n> pretty much like commit body and they may assume --grep would search\n> in that part too. Make it so.\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> On Fri, Sep 28, 2012 at 1:16 AM, Junio C Hamano \n> wrote:\n> > The output from \"log --show-notes\", on the other hand, is even more\n> > conflated and a casual user would view it as part of the message,\n> > so\n> > I would imagine that if we ever do the extention to cover notes\n> > data, the normal \"--grep\" should apply to it.\n>\n> Something like this?\n\nYes, that was what I had in mind.\n\n> revision.c | 6 ++++++\n> 1 file changed, 6 insertions(+)\n>\n> diff --git a/revision.c b/revision.c\n> index cfa0e2e..febb4d7 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -2223,6 +2223,12 @@ static int commit_match(struct commit *commit, struct rev_info *opt)\n> \t\tstrbuf_addch(&buf, '\\n');\n> \t\tstrbuf_addstr(&buf, commit->buffer);\n> \t}\n> +\tif (opt->show_notes) {\n> +\t\tif (!buf.len)\n> +\t\t\tstrbuf_addstr(&buf, commit->buffer);\n> +\t\tformat_display_notes(commit->object.sha1, &buf,\n> +\t\t\t\t get_log_output_encoding(), 0);\n> +\t}\n> \tif (buf.len)\n> \t\tretval = grep_buffer(&opt->grep_filter, buf.buf, buf.len);\n> \telse\n"},{"id":"200145","messageId":"1348893689-20240-1-git-send-email-pclouds@gmail.com","threadId":"31654","inReplyTo":"7vr4pmm3qz.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/3] revision: add --grep-reflog to filter commits by reflog messages","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-09-29T04:41:26Z","receivedAt":"2012-09-29T04:41:26Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, Sep 29, 2012 at 12:55 AM, Junio C Hamano wrote:\n> For that to happen, the code _must_ know what kind of headers we\n> would support; discarding the existing enum is going in a wrong\n> direction.\n\nOr what kind of manipulation is required for a header. The caller can\ndecide if it wants such manipulation or not. Somebody might want to\ngrep committer's date, for example.\n\n> When we introduce \"git log --header=frotz:xyzzy\" option that looks\n> for a generic \"frotz \" header and tries to see if \"xyzzy\" textually\n> appears in the field, we may want to add a generic \"this is not a\n> header that we have special treatment for\" enum to the mix. But for\n> known kinds of headers, we would need a list of what each header is\n> and what semantics it wants.\n>\n> So please reconsider undoing [1/3], and rerolling [2/3] that depends\n> on it.\n\nDone. The enum is kept. I added a few tests about grepping timestamp\nin 1/3 to keep people (or myself) from making the same mistake I did.\n\n3/3 is reposted for completeness, I don't care much about notes (so\nfar). It's up to you to take or drop it.\n\nNguyễn Thái Ngọc Duy (3):\n grep: prepare for new header field filter\n revision: add --grep-reflog to filter commits by reflog messages\n revision: make --grep search in notes too if shown\n\n Documentation/rev-list-options.txt | 8 ++++++++\n grep.c | 10 +++++++++-\n grep.h | 7 +++++--\n revision.c | 26 ++++++++++++++++++++++++--\n t/t7810-grep.sh | 38 ++++++++++++++++++++++++++++++++++++++\n 5 files changed, 84 insertions(+), 5 deletions(-)\n\n-- \n1.7.12.1.406.g6ab07c4\n"},{"id":"200146","messageId":"1348893689-20240-2-git-send-email-pclouds@gmail.com","threadId":"31654","inReplyTo":"1348893689-20240-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 1/3] grep: prepare for new header field filter","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-09-29T04:41:27Z","receivedAt":"2012-09-29T04:41:27Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"grep supports only author and committer headers, which have the same\nspecial treatment that later headers may or may not have. Check for\nfield type and only strip_timestamp() when the field is either author\nor committer.\n\nGREP_HEADER_FIELD_MAX is put in the grep_header_field enum to be\ncalculated automatically, correctly, as long as it's at the end of the\nenum.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n grep.c | 9 ++++++++-\n grep.h | 6 ++++--\n t/t7810-grep.sh | 12 ++++++++++++\n 3 files changed, 24 insertions(+), 3 deletions(-)\n\ndiff --git a/grep.c b/grep.c\nindex 898be6e..8d73995 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -720,7 +720,14 @@ static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,\n \t\tif (strncmp(bol, field, len))\n \t\t\treturn 0;\n \t\tbol += len;\n-\t\tsaved_ch = strip_timestamp(bol, &eol);\n+\t\tswitch (p->field) {\n+\t\tcase GREP_HEADER_AUTHOR:\n+\t\tcase GREP_HEADER_COMMITTER:\n+\t\t\tsaved_ch = strip_timestamp(bol, &eol);\n+\t\t\tbreak;\n+\t\tdefault:\n+\t\t\tbreak;\n+\t\t}\n \t}\n \n again:\ndiff --git a/grep.h b/grep.h\nindex 8a28a67..d54adbe 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -29,9 +29,11 @@ enum grep_context {\n \n enum grep_header_field {\n \tGREP_HEADER_AUTHOR = 0,\n-\tGREP_HEADER_COMMITTER\n+\tGREP_HEADER_COMMITTER,\n+\n+\t/* Must be at the end of the enum */\n+\tGREP_HEADER_FIELD_MAX\n };\n-#define GREP_HEADER_FIELD_MAX (GREP_HEADER_COMMITTER + 1)\n \n struct grep_pat {\n \tstruct grep_pat *next;\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex 91db352..30eaa9a 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -628,6 +628,18 @@ test_expect_success 'log --all-match --grep --grep --author takes intersection'\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'log --author does not search in timestamp' '\n+\t: >expect &&\n+\tgit log --author=\"$GIT_AUTHOR_DATE\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'log --committer does not search in timestamp' '\n+\t: >expect &&\n+\tgit log --committer=\"$GIT_COMMITTER_DATE\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'grep with CE_VALID file' '\n \tgit update-index --assume-unchanged t/t &&\n \trm t/t &&\n-- \n1.7.12.1.406.g6ab07c4\n"},{"id":"200148","messageId":"1348893689-20240-3-git-send-email-pclouds@gmail.com","threadId":"31654","inReplyTo":"1348893689-20240-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 2/3] revision: add --grep-reflog to filter commits by reflog messages","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-09-29T04:41:28Z","receivedAt":"2012-09-29T04:41:28Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Similar to --author/--committer which filters commits by author and\ncommitter header fields. --grep-reflog adds a fake \"reflog\" header to\ncommit and a grep filter to search on that line.\n\nAll rules to --author/--committer apply except no timestamp stripping.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n Documentation/rev-list-options.txt | 8 ++++++++\n grep.c | 1 +\n grep.h | 1 +\n revision.c | 20 ++++++++++++++++++--\n t/t7810-grep.sh | 26 ++++++++++++++++++++++++++\n 5 files changed, 54 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex 1fc2a18..aa7cd9d 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -51,6 +51,14 @@ endif::git-rev-list[]\n \tcommits whose author matches any of the given patterns are\n \tchosen (similarly for multiple `--committer=`).\n \n+--grep-reflog=::\n+\n+\tLimit the commits output to ones with reflog entries that\n+\tmatch the specified pattern (regular expression). With\n+\tmore than one `--grep-reflog`, commits whose reflog message\n+\tmatches any of the given patterns are chosen. Ignored unless\n+\t`--walk-reflogs` is given.\n+\n --grep=::\n \n \tLimit the commits output to ones with log message that\ndiff --git a/grep.c b/grep.c\nindex 8d73995..d70dcdf 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -697,6 +697,7 @@ static struct {\n } header_field[] = {\n \t{ \"author \", 7 },\n \t{ \"committer \", 10 },\n+\t{ \"reflog \", 7 },\n };\n \n static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,\ndiff --git a/grep.h b/grep.h\nindex d54adbe..6e78b96 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -30,6 +30,7 @@ enum grep_context {\n enum grep_header_field {\n \tGREP_HEADER_AUTHOR = 0,\n \tGREP_HEADER_COMMITTER,\n+\tGREP_HEADER_REFLOG,\n \n \t/* Must be at the end of the enum */\n \tGREP_HEADER_FIELD_MAX\ndiff --git a/revision.c b/revision.c\nindex ae12e11..109bec1 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1595,6 +1595,9 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t} else if ((argcount = parse_long_opt(\"committer\", argv, &optarg))) {\n \t\tadd_header_grep(revs, GREP_HEADER_COMMITTER, optarg);\n \t\treturn argcount;\n+\t} else if ((argcount = parse_long_opt(\"grep-reflog\", argv, &optarg))) {\n+\t\tadd_header_grep(revs, GREP_HEADER_REFLOG, optarg);\n+\t\treturn argcount;\n \t} else if ((argcount = parse_long_opt(\"grep\", argv, &optarg))) {\n \t\tadd_message_grep(revs, optarg);\n \t\treturn argcount;\n@@ -2210,10 +2213,23 @@ static int rewrite_parents(struct rev_info *revs, struct commit *commit)\n \n static int commit_match(struct commit *commit, struct rev_info *opt)\n {\n+\tint retval;\n+\tstruct strbuf buf = STRBUF_INIT;\n \tif (!opt->grep_filter.pattern_list && !opt->grep_filter.header_list)\n \t\treturn 1;\n-\treturn grep_buffer(&opt->grep_filter,\n-\t\t\t commit->buffer, strlen(commit->buffer));\n+\tif (opt->reflog_info) {\n+\t\tstrbuf_addstr(&buf, \"reflog \");\n+\t\tget_reflog_message(&buf, opt->reflog_info);\n+\t\tstrbuf_addch(&buf, '\\n');\n+\t\tstrbuf_addstr(&buf, commit->buffer);\n+\t}\n+\tif (buf.len)\n+\t\tretval = grep_buffer(&opt->grep_filter, buf.buf, buf.len);\n+\telse\n+\t\tretval = grep_buffer(&opt->grep_filter,\n+\t\t\t\t commit->buffer, strlen(commit->buffer));\n+\tstrbuf_release(&buf);\n+\treturn retval;\n }\n \n static inline int want_ancestry(struct rev_info *revs)\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex 30eaa9a..3a5d0fd 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -546,6 +546,32 @@ test_expect_success 'log grep (6)' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'log grep (7)' '\n+\tgit log -g --grep-reflog=\"commit: third\" --pretty=tformat:%s >actual &&\n+\techo third >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'log grep (8)' '\n+\tgit log -g --grep-reflog=\"commit: third\" --grep-reflog=\"commit: second\" --pretty=tformat:%s >actual &&\n+\t{\n+\t\techo third && echo second\n+\t} >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'log grep (9)' '\n+\tgit log -g --grep-reflog=\"commit: third\" --author=\"Thor\" --pretty=tformat:%s >actual &&\n+\techo third >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'log grep (9)' '\n+\tgit log -g --grep-reflog=\"commit: third\" --author=\"non-existant\" --pretty=tformat:%s >actual &&\n+\t: >expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'log with multiple --grep uses union' '\n \tgit log --grep=i --grep=r --format=%s >actual &&\n \t{\n-- \n1.7.12.1.406.g6ab07c4\n"},{"id":"200147","messageId":"1348893689-20240-4-git-send-email-pclouds@gmail.com","threadId":"31654","inReplyTo":"1348893689-20240-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 3/3] revision: make --grep search in notes too if shown","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-09-29T04:41:29Z","receivedAt":"2012-09-29T04:41:29Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Notes are shown after commit body. From user perspective it looks\npretty much like commit body and they may assume --grep would search\nin that part too. Make it so.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n revision.c | 6 ++++++\n 1 file changed, 6 insertions(+)\n\ndiff --git a/revision.c b/revision.c\nindex 109bec1..dff6fb7 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2223,6 +2223,12 @@ static int commit_match(struct commit *commit, struct rev_info *opt)\n \t\tstrbuf_addch(&buf, '\\n');\n \t\tstrbuf_addstr(&buf, commit->buffer);\n \t}\n+\tif (opt->show_notes) {\n+\t\tif (!buf.len)\n+\t\t\tstrbuf_addstr(&buf, commit->buffer);\n+\t\tformat_display_notes(commit->object.sha1, &buf,\n+\t\t\t\t get_log_output_encoding(), 0);\n+\t}\n \tif (buf.len)\n \t\tretval = grep_buffer(&opt->grep_filter, buf.buf, buf.len);\n \telse\n-- \n1.7.12.1.406.g6ab07c4\n"},{"id":"200150","messageId":"20120929052202.GA3330@sigill.intra.peff.net","threadId":"31654","inReplyTo":"1348893689-20240-2-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 1/3] grep: prepare for new header field filter","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-09-29T05:22:02Z","receivedAt":"2012-09-29T05:22:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Sep 29, 2012 at 11:41:27AM +0700, Nguyen Thai Ngoc Duy wrote:\n\n> diff --git a/grep.c b/grep.c\n> index 898be6e..8d73995 100644\n> --- a/grep.c\n> +++ b/grep.c\n> @@ -720,7 +720,14 @@ static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,\n> \t\tif (strncmp(bol, field, len))\n> \t\t\treturn 0;\n> \t\tbol += len;\n> -\t\tsaved_ch = strip_timestamp(bol, &eol);\n> +\t\tswitch (p->field) {\n> +\t\tcase GREP_HEADER_AUTHOR:\n> +\t\tcase GREP_HEADER_COMMITTER:\n> +\t\t\tsaved_ch = strip_timestamp(bol, &eol);\n> +\t\t\tbreak;\n> +\t\tdefault:\n> +\t\t\tbreak;\n> +\t\t}\n\nReading this hunk, I wondered what happens to saved_ch if we do not set\nit here. Fortunately it is initialized to 0, as we already have to\nhandle the non-header case. Then later we do this, which does introduce\na new condition (saved_ch was not set, but we trigger the first half of\nthe conditional):\n\n if (p->token == GREP_PATTERN_HEAD && saved_ch)\n *eol = saved_ch;\n\nHowever, the second half of that conditional (which previously was only\ntriggered when we tried to split the timestamp, but there was a bogus\nline with no \">\" on it) prevents us from overwriting *eol.\n\nSo I think it is good, but it was non-obvious enough that I wanted to\nsave other reviewers from investigating it. The rest of the patch looks\ngood to me, as well.\n\n-Peff\n"},{"id":"200152","messageId":"20120929053013.GB3330@sigill.intra.peff.net","threadId":"31654","inReplyTo":"1348893689-20240-3-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 2/3] revision: add --grep-reflog to filter commits by reflog messages","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-09-29T05:30:13Z","receivedAt":"2012-09-29T05:30:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Sep 29, 2012 at 11:41:28AM +0700, Nguyen Thai Ngoc Duy wrote:\n\n> @@ -2210,10 +2213,23 @@ static int rewrite_parents(struct rev_info *revs, struct commit *commit)\n> \n> static int commit_match(struct commit *commit, struct rev_info *opt)\n> {\n> +\tint retval;\n> +\tstruct strbuf buf = STRBUF_INIT;\n> \tif (!opt->grep_filter.pattern_list && !opt->grep_filter.header_list)\n> \t\treturn 1;\n> -\treturn grep_buffer(&opt->grep_filter,\n> -\t\t\t commit->buffer, strlen(commit->buffer));\n> +\tif (opt->reflog_info) {\n> +\t\tstrbuf_addstr(&buf, \"reflog \");\n> +\t\tget_reflog_message(&buf, opt->reflog_info);\n> +\t\tstrbuf_addch(&buf, '\\n');\n> +\t\tstrbuf_addstr(&buf, commit->buffer);\n> +\t}\n> +\tif (buf.len)\n> +\t\tretval = grep_buffer(&opt->grep_filter, buf.buf, buf.len);\n> +\telse\n> +\t\tretval = grep_buffer(&opt->grep_filter,\n> +\t\t\t\t commit->buffer, strlen(commit->buffer));\n> +\tstrbuf_release(&buf);\n> +\treturn retval;\n\nI like how callers not doing a reflog walk do not have to pay the price\nto do the extra allocating. We could further limit it to only when\n--grep-reflog is in effect, but I guess that would mean wading through\ngrep_filter's patterns, since it could be buried amidst ANDs and ORs?\n\nOne alternative would be to set a bit in the grep_opt when we call\nappend_header_grep_pattern. It feels a bit like a layering violation,\nthough. I guess the bit could also go into rev_info. It may not even be\na measurable slowdown, though. Premature optimization and all that.\n\nOther than that, I didn't notice anything wrong in the patch.\n\n-Peff\n"},{"id":"200153","messageId":"7v4nmhjsqr.fsf@alter.siamese.dyndns.org","threadId":"31654","inReplyTo":"1348893689-20240-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 2/3] revision: add --grep-reflog to filter commits by reflog messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-29T05:35:56Z","receivedAt":"2012-09-29T05:35:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> On Sat, Sep 29, 2012 at 12:55 AM, Junio C Hamano wrote:\n>> For that to happen, the code _must_ know what kind of headers we\n>> would support; discarding the existing enum is going in a wrong\n>> direction.\n>\n> Or what kind of manipulation is required for a header. The caller can\n> decide if it wants such manipulation or not. Somebody might want to\n> grep committer's date, for example.\n\nYes, if we wanted to filter log outputs to commits authored only on\nThursdays, we would introduce --author-date=\" language in the GREP_PATTERN_HEAD\npart of match_one_pattern() and use the p->field to select that\nlogic. At that point, we may be better off introducing a helper\nfunction to split the more complex logic for GREP_PATTERN_HEAD out\nof the match_one_pattern() function.\n"},{"id":"200157","messageId":"7vpq55idbe.fsf@alter.siamese.dyndns.org","threadId":"31654","inReplyTo":"20120929053013.GB3330@sigill.intra.peff.net","subject":"Re: [PATCH 2/3] revision: add --grep-reflog to filter commits by reflog messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-29T05:54:29Z","receivedAt":"2012-09-29T05:54:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> +\tif (opt->reflog_info) {\n>> +\t\tstrbuf_addstr(&buf, \"reflog \");\n>> +\t\tget_reflog_message(&buf, opt->reflog_info);\n>> +\t\tstrbuf_addch(&buf, '\\n');\n>> +\t\tstrbuf_addstr(&buf, commit->buffer);\n>> +\t}\n>> +\tif (buf.len)\n>> +\t\tretval = grep_buffer(&opt->grep_filter, buf.buf, buf.len);\n>> +\telse\n>> +\t\tretval = grep_buffer(&opt->grep_filter,\n>> +\t\t\t\t commit->buffer, strlen(commit->buffer));\n>> +\tstrbuf_release(&buf);\n>> +\treturn retval;\n>\n> I like how callers not doing a reflog walk do not have to pay the price\n> to do the extra allocating. We could further limit it to only when\n> --grep-reflog is in effect, but I guess that would mean wading through\n> grep_filter's patterns, since it could be buried amidst ANDs and ORs?\n>\n> One alternative would be to set a bit in the grep_opt when we call\n> append_header_grep_pattern. It feels a bit like a layering violation,\n> though. I guess the bit could also go into rev_info. It may not even be\n> a measurable slowdown, though. Premature optimization and all that.\n\nI do not think it is a layering violation. compile_grep_exp()\nshould be aware of the short-cut possibilities and your \"our\nexpression is interested in reflog so we need to read it\" is very\nsimilar in spirit to the existing opt->extended bit.\n\nIt will obviously allow us to avoid reading reflog information\nunnecessarily here. I think it makes perfect sense.\n\nWe may also want to flag the use of the --grep-reflog option when\nthe --walk-reflogs option is not in effect in setup_revisions() as\nan error, or something.\n"},{"id":"200158","messageId":"CACsJy8A1UroqCezJFjqOqBQg+puX=jc1Q-CMSB4f=CHULY=OyA@mail.gmail.com","threadId":"31654","inReplyTo":"7vpq55idbe.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/3] revision: add --grep-reflog to filter commits by reflog messages","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-09-29T06:13:32Z","receivedAt":"2012-09-29T06:13:32Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, Sep 29, 2012 at 12:54 PM, Junio C Hamano wrote:\n>> I like how callers not doing a reflog walk do not have to pay the price\n>> to do the extra allocating. We could further limit it to only when\n>> --grep-reflog is in effect, but I guess that would mean wading through\n>> grep_filter's patterns, since it could be buried amidst ANDs and ORs?\n>>\n>> One alternative would be to set a bit in the grep_opt when we call\n>> append_header_grep_pattern. It feels a bit like a layering violation,\n>> though. I guess the bit could also go into rev_info. It may not even be\n>> a measurable slowdown, though. Premature optimization and all that.\n>\n> I do not think it is a layering violation. compile_grep_exp()\n> should be aware of the short-cut possibilities and your \"our\n> expression is interested in reflog so we need to read it\" is very\n> similar in spirit to the existing opt->extended bit.\n>\n> It will obviously allow us to avoid reading reflog information\n> unnecessarily here. I think it makes perfect sense.\n\nreflog, in terms of both the number of commits and message length, is\nusually short enough that slowdown does not really show, especially\nwhen used with git-log, an interactive command.\n\nWithout the changes:\n\n$ time git log -g --grep . >/dev/null\n\nreal 0m0.480s\nuser 0m0.451s\nsys 0m0.025s\n\nWith the changes:\n\n$ time ./git log -g --grep . >/dev/null\n\nreal 0m0.490s\nuser 0m0.471s\nsys 0m0.018s\n\n> We may also want to flag the use of the --grep-reflog option when\n> the --walk-reflogs option is not in effect in setup_revisions() as\n> an error, or something.\n\nThat's why I put \"Ignored unless --walk-reflogs is given\" in the\ndocument. But an error would be fine too. I suppose an error is\npreferable in case users do not read document carefully?\n-- \nDuy\n"},{"id":"200167","messageId":"20120929061641.GA4425@sigill.intra.peff.net","threadId":"31654","inReplyTo":"7vpq55idbe.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/3] revision: add --grep-reflog to filter commits by reflog messages","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-09-29T06:16:41Z","receivedAt":"2012-09-29T06:16:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Sep 28, 2012 at 10:54:29PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> >> +\tif (opt->reflog_info) {\n> >> +\t\tstrbuf_addstr(&buf, \"reflog \");\n> >> +\t\tget_reflog_message(&buf, opt->reflog_info);\n> >> +\t\tstrbuf_addch(&buf, '\\n');\n> >> +\t\tstrbuf_addstr(&buf, commit->buffer);\n> >> +\t}\n> >> +\tif (buf.len)\n> >> +\t\tretval = grep_buffer(&opt->grep_filter, buf.buf, buf.len);\n> >> +\telse\n> >> +\t\tretval = grep_buffer(&opt->grep_filter,\n> >> +\t\t\t\t commit->buffer, strlen(commit->buffer));\n> >> +\tstrbuf_release(&buf);\n> >> +\treturn retval;\n> >\n> > I like how callers not doing a reflog walk do not have to pay the price\n> > to do the extra allocating. We could further limit it to only when\n> > --grep-reflog is in effect, but I guess that would mean wading through\n> > grep_filter's patterns, since it could be buried amidst ANDs and ORs?\n> >\n> > One alternative would be to set a bit in the grep_opt when we call\n> > append_header_grep_pattern. It feels a bit like a layering violation,\n> > though. I guess the bit could also go into rev_info. It may not even be\n> > a measurable slowdown, though. Premature optimization and all that.\n> \n> I do not think it is a layering violation. compile_grep_exp()\n> should be aware of the short-cut possibilities and your \"our\n> expression is interested in reflog so we need to read it\" is very\n> similar in spirit to the existing opt->extended bit.\n\nHmm. Yeah, I guess so. I was thinking that the grep code did not know\nthere was a commit or reflog involved at all (we just pass it a buffer,\nand how we prepare it is our business), but it does already know about\nthe magic GREP_HEADER_* variables, and this is definitely part of that.\n\n> We may also want to flag the use of the --grep-reflog option when\n> the --walk-reflogs option is not in effect in setup_revisions() as\n> an error, or something.\n\nGood point. I think the docs in the patch just say it is ignored unless\nwalking, but it would be better to flag the error.\n\n-Peff\n"},{"id":"200182","messageId":"7vbogoiqzs.fsf@alter.siamese.dyndns.org","threadId":"31654","inReplyTo":"CACsJy8A1UroqCezJFjqOqBQg+puX=jc1Q-CMSB4f=CHULY=OyA@mail.gmail.com","subject":"Re: [PATCH 2/3] revision: add --grep-reflog to filter commits by reflog messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-29T19:11:19Z","receivedAt":"2012-09-29T19:11:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyen Thai Ngoc Duy writes:\n\n> reflog, in terms of both the number of commits and message length, is\n> usually short enough that slowdown does not really show, especially\n> when used with git-log, an interactive command.\n\nYou shouldn't do things you can easily tell you do not need to,\nespecially when the effort to avoid doing unnecessary allocation,\ncopying and deallocation is not excessively larger than the saving.\n\nI personally think that lack of perceived performance impact is not\nan excuse to be sloppy. These things tend to add up.\n\n> That's why I put \"Ignored unless --walk-reflogs is given\" in the\n> document. But an error would be fine too. I suppose an error is\n> preferable in case users do not read document carefully?\n\n\nA reader who reads documentation carefully will spot that it is a\nsloppy coding that such a nonsense combination of two options are\nnot caught and diagnosed as an error, when accepting and silently\nignoring the option would not help anybody.\n\nAn alternative may be to turn -g on when --grep-reflog is given.\nStarting from a version that forbids --grep-reflog without -g will\nlet us change the command line parser to do so in the future without\nbackward incompatibility, but you cannot do so if you start from a\nversion that accepts and silently ignores.\n\nHow about squashing this in? I've future-proofed commit_match() a\nbit while at it; it would be cleaner to add new fake headers if the\nfunction is done this way.\n\n Documentation/rev-list-options.txt | 4 ++--\n grep.c | 2 ++\n grep.h | 1 +\n revision.c | 13 +++++++++++--\n t/t7810-grep.sh | 4 ++++\n 5 files changed, 20 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex aa7cd9d..ca22106 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -56,8 +56,8 @@ endif::git-rev-list[]\n \tLimit the commits output to ones with reflog entries that\n \tmatch the specified pattern (regular expression). With\n \tmore than one `--grep-reflog`, commits whose reflog message\n-\tmatches any of the given patterns are chosen. Ignored unless\n-\t`--walk-reflogs` is given.\n+\tmatches any of the given patterns are chosen. It is an\n+\terror to use this option unless `--walk-reflogs` is in use.\n \n --grep=::\n \ndiff --git a/grep.c b/grep.c\nindex d70dcdf..edc7776 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -64,6 +64,8 @@ void append_header_grep_pattern(struct grep_opt *opt,\n {\n \tstruct grep_pat *p = create_grep_pat(pat, strlen(pat), \"header\", 0,\n \t\t\t\t\t GREP_PATTERN_HEAD, field);\n+\tif (field == GREP_HEADER_REFLOG)\n+\t\topt->use_reflog_filter = 1;\n \tdo_append_grep_pat(&opt->header_tail, p);\n }\n \ndiff --git a/grep.h b/grep.h\nindex 6e78b96..c256ac6 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -107,6 +107,7 @@ struct grep_opt {\n #define GREP_BINARY_TEXT\t2\n \tint binary;\n \tint extended;\n+\tint use_reflog_filter;\n \tint pcre;\n \tint relative;\n \tint pathname;\ndiff --git a/revision.c b/revision.c\nindex 109bec1..9ad72df 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1908,6 +1908,8 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \n \tif (revs->reflog_info && revs->graph)\n \t\tdie(\"cannot combine --walk-reflogs with --graph\");\n+\tif (!revs->reflog_info && revs->grep_filter.use_reflog_filter)\n+\t\tdie(\"cannot use --grep-reflog without --walk-reflogs\");\n \n \treturn left;\n }\n@@ -2217,12 +2219,19 @@ static int commit_match(struct commit *commit, struct rev_info *opt)\n \tstruct strbuf buf = STRBUF_INIT;\n \tif (!opt->grep_filter.pattern_list && !opt->grep_filter.header_list)\n \t\treturn 1;\n-\tif (opt->reflog_info) {\n+\n+\t/* Prepend \"fake\" headers as needed */\n+\tif (opt->grep_filter.use_reflog_filter) {\n \t\tstrbuf_addstr(&buf, \"reflog \");\n \t\tget_reflog_message(&buf, opt->reflog_info);\n \t\tstrbuf_addch(&buf, '\\n');\n-\t\tstrbuf_addstr(&buf, commit->buffer);\n \t}\n+\n+\t/* Copy the commit to temporary if we are using \"fake\" headers */\n+\tif (buf.len)\n+\t\tstrbuf_addstr(&buf, commit->buffer);\n+\n+\t/* Find either in the commit object, or in the temporary */\n \tif (buf.len)\n \t\tretval = grep_buffer(&opt->grep_filter, buf.buf, buf.len);\n \telse\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex 3a5d0fd..f698001 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -572,6 +572,10 @@ test_expect_success 'log grep (9)' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'log --grep-reflog can only be used under -g' '\n+\ttest_must_fail git log --grep-reflog=\"commit: third\"\n+'\n+\n test_expect_success 'log with multiple --grep uses union' '\n \tgit log --grep=i --grep=r --format=%s >actual &&\n \t{\n-- \n1.7.12.1.484.g1764bc0\n"},{"id":"200189","messageId":"CACsJy8DC=rpuFQbKG9MUh3Aknf6zxJJpww+XoCYki5tu3SDF4w@mail.gmail.com","threadId":"31654","inReplyTo":"7vbogoiqzs.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/3] revision: add --grep-reflog to filter commits by reflog messages","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-09-30T03:45:30Z","receivedAt":"2012-09-30T03:45:30Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, Sep 30, 2012 at 2:11 AM, Junio C Hamano wrote:\n> How about squashing this in? I've future-proofed commit_match() a\n> bit while at it; it would be cleaner to add new fake headers if the\n> function is done this way.\n\nSure. It looks good to me.\n\n> --- a/Documentation/rev-list-options.txt\n> +++ b/Documentation/rev-list-options.txt\n> @@ -56,8 +56,8 @@ endif::git-rev-list[]\n> Limit the commits output to ones with reflog entries that\n> match the specified pattern (regular expression). With\n> more than one `--grep-reflog`, commits whose reflog message\n> - matches any of the given patterns are chosen. Ignored unless\n> - `--walk-reflogs` is given.\n> + matches any of the given patterns are chosen. It is an\n> + error to use this option unless `--walk-reflogs` is in use.\n\nWe could even drop the part about --walk-reflogs. Users will soon\nfigure it out when they try it.\n-- \nDuy\n"}]}