{"thread":{"id":"49500","subject":"git log -S or -G","startedAt":"2018-10-06T15:14:24Z","lastAt":"2018-10-09T13:51:26Z","messageCount":11,"participants":["Julia Lawall","Ævar Arnfjörð Bjarmason","Junio C Hamano","Jeff King","Jacob Keller"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"359768","messageId":"alpine.DEB.2.21.1810061712260.2402@hadrien","threadId":"49500","inReplyTo":null,"subject":"git log -S or -G","fromName":"Julia Lawall","fromEmail":"julia.lawall@lip6.fr","sentAt":"2018-10-06T15:14:22Z","receivedAt":"2018-10-06T15:14:24Z","isPatch":false,"sender":{"key":"julia.lawall@lip6.fr","avatar":null},"body":"Hello,\n\nGit log -S or -G make it possible to find commits that have particular\nwords in the changed lines. Sometimes it would be helpful to search for\nwords in the removed lines or in the added lines specifically. From the\nimplementation, I had the impression that this would be easy to implement.\nThe main question would be how to allow the user to specify what is\nwanted.\n\nthanks,\njulia\n"},{"id":"359770","messageId":"CACBZZX6PmG=-8563eYE4z98yvHePenZf_Kz1xgpse0ngjB5QyA@mail.gmail.com","threadId":"49500","inReplyTo":"alpine.DEB.2.21.1810061712260.2402@hadrien","subject":"Re: git log -S or -G","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-06T16:22:57Z","receivedAt":"2018-10-06T16:23:10Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Sat, Oct 6, 2018 at 5:16 PM Julia Lawall wrote:\n> Git log -S or -G make it possible to find commits that have particular\n> words in the changed lines. Sometimes it would be helpful to search for\n> words in the removed lines or in the added lines specifically. From the\n> implementation, I had the impression that this would be easy to implement.\n> The main question would be how to allow the user to specify what is\n> wanted.\n\nAs far as I know this isn't possible. The --diff-filter option is\nsimilar in spirit, but e.g. adding \"foo\" and then removing it from an\nexisting file will both be covered under --diff-filter=M, so that\nisn't what you're looking for.\n"},{"id":"359783","messageId":"xmqqd0smvay0.fsf@gitster-ct.c.googlers.com","threadId":"49500","inReplyTo":"CACBZZX6PmG=-8563eYE4z98yvHePenZf_Kz1xgpse0ngjB5QyA@mail.gmail.com","subject":"Re: git log -S or -G","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-07T00:48:23Z","receivedAt":"2018-10-07T00:48:29Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> On Sat, Oct 6, 2018 at 5:16 PM Julia Lawall wrote:\n>> Git log -S or -G make it possible to find commits that have particular\n>> words in the changed lines. Sometimes it would be helpful to search for\n>> words in the removed lines or in the added lines specifically. From the\n>> implementation, I had the impression that this would be easy to implement.\n>> The main question would be how to allow the user to specify what is\n>> wanted.\n>\n> As far as I know this isn't possible. The --diff-filter option is\n> similar in spirit, but e.g. adding \"foo\" and then removing it from an\n> existing file will both be covered under --diff-filter=M, so that\n> isn't what you're looking for.\n\nI agree with Julia that UI to the feature is harder than the\nmachinery to implement the feature to add \"I am interested in seeing\na patch that contains a hunk that adds 'foo' but am not interested\nin removal\" (or vice versa) for -G. You tweak\ndiffcore-pickaxe.c::diffgrep_consume() and you'are done.\n\nDoing the same for -S is much harder at the machinery level, as it\nperforms its thing without internally running \"diff\" twice, but just\ncounts the number of occurrences of 'foo'---that is sufficient for\nits intended use, and more efficient.\n\n"},{"id":"359787","messageId":"alpine.DEB.2.21.1810070719200.2347@hadrien","threadId":"49500","inReplyTo":"xmqqd0smvay0.fsf@gitster-ct.c.googlers.com","subject":"Re: git log -S or -G","fromName":"Julia Lawall","fromEmail":"julia.lawall@lip6.fr","sentAt":"2018-10-07T05:21:26Z","receivedAt":"2018-10-07T05:21:31Z","isPatch":false,"sender":{"key":"julia.lawall@lip6.fr","avatar":null},"body":"\n\nOn Sun, 7 Oct 2018, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n> > On Sat, Oct 6, 2018 at 5:16 PM Julia Lawall wrote:\n> >> Git log -S or -G make it possible to find commits that have particular\n> >> words in the changed lines. Sometimes it would be helpful to search for\n> >> words in the removed lines or in the added lines specifically. From the\n> >> implementation, I had the impression that this would be easy to implement.\n> >> The main question would be how to allow the user to specify what is\n> >> wanted.\n> >\n> > As far as I know this isn't possible. The --diff-filter option is\n> > similar in spirit, but e.g. adding \"foo\" and then removing it from an\n> > existing file will both be covered under --diff-filter=M, so that\n> > isn't what you're looking for.\n>\n> I agree with Julia that UI to the feature is harder than the\n> machinery to implement the feature to add \"I am interested in seeing\n> a patch that contains a hunk that adds 'foo' but am not interested\n> in removal\" (or vice versa) for -G. You tweak\n> diffcore-pickaxe.c::diffgrep_consume() and you'are done.\n>\n> Doing the same for -S is much harder at the machinery level, as it\n> performs its thing without internally running \"diff\" twice, but just\n> counts the number of occurrences of 'foo'---that is sufficient for\n> its intended use, and more efficient.\n\nThere is still the question of whether the number of occurrences of foo\ndecreases or increases.\n\njulia"},{"id":"359882","messageId":"xmqq8t38t4r7.fsf@gitster-ct.c.googlers.com","threadId":"49500","inReplyTo":"alpine.DEB.2.21.1810070719200.2347@hadrien","subject":"Re: git log -S or -G","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-08T23:09:32Z","receivedAt":"2018-10-08T23:09:38Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Julia Lawall writes:\n\n>> Doing the same for -S is much harder at the machinery level, as it\n>> performs its thing without internally running \"diff\" twice, but just\n>> counts the number of occurrences of 'foo'---that is sufficient for\n>> its intended use, and more efficient.\n>\n> There is still the question of whether the number of occurrences of foo\n> decreases or increases.\n\nHmph, taking the changes that makes the number of hits decrease\nwould catch a subset of \"changes that removes 'foo' only---I am not\ninterested in the ones that adds 'foo'\". It will avoid getting\nconfused by a change that moves an existing 'foo' to another place\nin the same file (as the number of hits does not change), but at the\nsame time, it will miss a change that genuinely removes an existing\n'foo' and happens to add a 'foo' at a different place in the same\nfile that is unrelated to the original 'foo'. Depending on the\ndefinition of \"I am only interested in removed ones\", that may or\nmay not be acceptable.\n\n\n\n"},{"id":"359893","messageId":"20181009032124.GE6250@sigill.intra.peff.net","threadId":"49500","inReplyTo":"xmqq8t38t4r7.fsf@gitster-ct.c.googlers.com","subject":"Re: git log -S or -G","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-09T03:21:24Z","receivedAt":"2018-10-09T03:21:28Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 09, 2018 at 08:09:32AM +0900, Junio C Hamano wrote:\n\n> Julia Lawall writes:\n> \n> >> Doing the same for -S is much harder at the machinery level, as it\n> >> performs its thing without internally running \"diff\" twice, but just\n> >> counts the number of occurrences of 'foo'---that is sufficient for\n> >> its intended use, and more efficient.\n> >\n> > There is still the question of whether the number of occurrences of foo\n> > decreases or increases.\n> \n> Hmph, taking the changes that makes the number of hits decrease\n> would catch a subset of \"changes that removes 'foo' only---I am not\n> interested in the ones that adds 'foo'\". It will avoid getting\n> confused by a change that moves an existing 'foo' to another place\n> in the same file (as the number of hits does not change), but at the\n> same time, it will miss a change that genuinely removes an existing\n> 'foo' and happens to add a 'foo' at a different place in the same\n> file that is unrelated to the original 'foo'. Depending on the\n> definition of \"I am only interested in removed ones\", that may or\n> may not be acceptable.\n\nI think that is the best we could do for \"-S\", though, which is\ninherently about counting hits.\n\nFor \"-G\", we are literally grepping the diff. It does not seem\nunreasonable to add the ability to grep only \"-\" or \"+\" lines, and the\ninterface for that should be pretty straightforward (a tri-state flag to\nlook in remove, added, or both lines).\n\n-Peff\n"},{"id":"359896","messageId":"CA+P7+xpnVeWrW5r6uj4E4NSFPjhA_f0iwaCTJb8-WFqZChHEvA@mail.gmail.com","threadId":"49500","inReplyTo":"20181009032124.GE6250@sigill.intra.peff.net","subject":"Re: git log -S or -G","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2018-10-09T03:58:56Z","receivedAt":"2018-10-09T03:59:11Z","isPatch":false,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Mon, Oct 8, 2018 at 8:22 PM Jeff King wrote:\n>\n> On Tue, Oct 09, 2018 at 08:09:32AM +0900, Junio C Hamano wrote:\n>\n> > Julia Lawall writes:\n> >\n> > >> Doing the same for -S is much harder at the machinery level, as it\n> > >> performs its thing without internally running \"diff\" twice, but just\n> > >> counts the number of occurrences of 'foo'---that is sufficient for\n> > >> its intended use, and more efficient.\n> > >\n> > > There is still the question of whether the number of occurrences of foo\n> > > decreases or increases.\n> >\n> > Hmph, taking the changes that makes the number of hits decrease\n> > would catch a subset of \"changes that removes 'foo' only---I am not\n> > interested in the ones that adds 'foo'\". It will avoid getting\n> > confused by a change that moves an existing 'foo' to another place\n> > in the same file (as the number of hits does not change), but at the\n> > same time, it will miss a change that genuinely removes an existing\n> > 'foo' and happens to add a 'foo' at a different place in the same\n> > file that is unrelated to the original 'foo'. Depending on the\n> > definition of \"I am only interested in removed ones\", that may or\n> > may not be acceptable.\n>\n> I think that is the best we could do for \"-S\", though, which is\n> inherently about counting hits.\n>\n> For \"-G\", we are literally grepping the diff. It does not seem\n> unreasonable to add the ability to grep only \"-\" or \"+\" lines, and the\n> interface for that should be pretty straightforward (a tri-state flag to\n> look in remove, added, or both lines).\n>\n> -Peff\n\nYea. I know I've wanted something like this in the past.\n\nThanks,\nJake\n"},{"id":"359899","messageId":"xmqqwoqrr8y2.fsf@gitster-ct.c.googlers.com","threadId":"49500","inReplyTo":"20181009032124.GE6250@sigill.intra.peff.net","subject":"Re: git log -S or -G","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-09T05:21:57Z","receivedAt":"2018-10-09T05:22:04Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I think that is the best we could do for \"-S\", though, which is\n> inherently about counting hits.\n>\n> For \"-G\", we are literally grepping the diff. It does not seem\n> unreasonable to add the ability to grep only \"-\" or \"+\" lines, and the\n> interface for that should be pretty straightforward (a tri-state flag to\n> look in remove, added, or both lines).\n\nYeah, here is a lunchtime hack that hasn't even been compile tested.\n\n diff.c | 4 ++++\n diff.h | 2 ++\n diffcore-pickaxe.c | 22 ++++++++++++++++++++--\n 3 files changed, 26 insertions(+), 2 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex f0c7557b40..d1f2780844 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -5068,6 +5068,10 @@ int diff_opt_parse(struct diff_options *options,\n \t}\n \telse if (!strcmp(arg, \"--pickaxe-all\"))\n \t\toptions->pickaxe_opts |= DIFF_PICKAXE_ALL;\n+\telse if (!strcmp(arg, \"--pickaxe-ignore-add\"))\n+\t\toptions->pickaxe_opts |= DIFF_PICKAXE_IGNORE_ADD;\n+\telse if (!strcmp(arg, \"--pickaxe-ignore-del\"))\n+\t\toptions->pickaxe_opts |= DIFF_PICKAXE_IGNORE_DEL;\n \telse if (!strcmp(arg, \"--pickaxe-regex\"))\n \t\toptions->pickaxe_opts |= DIFF_PICKAXE_REGEX;\n \telse if ((argcount = short_opt('O', av, &optarg))) {\ndiff --git a/diff.h b/diff.h\nindex a30cc35ec3..147c47ace7 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -358,6 +358,8 @@ int git_config_rename(const char *var, const char *value);\n \t\t\t\t DIFF_PICKAXE_KIND_OBJFIND)\n \n #define DIFF_PICKAXE_IGNORE_CASE\t32\n+#define DIFF_PICKAXE_IGNORE_ADD\t64\n+#define DIFF_PICKAXE_IGNORE_DEL 128\n \n void diffcore_std(struct diff_options *);\n void diffcore_fix_diff_index(struct diff_options *);\ndiff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c\nindex 800a899c86..826dde6bd4 100644\n--- a/diffcore-pickaxe.c\n+++ b/diffcore-pickaxe.c\n@@ -16,6 +16,7 @@ typedef int (*pickaxe_fn)(mmfile_t *one, mmfile_t *two,\n \n struct diffgrep_cb {\n \tregex_t *regexp;\n+\tstruct diff_options *diff_options;\n \tint hit;\n };\n \n@@ -23,9 +24,14 @@ static void diffgrep_consume(void *priv, char *line, unsigned long len)\n {\n \tstruct diffgrep_cb *data = priv;\n \tregmatch_t regmatch;\n+\tunsigned pickaxe_opts = data->diff_options->pickaxe_opts;\n \n \tif (line[0] != '+' && line[0] != '-')\n \t\treturn;\n+\tif ((pickaxe_opts & DIFF_PICKAXE_IGNORE_ADD) &&\tline[0] == '+')\n+\t\treturn;\n+\tif ((pickaxe_opts & DIFF_PICKAXE_IGNORE_DEL) &&\tline[0] == '-')\n+\t\treturn;\n \tif (data->hit)\n \t\t/*\n \t\t * NEEDSWORK: we should have a way to terminate the\n@@ -45,13 +51,20 @@ static int diff_grep(mmfile_t *one, mmfile_t *two,\n \txpparam_t xpp;\n \txdemitconf_t xecfg;\n \n-\tif (!one)\n+\tif (!one) {\n+\t\tif (o->pickaxe_opts & DIFF_PICKAXE_IGNORE_ADD)\n+\t\t\treturn 0;\n \t\treturn !regexec_buf(regexp, two->ptr, two->size,\n \t\t\t\t 1, ®match, 0);\n-\tif (!two)\n+\t}\n+\tif (!two) {\n+\t\tif (o->pickaxe_opts & DIFF_PICKAXE_IGNORE_DEL)\n+\t\t\treturn 0;\n \t\treturn !regexec_buf(regexp, one->ptr, one->size,\n \t\t\t\t 1, ®match, 0);\n+\t}\n \n+\tecbdata.diff_options = o;\n \t/*\n \t * We have both sides; need to run textual diff and see if\n \t * the pattern appears on added/deleted lines.\n@@ -113,6 +126,11 @@ static int has_changes(mmfile_t *one, mmfile_t *two,\n {\n \tunsigned int one_contains = one ? contains(one, regexp, kws) : 0;\n \tunsigned int two_contains = two ? contains(two, regexp, kws) : 0;\n+\n+\tif (o->pickaxe_opts & DIFF_PICKAXE_IGNORE_ADD)\n+\t\treturn one_contains > two_contains;\n+\tif (o->pickaxe_opts & DIFF_PICKAXE_IGNORE_DEL)\n+\t\treturn one_contains < two_contains;\n \treturn one_contains != two_contains;\n }\n \n"},{"id":"359903","messageId":"alpine.DEB.2.21.1810090837270.2430@hadrien","threadId":"49500","inReplyTo":"CA+P7+xpnVeWrW5r6uj4E4NSFPjhA_f0iwaCTJb8-WFqZChHEvA@mail.gmail.com","subject":"Re: git log -S or -G","fromName":"Julia Lawall","fromEmail":"julia.lawall@lip6.fr","sentAt":"2018-10-09T06:39:15Z","receivedAt":"2018-10-09T06:39:21Z","isPatch":false,"sender":{"key":"julia.lawall@lip6.fr","avatar":null},"body":"\n\nOn Mon, 8 Oct 2018, Jacob Keller wrote:\n\n> On Mon, Oct 8, 2018 at 8:22 PM Jeff King wrote:\n> >\n> > On Tue, Oct 09, 2018 at 08:09:32AM +0900, Junio C Hamano wrote:\n> >\n> > > Julia Lawall writes:\n> > >\n> > > >> Doing the same for -S is much harder at the machinery level, as it\n> > > >> performs its thing without internally running \"diff\" twice, but just\n> > > >> counts the number of occurrences of 'foo'---that is sufficient for\n> > > >> its intended use, and more efficient.\n> > > >\n> > > > There is still the question of whether the number of occurrences of foo\n> > > > decreases or increases.\n> > >\n> > > Hmph, taking the changes that makes the number of hits decrease\n> > > would catch a subset of \"changes that removes 'foo' only---I am not\n> > > interested in the ones that adds 'foo'\". It will avoid getting\n> > > confused by a change that moves an existing 'foo' to another place\n> > > in the same file (as the number of hits does not change), but at the\n> > > same time, it will miss a change that genuinely removes an existing\n> > > 'foo' and happens to add a 'foo' at a different place in the same\n> > > file that is unrelated to the original 'foo'. Depending on the\n> > > definition of \"I am only interested in removed ones\", that may or\n> > > may not be acceptable.\n> >\n> > I think that is the best we could do for \"-S\", though, which is\n> > inherently about counting hits.\n> >\n> > For \"-G\", we are literally grepping the diff. It does not seem\n> > unreasonable to add the ability to grep only \"-\" or \"+\" lines, and the\n> > interface for that should be pretty straightforward (a tri-state flag to\n> > look in remove, added, or both lines).\n> >\n> > -Peff\n>\n> Yea. I know I've wanted something like this in the past.\n\nIt could also be nice to be able to specify multiple patterns, with and\nand or between them. So -A&-B would be remove A somewhere and remove B\nsomewhere.\n\njulia\n"},{"id":"359919","messageId":"87lg77cmr1.fsf@evledraar.gmail.com","threadId":"49500","inReplyTo":"xmqqwoqrr8y2.fsf@gitster-ct.c.googlers.com","subject":"Re: git log -S or -G","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-09T12:45:06Z","receivedAt":"2018-10-09T12:45:12Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Oct 09 2018, Junio C Hamano wrote:\n\n> Jeff King writes:\n>\n>> I think that is the best we could do for \"-S\", though, which is\n>> inherently about counting hits.\n>>\n>> For \"-G\", we are literally grepping the diff. It does not seem\n>> unreasonable to add the ability to grep only \"-\" or \"+\" lines, and the\n>> interface for that should be pretty straightforward (a tri-state flag to\n>> look in remove, added, or both lines).\n>\n> Yeah, here is a lunchtime hack that hasn't even been compile tested.\n>\n> diff.c | 4 ++++\n> diff.h | 2 ++\n> diffcore-pickaxe.c | 22 ++++++++++++++++++++--\n> 3 files changed, 26 insertions(+), 2 deletions(-)\n>\n> diff --git a/diff.c b/diff.c\n> index f0c7557b40..d1f2780844 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -5068,6 +5068,10 @@ int diff_opt_parse(struct diff_options *options,\n> \t}\n> \telse if (!strcmp(arg, \"--pickaxe-all\"))\n> \t\toptions->pickaxe_opts |= DIFF_PICKAXE_ALL;\n> +\telse if (!strcmp(arg, \"--pickaxe-ignore-add\"))\n> +\t\toptions->pickaxe_opts |= DIFF_PICKAXE_IGNORE_ADD;\n> +\telse if (!strcmp(arg, \"--pickaxe-ignore-del\"))\n> +\t\toptions->pickaxe_opts |= DIFF_PICKAXE_IGNORE_DEL;\n> \telse if (!strcmp(arg, \"--pickaxe-regex\"))\n> \t\toptions->pickaxe_opts |= DIFF_PICKAXE_REGEX;\n> \telse if ((argcount = short_opt('O', av, &optarg))) {\n> diff --git a/diff.h b/diff.h\n> index a30cc35ec3..147c47ace7 100644\n> --- a/diff.h\n> +++ b/diff.h\n> @@ -358,6 +358,8 @@ int git_config_rename(const char *var, const char *value);\n> \t\t\t\t DIFF_PICKAXE_KIND_OBJFIND)\n>\n> #define DIFF_PICKAXE_IGNORE_CASE\t32\n> +#define DIFF_PICKAXE_IGNORE_ADD\t64\n> +#define DIFF_PICKAXE_IGNORE_DEL 128\n>\n> void diffcore_std(struct diff_options *);\n> void diffcore_fix_diff_index(struct diff_options *);\n> diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c\n> index 800a899c86..826dde6bd4 100644\n> --- a/diffcore-pickaxe.c\n> +++ b/diffcore-pickaxe.c\n> @@ -16,6 +16,7 @@ typedef int (*pickaxe_fn)(mmfile_t *one, mmfile_t *two,\n>\n> struct diffgrep_cb {\n> \tregex_t *regexp;\n> +\tstruct diff_options *diff_options;\n> \tint hit;\n> };\n>\n> @@ -23,9 +24,14 @@ static void diffgrep_consume(void *priv, char *line, unsigned long len)\n> {\n> \tstruct diffgrep_cb *data = priv;\n> \tregmatch_t regmatch;\n> +\tunsigned pickaxe_opts = data->diff_options->pickaxe_opts;\n>\n> \tif (line[0] != '+' && line[0] != '-')\n> \t\treturn;\n> +\tif ((pickaxe_opts & DIFF_PICKAXE_IGNORE_ADD) &&\tline[0] == '+')\n> +\t\treturn;\n> +\tif ((pickaxe_opts & DIFF_PICKAXE_IGNORE_DEL) &&\tline[0] == '-')\n> +\t\treturn;\n> \tif (data->hit)\n> \t\t/*\n\nLooks good, but I wonder if a more general version of this couldn't be\nthat instead of returning early if the line doesn't start with +/-\nabove, we have an option to skip that early return.\n\nThen you can simply specify a regex that starts by matching a + or - at\nthe start of the line, but you also get the poweruser tool of matching\nlines around those lines, as tweaked by the -U option. I.e. this:\n\ndiff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c\nindex 800a899c86..90625a110c 100644\n--- a/diffcore-pickaxe.c\n+++ b/diffcore-pickaxe.c\n@@ -24,15 +24,13 @@ static void diffgrep_consume(void *priv, char *line, unsigned long len)\n struct diffgrep_cb *data = priv;\n regmatch_t regmatch;\n\n- if (line[0] != '+' && line[0] != '-')\n- return;\n if (data->hit)\n /*\n * NEEDSWORK: we should have a way to terminate the\n * caller early.\n */\n return;\n- data->hit = !regexec_buf(data->regexp, line + 1, len - 1, 1,\n+ data->hit = !regexec_buf(data->regexp, line, len, 1,\n ®match, 0);\n }\n\nThat patch obviously breaks existing -G, so it would need to be\noptional, but allows me e.g. on git.git to do:\n\n ~/g/git/git --exec-path=/home/avar/g/git log -G'^ .*marc\\.info' -p -U2 -- README.md\n\nTo find a change whose first line of context is a line mentioning\nmarc.info, and then I can use -G'^\\+' to find added lines matching\n etc.\n\nThen the --pickaxe-ignore-add and --pickaxe-ignore-del options in your\npatch could just be implemented in terms of that feature, i.e. by\nimplicitly adding a \"^-\" or \"^\\+\" to the beginning of the regex,\nrespectively, and implicitly turning on a new --pickaxe-raw-lines or\nwhatever we'd call it.\n\n> \t\t * NEEDSWORK: we should have a way to terminate the\n> @@ -45,13 +51,20 @@ static int diff_grep(mmfile_t *one, mmfile_t *two,\n> \txpparam_t xpp;\n> \txdemitconf_t xecfg;\n>\n> -\tif (!one)\n> +\tif (!one) {\n> +\t\tif (o->pickaxe_opts & DIFF_PICKAXE_IGNORE_ADD)\n> +\t\t\treturn 0;\n> \t\treturn !regexec_buf(regexp, two->ptr, two->size,\n> \t\t\t\t 1, ®match, 0);\n> -\tif (!two)\n> +\t}\n> +\tif (!two) {\n> +\t\tif (o->pickaxe_opts & DIFF_PICKAXE_IGNORE_DEL)\n> +\t\t\treturn 0;\n> \t\treturn !regexec_buf(regexp, one->ptr, one->size,\n> \t\t\t\t 1, ®match, 0);\n> +\t}\n>\n> +\tecbdata.diff_options = o;\n> \t/*\n> \t * We have both sides; need to run textual diff and see if\n> \t * the pattern appears on added/deleted lines.\n> @@ -113,6 +126,11 @@ static int has_changes(mmfile_t *one, mmfile_t *two,\n> {\n> \tunsigned int one_contains = one ? contains(one, regexp, kws) : 0;\n> \tunsigned int two_contains = two ? contains(two, regexp, kws) : 0;\n> +\n> +\tif (o->pickaxe_opts & DIFF_PICKAXE_IGNORE_ADD)\n> +\t\treturn one_contains > two_contains;\n> +\tif (o->pickaxe_opts & DIFF_PICKAXE_IGNORE_DEL)\n> +\t\treturn one_contains < two_contains;\n> \treturn one_contains != two_contains;\n> }\n>\n"},{"id":"359922","messageId":"alpine.DEB.2.21.1810091549530.2430@hadrien","threadId":"49500","inReplyTo":"87lg77cmr1.fsf@evledraar.gmail.com","subject":"Re: git log -S or -G","fromName":"Julia Lawall","fromEmail":"julia.lawall@lip6.fr","sentAt":"2018-10-09T13:51:22Z","receivedAt":"2018-10-09T13:51:26Z","isPatch":false,"sender":{"key":"julia.lawall@lip6.fr","avatar":null},"body":"\n\nOn Tue, 9 Oct 2018, Ævar Arnfjörð Bjarmason wrote:\n\n>\n> On Tue, Oct 09 2018, Junio C Hamano wrote:\n>\n> > Jeff King writes:\n> >\n> >> I think that is the best we could do for \"-S\", though, which is\n> >> inherently about counting hits.\n> >>\n> >> For \"-G\", we are literally grepping the diff. It does not seem\n> >> unreasonable to add the ability to grep only \"-\" or \"+\" lines, and the\n> >> interface for that should be pretty straightforward (a tri-state flag to\n> >> look in remove, added, or both lines).\n> >\n> > Yeah, here is a lunchtime hack that hasn't even been compile tested.\n> >\n> > diff.c | 4 ++++\n> > diff.h | 2 ++\n> > diffcore-pickaxe.c | 22 ++++++++++++++++++++--\n> > 3 files changed, 26 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/diff.c b/diff.c\n> > index f0c7557b40..d1f2780844 100644\n> > --- a/diff.c\n> > +++ b/diff.c\n> > @@ -5068,6 +5068,10 @@ int diff_opt_parse(struct diff_options *options,\n> > \t}\n> > \telse if (!strcmp(arg, \"--pickaxe-all\"))\n> > \t\toptions->pickaxe_opts |= DIFF_PICKAXE_ALL;\n> > +\telse if (!strcmp(arg, \"--pickaxe-ignore-add\"))\n> > +\t\toptions->pickaxe_opts |= DIFF_PICKAXE_IGNORE_ADD;\n> > +\telse if (!strcmp(arg, \"--pickaxe-ignore-del\"))\n> > +\t\toptions->pickaxe_opts |= DIFF_PICKAXE_IGNORE_DEL;\n> > \telse if (!strcmp(arg, \"--pickaxe-regex\"))\n> > \t\toptions->pickaxe_opts |= DIFF_PICKAXE_REGEX;\n> > \telse if ((argcount = short_opt('O', av, &optarg))) {\n> > diff --git a/diff.h b/diff.h\n> > index a30cc35ec3..147c47ace7 100644\n> > --- a/diff.h\n> > +++ b/diff.h\n> > @@ -358,6 +358,8 @@ int git_config_rename(const char *var, const char *value);\n> > \t\t\t\t DIFF_PICKAXE_KIND_OBJFIND)\n> >\n> > #define DIFF_PICKAXE_IGNORE_CASE\t32\n> > +#define DIFF_PICKAXE_IGNORE_ADD\t64\n> > +#define DIFF_PICKAXE_IGNORE_DEL 128\n> >\n> > void diffcore_std(struct diff_options *);\n> > void diffcore_fix_diff_index(struct diff_options *);\n> > diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c\n> > index 800a899c86..826dde6bd4 100644\n> > --- a/diffcore-pickaxe.c\n> > +++ b/diffcore-pickaxe.c\n> > @@ -16,6 +16,7 @@ typedef int (*pickaxe_fn)(mmfile_t *one, mmfile_t *two,\n> >\n> > struct diffgrep_cb {\n> > \tregex_t *regexp;\n> > +\tstruct diff_options *diff_options;\n> > \tint hit;\n> > };\n> >\n> > @@ -23,9 +24,14 @@ static void diffgrep_consume(void *priv, char *line, unsigned long len)\n> > {\n> > \tstruct diffgrep_cb *data = priv;\n> > \tregmatch_t regmatch;\n> > +\tunsigned pickaxe_opts = data->diff_options->pickaxe_opts;\n> >\n> > \tif (line[0] != '+' && line[0] != '-')\n> > \t\treturn;\n> > +\tif ((pickaxe_opts & DIFF_PICKAXE_IGNORE_ADD) &&\tline[0] == '+')\n> > +\t\treturn;\n> > +\tif ((pickaxe_opts & DIFF_PICKAXE_IGNORE_DEL) &&\tline[0] == '-')\n> > +\t\treturn;\n> > \tif (data->hit)\n> > \t\t/*\n>\n> Looks good, but I wonder if a more general version of this couldn't be\n> that instead of returning early if the line doesn't start with +/-\n> above, we have an option to skip that early return.\n>\n> Then you can simply specify a regex that starts by matching a + or - at\n> the start of the line, but you also get the poweruser tool of matching\n> lines around those lines, as tweaked by the -U option. I.e. this:\n>\n> diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c\n> index 800a899c86..90625a110c 100644\n> --- a/diffcore-pickaxe.c\n> +++ b/diffcore-pickaxe.c\n> @@ -24,15 +24,13 @@ static void diffgrep_consume(void *priv, char *line, unsigned long len)\n> struct diffgrep_cb *data = priv;\n> regmatch_t regmatch;\n>\n> - if (line[0] != '+' && line[0] != '-')\n> - return;\n> if (data->hit)\n> /*\n> * NEEDSWORK: we should have a way to terminate the\n> * caller early.\n> */\n> return;\n> - data->hit = !regexec_buf(data->regexp, line + 1, len - 1, 1,\n> + data->hit = !regexec_buf(data->regexp, line, len, 1,\n> ®match, 0);\n> }\n>\n> That patch obviously breaks existing -G, so it would need to be\n> optional, but allows me e.g. on git.git to do:\n>\n> ~/g/git/git --exec-path=/home/avar/g/git log -G'^ .*marc\\.info' -p -U2 -- README.md\n>\n> To find a change whose first line of context is a line mentioning\n> marc.info, and then I can use -G'^\\+' to find added lines matching\n> etc.\n\nDo -G's accumulate? I had the impression that only the last one was taken\ninto account, but I didn't check the code on that.\n\njulia\n\n>\n> Then the --pickaxe-ignore-add and --pickaxe-ignore-del options in your\n> patch could just be implemented in terms of that feature, i.e. by\n> implicitly adding a \"^-\" or \"^\\+\" to the beginning of the regex,\n> respectively, and implicitly turning on a new --pickaxe-raw-lines or\n> whatever we'd call it.\n>\n> > \t\t * NEEDSWORK: we should have a way to terminate the\n> > @@ -45,13 +51,20 @@ static int diff_grep(mmfile_t *one, mmfile_t *two,\n> > \txpparam_t xpp;\n> > \txdemitconf_t xecfg;\n> >\n> > -\tif (!one)\n> > +\tif (!one) {\n> > +\t\tif (o->pickaxe_opts & DIFF_PICKAXE_IGNORE_ADD)\n> > +\t\t\treturn 0;\n> > \t\treturn !regexec_buf(regexp, two->ptr, two->size,\n> > \t\t\t\t 1, ®match, 0);\n> > -\tif (!two)\n> > +\t}\n> > +\tif (!two) {\n> > +\t\tif (o->pickaxe_opts & DIFF_PICKAXE_IGNORE_DEL)\n> > +\t\t\treturn 0;\n> > \t\treturn !regexec_buf(regexp, one->ptr, one->size,\n> > \t\t\t\t 1, ®match, 0);\n> > +\t}\n> >\n> > +\tecbdata.diff_options = o;\n> > \t/*\n> > \t * We have both sides; need to run textual diff and see if\n> > \t * the pattern appears on added/deleted lines.\n> > @@ -113,6 +126,11 @@ static int has_changes(mmfile_t *one, mmfile_t *two,\n> > {\n> > \tunsigned int one_contains = one ? contains(one, regexp, kws) : 0;\n> > \tunsigned int two_contains = two ? contains(two, regexp, kws) : 0;\n> > +\n> > +\tif (o->pickaxe_opts & DIFF_PICKAXE_IGNORE_ADD)\n> > +\t\treturn one_contains > two_contains;\n> > +\tif (o->pickaxe_opts & DIFF_PICKAXE_IGNORE_DEL)\n> > +\t\treturn one_contains < two_contains;\n> > \treturn one_contains != two_contains;\n> > }\n> >\n>"}]}