{"thread":{"id":"50983","subject":"Multi-line 'git log -G<regex>'?","startedAt":"2019-04-24T10:26:23Z","lastAt":"2019-05-03T09:11:07Z","messageCount":14,"participants":["Eugeniu Rosca","Ævar Arnfjörð Bjarmason","Junio C Hamano","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"374383","messageId":"20190424102609.GA19697@vmlxhi-102.adit-jv.com","threadId":"50983","inReplyTo":null,"subject":"Multi-line 'git log -G<regex>'?","fromName":"Eugeniu Rosca","fromEmail":"erosca@de.adit-jv.com","sentAt":"2019-04-24T10:26:09Z","receivedAt":"2019-04-24T10:26:23Z","isPatch":false,"sender":{"key":"erosca@de.adit-jv.com","avatar":null},"body":"Hello git community,\n\nIn the context of [1], I would like to find all Linux commits which\nreplaced:\n\t'devm_request_threaded_irq(* IRQF_SHARED *)'\nby:\n\t'devm_request_threaded_irq(* IRQF_ONESHOT *)'\n\nActually, I would be happy with a much lower degree of precision, e.g.\nfinding commits which _removed_ IRQF_SHARED and _added_ IRQF_ONESHOT.\n\nI am aware of the difference between `git log -G` and `git log -S`, but\nthese two options alone don't seem to help me in this particular\nscenario. Is there any git built-in way to achieve my goal?\n\n[1] https://patchwork.kernel.org/patch/10914083/\n    (\"[v4,1/2] thermal: rcar_gen3_thermal: fix interrupt type\")\n\n-- \nBest regards,\nEugeniu.\n"},{"id":"374407","messageId":"20190424152215.16251-1-avarab@gmail.com","threadId":"50983","inReplyTo":"20190424102609.GA19697@vmlxhi-102.adit-jv.com","subject":"[PATCH 0/2] diffcore-pickaxe: implement --pickaxe-raw-diff","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-04-24T15:22:13Z","receivedAt":"2019-04-24T15:22:30Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This implements --pickaxe-raw-diff as explained in 2/2. I based this\non \"next\" because Duy's in-flight diff option refactoring would have\nconflicted with it.\n\nÆvar Arnfjörð Bjarmason (2):\n  diffcore-pickaxe: refactor !one or !two case in diff_grep\n  diffcore-pickaxe: add --pickaxe-raw-diff for use with -G\n\n Documentation/diff-options.txt | 17 ++++++++++++\n diff.c                         |  3 +++\n diff.h                         |  2 ++\n diffcore-pickaxe.c             | 48 +++++++++++++++++++++++++++-------\n t/t4013-diff-various.sh        |  1 +\n t/t4209-log-pickaxe.sh         | 45 +++++++++++++++++++++++++++++++\n 6 files changed, 107 insertions(+), 9 deletions(-)\n\n-- \n2.21.0.593.g511ec345e18\n\n"},{"id":"374408","messageId":"20190424152215.16251-2-avarab@gmail.com","threadId":"50983","inReplyTo":"20190424102609.GA19697@vmlxhi-102.adit-jv.com","subject":"[PATCH 1/2] diffcore-pickaxe: refactor !one or !two case in diff_grep","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-04-24T15:22:14Z","receivedAt":"2019-04-24T15:22:32Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Refactor the code around processing an added (!one) or deleted (!two)\nfile in diff_grep, which is used by the -G option.\n\nThis makes a subsequent change where we'd like to munge the \"one\" or\n\"two\" \"ptr\" smaller. While we're at it let's add an assert that \"one\"\nand \"two\" can't both be false at the same time, which is always the\ncase.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n diffcore-pickaxe.c | 16 ++++++++++------\n 1 file changed, 10 insertions(+), 6 deletions(-)\n\ndiff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c\nindex a9c6d60df2..3c6416bfe2 100644\n--- a/diffcore-pickaxe.c\n+++ b/diffcore-pickaxe.c\n@@ -45,12 +45,16 @@ static int diff_grep(mmfile_t *one, mmfile_t *two,\n \txpparam_t xpp;\n \txdemitconf_t xecfg;\n \n-\tif (!one)\n-\t\treturn !regexec_buf(regexp, two->ptr, two->size,\n-\t\t\t\t    1, &regmatch, 0);\n-\tif (!two)\n-\t\treturn !regexec_buf(regexp, one->ptr, one->size,\n-\t\t\t\t    1, &regmatch, 0);\n+\tif (!one || !two) {\n+\t\tmmfile_t *which = one ? one : two;\n+\t\tint ret;\n+\t\tchar *string = which->ptr;\n+\t\tsize_t size = which->size;\n+\t\tassert(!(!one && !two));\n+\t\tret = !regexec_buf(regexp, string, size,\n+\t\t\t\t   1, &regmatch, 0);\n+\t\treturn ret;\n+\t}\n \n \t/*\n \t * We have both sides; need to run textual diff and see if\n-- \n2.21.0.593.g511ec345e18\n\n"},{"id":"374409","messageId":"20190424152215.16251-3-avarab@gmail.com","threadId":"50983","inReplyTo":"20190424102609.GA19697@vmlxhi-102.adit-jv.com","subject":"[PATCH 2/2] diffcore-pickaxe: add --pickaxe-raw-diff for use with -G","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-04-24T15:22:15Z","receivedAt":"2019-04-24T15:22:34Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add the ability for the -G<regex> pickaxe to search only through added\nor removed lines in the diff, or even through an arbitrary amount of\ncontext lines when combined with -U<n>.\n\nThis has been requested[1][2] a few times in the past, and isn't\ncurrently possible. Instead users need to do -G<regex> and then write\ntheir own post-parsing script to see if the <regex> matched added or\nremoved lines, or both. There was no way to match the adjacent context\nlines other than running and grepping the equivalent of a \"log -p -U<n>\".\n\n1. https://public-inbox.org/git/xmqqwoqrr8y2.fsf@gitster-ct.c.googlers.com/\n2. https://public-inbox.org/git/20190424102609.GA19697@vmlxhi-102.adit-jv.com/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n Documentation/diff-options.txt | 17 +++++++++++++\n diff.c                         |  3 +++\n diff.h                         |  2 ++\n diffcore-pickaxe.c             | 34 ++++++++++++++++++++++---\n t/t4013-diff-various.sh        |  1 +\n t/t4209-log-pickaxe.sh         | 45 ++++++++++++++++++++++++++++++++++\n 6 files changed, 98 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 09faee3b44..f367b40362 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -579,6 +579,9 @@ occurrences of that string did not change).\n Unless `--text` is supplied patches of binary files without a textconv\n filter will be ignored.\n +\n+When `--pickaxe-raw-diff` is supplied the whole diff is searched\n+instead of just added/removed lines. See below.\n++\n See the 'pickaxe' entry in linkgit:gitdiffcore[7] for more\n information.\n \n@@ -600,6 +603,20 @@ The object can be a blob or a submodule commit. It implies the `-t` option in\n \tTreat the <string> given to `-S` as an extended POSIX regular\n \texpression to match.\n \n+--pickaxe-raw-diff::\n+\tWhen `-G` looks for a change a diff will be generated, and\n+\tonly the added/removed lines will be matched against with the\n+\t\"+\" or \"-\" stripped.\n++\n+Supplying this option skips that pre-processing. This makes it\n+possible to match only lines that added or removed something matching\n+a <regex> with \"\\^\\+<regex>\" and \"^-<regex>\", respectively.\n++\n+It also allows for finding something in the diff context. E.g. \"\\^\n+<regex>\" will match the context lines (see `-U<n>` above) around the\n+added/removed lines, and doing an unanchored match will match any of\n+the the added/removed lines & diff context.\n+\n endif::git-format-patch[]\n \n -O<orderfile>::\ndiff --git a/diff.c b/diff.c\nindex 4d3cf83a27..4cdc000ee5 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -5503,6 +5503,9 @@ static void prep_parse_options(struct diff_options *options)\n \t\tOPT_BIT_F(0, \"pickaxe-regex\", &options->pickaxe_opts,\n \t\t\t  N_(\"treat <string> in -S as extended POSIX regular expression\"),\n \t\t\t  DIFF_PICKAXE_REGEX, PARSE_OPT_NONEG),\n+\t\tOPT_BIT_F(0, \"pickaxe-raw-diff\", &options->pickaxe_opts,\n+\t\t\t  N_(\"have <string> in -G match the raw diff output\"),\n+\t\t\t  DIFF_PICKAXE_G_RAW_DIFF, PARSE_OPT_NONEG),\n \t\tOPT_FILENAME('O', NULL, &options->orderfile,\n \t\t\t     N_(\"control the order in which files appear in the output\")),\n \t\tOPT_CALLBACK_F(0, \"find-object\", options, N_(\"<object-id>\"),\ndiff --git a/diff.h b/diff.h\nindex b20cbcc091..d431fbc602 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -370,6 +370,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_G_RAW_DIFF\t\t64\n+\n \n void diffcore_std(struct diff_options *);\n void diffcore_fix_diff_index(void);\ndiff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c\nindex 3c6416bfe2..e23f04b4f0 100644\n--- a/diffcore-pickaxe.c\n+++ b/diffcore-pickaxe.c\n@@ -17,14 +17,18 @@ typedef int (*pickaxe_fn)(mmfile_t *one, mmfile_t *two,\n struct diffgrep_cb {\n \tregex_t *regexp;\n \tint hit;\n+\tint raw_diff;\n };\n \n static void diffgrep_consume(void *priv, char *line, unsigned long len)\n {\n \tstruct diffgrep_cb *data = priv;\n \tregmatch_t regmatch;\n+\tint raw_diff = data->raw_diff;\n+\tconst char *string = raw_diff ? line : line + 1;\n+\tsize_t size = raw_diff ? len : len - 1;\n \n-\tif (line[0] != '+' && line[0] != '-')\n+\tif (!raw_diff && line[0] != '+' && line[0] != '-')\n \t\treturn;\n \tif (data->hit)\n \t\t/*\n@@ -32,7 +36,7 @@ static void diffgrep_consume(void *priv, char *line, unsigned long len)\n \t\t * caller early.\n \t\t */\n \t\treturn;\n-\tdata->hit = !regexec_buf(data->regexp, line + 1, len - 1, 1,\n+\tdata->hit = !regexec_buf(data->regexp, string, size, 1,\n \t\t\t\t &regmatch, 0);\n }\n \n@@ -44,15 +48,36 @@ static int diff_grep(mmfile_t *one, mmfile_t *two,\n \tstruct diffgrep_cb ecbdata;\n \txpparam_t xpp;\n \txdemitconf_t xecfg;\n+\tint raw_diff = o->pickaxe_opts & DIFF_PICKAXE_G_RAW_DIFF;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tchar *string;\n+\tsize_t size;\n \n \tif (!one || !two) {\n \t\tmmfile_t *which = one ? one : two;\n \t\tint ret;\n-\t\tchar *string = which->ptr;\n-\t\tsize_t size = which->size;\n \t\tassert(!(!one && !two));\n+\t\tif (raw_diff) {\n+\t\t\t/*\n+\t\t\t * When we have created/deleted files with\n+\t\t\t * --pickaxe-raw-diff we need to fake up the\n+\t\t\t * \"+\" and \"-\" at the start of the lines, a\n+\t\t\t * plain -G without --pickaxe-raw-diff didn't\n+\t\t\t * care since it would indiscriminately search\n+\t\t\t * through both added and removed lines.\n+\t\t\t */\n+\t\t\tstrbuf_add_lines(&sb, !one ? \"+\" : \"-\", which->ptr,\n+\t\t\t\t\t which->size);\n+\t\t\tstring = sb.buf;\n+\t\t\tsize = sb.len;\n+\t\t} else {\n+\t\t\tstring = which->ptr;\n+\t\t\tsize = which->size;\n+\t\t}\n \t\tret = !regexec_buf(regexp, string, size,\n \t\t\t\t   1, &regmatch, 0);\n+\t\tif (raw_diff)\n+\t\t\tstrbuf_release(&sb);\n \t\treturn ret;\n \t}\n \n@@ -64,6 +89,7 @@ static int diff_grep(mmfile_t *one, mmfile_t *two,\n \tmemset(&xecfg, 0, sizeof(xecfg));\n \tecbdata.regexp = regexp;\n \tecbdata.hit = 0;\n+\tecbdata.raw_diff = raw_diff;\n \txecfg.ctxlen = o->context;\n \txecfg.interhunkctxlen = o->interhunkcontext;\n \tif (xdi_diff_outf(one, two, discard_hunk_line, diffgrep_consume,\ndiff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh\nindex 9f8f0e84ad..39a1f6c230 100755\n--- a/t/t4013-diff-various.sh\n+++ b/t/t4013-diff-various.sh\n@@ -276,6 +276,7 @@ log -SF master --max-count=1\n log -SF master --max-count=2\n log -GF master\n log -GF -p master\n+log -GF -p --pickaxe-raw-diff master\n log -GF -p --pickaxe-all master\n log --decorate --all\n log --decorate=full --all\ndiff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh\nindex 5d06f5f45e..2d98318d23 100755\n--- a/t/t4209-log-pickaxe.sh\n+++ b/t/t4209-log-pickaxe.sh\n@@ -141,4 +141,49 @@ test_expect_success 'log -S looks into binary files' '\n \ttest_cmp log full-log\n '\n \n+test_expect_success 'setup log -G --pickaxe-raw-diff' '\n+\tgit checkout --orphan G-raw-diff &&\n+\ttest_write_lines A B C D E F G >file &&\n+\tgit add file &&\n+\tgit commit --allow-empty-message file &&\n+\tsed -i -e \"s/B/2/\" file &&\n+\tgit add file &&\n+\tgit commit --allow-empty-message file &&\n+\tsed -i -e \"s/D/4/\" file &&\n+\tgit add file &&\n+\tgit commit --allow-empty-message file &&\n+\tgit rm file &&\n+\tgit commit --allow-empty-message &&\n+\tgit log --oneline -1 HEAD~0 >file.fourth &&\n+\tgit log --oneline -1 HEAD~1 >file.third &&\n+\tgit log --oneline -1 HEAD~2 >file.second &&\n+\tgit log --oneline -1 HEAD~3 >file.first\n+'\n+\n+test_expect_success 'log -G --pickaxe-raw-diff skips header and range information' '\n+\tgit log --pickaxe-raw-diff -p -G\"(@@|file)\" >log &&\n+\ttest_must_be_empty log\n+'\n+\n+test_expect_success 'log -G --pickaxe-raw-diff searching in context' '\n+\tgit log --oneline --pickaxe-raw-diff -G\"^ F\" -U2 -s >log &&\n+\ttest_cmp file.third log &&\n+\tgit log --oneline --pickaxe-raw-diff -G\"^ F\" -U1 -s >log &&\n+\ttest_must_be_empty log\n+'\n+\n+test_expect_success 'log -G --pickaxe-raw-diff searching added / removed lines (skip create/delete)' '\n+\tgit log --oneline --pickaxe-raw-diff -G\"^-[D2]\" -s HEAD~1 >log &&\n+\ttest_cmp file.third log &&\n+\tgit log --oneline --pickaxe-raw-diff -G\"^\\+[D2]\" -s -1 >log &&\n+\ttest_cmp file.second log\n+'\n+\n+test_expect_success 'log -G --pickaxe-raw-diff searching created / deleted files' '\n+\tgit log --oneline --pickaxe-raw-diff -G\"^\\+A\" -s >log &&\n+\ttest_cmp file.first log &&\n+\tgit log --oneline --pickaxe-raw-diff -G\"^\\-A\" -s >log &&\n+\ttest_cmp file.fourth log\n+'\n+\n test_done\n-- \n2.21.0.593.g511ec345e18\n\n"},{"id":"374410","messageId":"87o94vs9cp.fsf@evledraar.gmail.com","threadId":"50983","inReplyTo":"20190424152215.16251-3-avarab@gmail.com","subject":"Re: [PATCH 2/2] diffcore-pickaxe: add --pickaxe-raw-diff for use with -G","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-04-24T15:37:10Z","receivedAt":"2019-04-24T15:37:16Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Apr 24 2019, Ævar Arnfjörð Bjarmason wrote:\n\n> Add the ability for the -G<regex> pickaxe to search only through added\n> or removed lines in the diff, or even through an arbitrary amount of\n> context lines when combined with -U<n>.\n>\n> This has been requested[1][2] a few times in the past, and isn't\n> currently possible. Instead users need to do -G<regex> and then write\n> their own post-parsing script to see if the <regex> matched added or\n> removed lines, or both. There was no way to match the adjacent context\n> lines other than running and grepping the equivalent of a \"log -p -U<n>\".\n>\n> 1. https://public-inbox.org/git/xmqqwoqrr8y2.fsf@gitster-ct.c.googlers.com/\n> 2. https://public-inbox.org/git/20190424102609.GA19697@vmlxhi-102.adit-jv.com/\n\nI see now once I actually read Eugeniu Rosca's E-Mail upthread instead\nof just knee-jerk sending out patches that this doesn't actually solve\nhis particular problem fully.\n\nI.e. if you want some AND/OR matching support this --pickaxe-raw-diff\nwon't give you that, but it *does* make it much easier to script up such\nan option. Run it twice with -G\"\\+<regex>\" and -G\"-<regex>\", \"sort |\nuniq -c\" the commit list, and see which things occur once or twice.\n\nOf course that doesn't give you more complex nested and/or cases, but if\ngit-log grew support for that like git-grep has the -G option could use\nthat, although at that point we'd probably want to spend effort on\nmaking the underlying machinery smarter to avoid duplicate work.\n\nFurthermore, and quoting Eugeniu upthread:\n\n    In the context of [1], I would like to find all Linux commits which\n    replaced:\n    \t'devm_request_threaded_irq(* IRQF_SHARED *)'\n    by:\n    \t'devm_request_threaded_irq(* IRQF_ONESHOT *)'\n\nSuch AND/OR machinery would give you what you wanted *most* of the time,\nbut it would also find removed/added pairs that were \"unrelated\" as well\nas \"related\". Solving *that* problem is more complex, but something the\ndiff machinery could in principle expose.\n\nBut the \"-G<regex> --pickaxe-raw-diff\" feature I have as-is is very\nuseful, I've had at least two people off-list ask me about a problem\nthat would be solved by it just in the last 1/2 year (unrelated to them\nhaving seen the WIP patch I sent last October).\n\nIt's more general than Junio's suggested --pickaxe-ignore-{add,del}\noptions[1], but those could be implemented in terms of this underlying\ncode if anyone cared to have those as aliases. You'd just take the\n-G<regex> and prefix the <regex> with \"^\\+\" or \"^-\" as appropriate and\nturn on the DIFF_PICKAXE_G_RAW_DIFF flag.\n\n1. https://public-inbox.org/git/xmqqwoqrr8y2.fsf@gitster-ct.c.googlers.com/\n"},{"id":"374436","messageId":"20190424224539.GA23849@vmlxhi-102.adit-jv.com","threadId":"50983","inReplyTo":"87o94vs9cp.fsf@evledraar.gmail.com","subject":"Re: [PATCH 2/2] diffcore-pickaxe: add --pickaxe-raw-diff for use with -G","fromName":"Eugeniu Rosca","fromEmail":"erosca@de.adit-jv.com","sentAt":"2019-04-24T22:46:54Z","receivedAt":"2019-04-24T22:47:11Z","isPatch":true,"sender":{"key":"erosca@de.adit-jv.com","avatar":null},"body":"Hi Ævar,\n\nThanks for the amazingly fast reply and for the useful feature (yay!).\n\nOn Wed, Apr 24, 2019 at 05:37:10PM +0200, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Wed, Apr 24 2019, Ævar Arnfjörð Bjarmason wrote:\n> \n> > Add the ability for the -G<regex> pickaxe to search only through added\n> > or removed lines in the diff, or even through an arbitrary amount of\n> > context lines when combined with -U<n>.\n> >\n> > This has been requested[1][2] a few times in the past, and isn't\n> > currently possible. Instead users need to do -G<regex> and then write\n> > their own post-parsing script to see if the <regex> matched added or\n> > removed lines, or both. There was no way to match the adjacent context\n> > lines other than running and grepping the equivalent of a \"log -p -U<n>\".\n> >\n> > 1. https://public-inbox.org/git/xmqqwoqrr8y2.fsf@gitster-ct.c.googlers.com/\n> > 2. https://public-inbox.org/git/20190424102609.GA19697@vmlxhi-102.adit-jv.com/\n> \n> I see now once I actually read Eugeniu Rosca's E-Mail upthread instead\n> of just knee-jerk sending out patches that this doesn't actually solve\n> his particular problem fully.\n> \n> I.e. if you want some AND/OR matching support this --pickaxe-raw-diff\n> won't give you that, but it *does* make it much easier to script up such\n> an option. Run it twice with -G\"\\+<regex>\" and -G\"-<regex>\", \"sort |\n> uniq -c\" the commit list, and see which things occur once or twice.\n> \n> Of course that doesn't give you more complex nested and/or cases, but if\n> git-log grew support for that like git-grep has the -G option could use\n> that, although at that point we'd probably want to spend effort on\n> making the underlying machinery smarter to avoid duplicate work.\n\nPurely from user's standpoint, I feel more comfortable with `git grep`\nand `git log --grep` particularly b/c they support '--all-match' [2],\nallowing more flexible multi-line searches. Based on your feedback, it\nlooks to me that `git log -G/-S` did not have a chance to develop their\nfeatures to the same level.\n\n> \n> Furthermore, and quoting Eugeniu upthread:\n> \n>     In the context of [1], I would like to find all Linux commits which\n>     replaced:\n>     \t'devm_request_threaded_irq(* IRQF_SHARED *)'\n>     by:\n>     \t'devm_request_threaded_irq(* IRQF_ONESHOT *)'\n> \n> Such AND/OR machinery would give you what you wanted *most* of the time,\n> but it would also find removed/added pairs that were \"unrelated\" as well\n> as \"related\". Solving *that* problem is more complex, but something the\n> diff machinery could in principle expose.\n\nI expect some false positives, since git is agnostic on the language\nused to write the versioned files (the latter sounds like a research\ntopic to me - I hope there is somebody willing to experiment with that\nin future).\n\n> \n> But the \"-G<regex> --pickaxe-raw-diff\" feature I have as-is is very\n> useful, \n\nI agree. I am a bit bothered by the fact that\n`git log --oneline -Ux -G<regex> --pickaxe-raw-diff` outputs the\ncontents/patch of a commit. My expectation is that we have the\n`log -p` knob for that?\n\n> I've had at least two people off-list ask me about a problem\n> that would be solved by it just in the last 1/2 year (unrelated to them\n> having seen the WIP patch I sent last October).\n> \n> It's more general than Junio's suggested --pickaxe-ignore-{add,del}\n\nAs a user, I would be happier to freely grep in the raw commit contents\nrather than learning a dozen of new options which provide small subsets\nof the same functionality. So, I personally vote for the approach taken\nby --pickaxe-raw-diff. This would also reduce the complexity of my\ncurrent git aliases and/or allow dropping some of them altogether.\n\nQuite off topic, but I also needed to come up with a solution to get\nthe C functions modified/touched by a git commit [3]. It is my\nunderstanding that --pickaxe-raw-diff can't help here and I still have\nto rely on parsing the output of `git log -p`?\n\n> options[1], but those could be implemented in terms of this underlying\n> code if anyone cared to have those as aliases. You'd just take the\n> -G<regex> and prefix the <regex> with \"^\\+\" or \"^-\" as appropriate and\n> turn on the DIFF_PICKAXE_G_RAW_DIFF flag.\n> \n> 1. https://public-inbox.org/git/xmqqwoqrr8y2.fsf@gitster-ct.c.googlers.com/\n\nThanks!\n\n[2] https://gitster.livejournal.com/30195.html\n[3] https://stackoverflow.com/questions/50707171/how-to-get-all-c-functions-modified-by-a-git-commit\n\n-- \nBest regards,\nEugeniu.\n"},{"id":"374439","messageId":"87mukfrnp3.fsf@evledraar.gmail.com","threadId":"50983","inReplyTo":"20190424224539.GA23849@vmlxhi-102.adit-jv.com","subject":"Re: [PATCH 2/2] diffcore-pickaxe: add --pickaxe-raw-diff for use with -G","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-04-24T23:24:56Z","receivedAt":"2019-04-24T23:25:04Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Apr 25 2019, Eugeniu Rosca wrote:\n\n> Hi Ævar,\n>\n> Thanks for the amazingly fast reply and for the useful feature (yay!).\n>\n> On Wed, Apr 24, 2019 at 05:37:10PM +0200, Ævar Arnfjörð Bjarmason wrote:\n>>\n>> On Wed, Apr 24 2019, Ævar Arnfjörð Bjarmason wrote:\n>>\n>> > Add the ability for the -G<regex> pickaxe to search only through added\n>> > or removed lines in the diff, or even through an arbitrary amount of\n>> > context lines when combined with -U<n>.\n>> >\n>> > This has been requested[1][2] a few times in the past, and isn't\n>> > currently possible. Instead users need to do -G<regex> and then write\n>> > their own post-parsing script to see if the <regex> matched added or\n>> > removed lines, or both. There was no way to match the adjacent context\n>> > lines other than running and grepping the equivalent of a \"log -p -U<n>\".\n>> >\n>> > 1. https://public-inbox.org/git/xmqqwoqrr8y2.fsf@gitster-ct.c.googlers.com/\n>> > 2. https://public-inbox.org/git/20190424102609.GA19697@vmlxhi-102.adit-jv.com/\n>>\n>> I see now once I actually read Eugeniu Rosca's E-Mail upthread instead\n>> of just knee-jerk sending out patches that this doesn't actually solve\n>> his particular problem fully.\n>>\n>> I.e. if you want some AND/OR matching support this --pickaxe-raw-diff\n>> won't give you that, but it *does* make it much easier to script up such\n>> an option. Run it twice with -G\"\\+<regex>\" and -G\"-<regex>\", \"sort |\n>> uniq -c\" the commit list, and see which things occur once or twice.\n>>\n>> Of course that doesn't give you more complex nested and/or cases, but if\n>> git-log grew support for that like git-grep has the -G option could use\n>> that, although at that point we'd probably want to spend effort on\n>> making the underlying machinery smarter to avoid duplicate work.\n>\n> Purely from user's standpoint, I feel more comfortable with `git grep`\n> and `git log --grep` particularly b/c they support '--all-match' [2],\n> allowing more flexible multi-line searches. Based on your feedback, it\n> looks to me that `git log -G/-S` did not have a chance to develop their\n> features to the same level.\n>\n>>\n>> Furthermore, and quoting Eugeniu upthread:\n>>\n>>     In the context of [1], I would like to find all Linux commits which\n>>     replaced:\n>>     \t'devm_request_threaded_irq(* IRQF_SHARED *)'\n>>     by:\n>>     \t'devm_request_threaded_irq(* IRQF_ONESHOT *)'\n>>\n>> Such AND/OR machinery would give you what you wanted *most* of the time,\n>> but it would also find removed/added pairs that were \"unrelated\" as well\n>> as \"related\". Solving *that* problem is more complex, but something the\n>> diff machinery could in principle expose.\n>\n> I expect some false positives, since git is agnostic on the language\n> used to write the versioned files (the latter sounds like a research\n> topic to me - I hope there is somebody willing to experiment with that\n> in future).\n\nI was thinking of something where the added/removed could be filtered to\ncases that occur in the same diff hunk.\n\n>>\n>> But the \"-G<regex> --pickaxe-raw-diff\" feature I have as-is is very\n>> useful,\n>\n> I agree. I am a bit bothered by the fact that\n> `git log --oneline -Ux -G<regex> --pickaxe-raw-diff` outputs the\n> contents/patch of a commit. My expectation is that we have the\n> `log -p` knob for that?\n\nThis is unrelated to --pickaxe-raw-diff, -U<n> just implies -p in\ngeneral. See e.g. \"git log -U1\".\n\n>> I've had at least two people off-list ask me about a problem\n>> that would be solved by it just in the last 1/2 year (unrelated to them\n>> having seen the WIP patch I sent last October).\n>>\n>> It's more general than Junio's suggested --pickaxe-ignore-{add,del}\n>\n> As a user, I would be happier to freely grep in the raw commit contents\n> rather than learning a dozen of new options which provide small subsets\n> of the same functionality. So, I personally vote for the approach taken\n> by --pickaxe-raw-diff. This would also reduce the complexity of my\n> current git aliases and/or allow dropping some of them altogether.\n>\n> Quite off topic, but I also needed to come up with a solution to get\n> the C functions modified/touched by a git commit [3]. It is my\n> understanding that --pickaxe-raw-diff can't help here and I still have\n> to rely on parsing the output of `git log -p`?\n\nYeah, it doesn't help with that. When it runs we haven't generated the\ncontext line or the \"@@\" line yet, that's later. You can breakpoint on\nxdl_format_hunk_hdr and diffgrep_consume to see it in action.\n\nIt's a waste of CPU to generate that for all possible hunks, most of\nwhich we won't show at all.\n\nBut it's of course possible to do so by running the full diff machinery\nover every commit and matching on the result, the current pickaxe is\njust taking shortcuts and not doing that.\n\n>> options[1], but those could be implemented in terms of this underlying\n>> code if anyone cared to have those as aliases. You'd just take the\n>> -G<regex> and prefix the <regex> with \"^\\+\" or \"^-\" as appropriate and\n>> turn on the DIFF_PICKAXE_G_RAW_DIFF flag.\n>>\n>> 1. https://public-inbox.org/git/xmqqwoqrr8y2.fsf@gitster-ct.c.googlers.com/\n>\n> Thanks!\n>\n> [2] https://gitster.livejournal.com/30195.html\n> [3] https://stackoverflow.com/questions/50707171/how-to-get-all-c-functions-modified-by-a-git-commit\n"},{"id":"374443","messageId":"xmqqsgu6zzev.fsf@gitster-ct.c.googlers.com","threadId":"50983","inReplyTo":"87mukfrnp3.fsf@evledraar.gmail.com","subject":"Re: [PATCH 2/2] diffcore-pickaxe: add --pickaxe-raw-diff for use with -G","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-04-25T00:44:40Z","receivedAt":"2019-04-25T00:44:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n>> I agree. I am a bit bothered by the fact that\n>> `git log --oneline -Ux -G<regex> --pickaxe-raw-diff` outputs the\n>> contents/patch of a commit. My expectation is that we have the\n>> `log -p` knob for that?\n>\n> This is unrelated to --pickaxe-raw-diff, -U<n> just implies -p in\n> general. See e.g. \"git log -U1\".\n\nThe reason why I found this exchange interesting is because I think\nit shows a noteworthy gap between end-user expectations and what the\nimplementors know.\n\nStepping back (or sideways) a bit, pretend for a while that there\nwere no \"pickaxe\" feature in Git.  Instead there is the \"patch-grep\"\ntool whose design is roughly:\n\n   1. It reads \"git log -p\" output from its standard input, and\n      splits the lines into records, each of which consists of the\n      header part (i.e. starting at the \"commit <object name>\" line,\n      to the first blank line before the title), the log message\n      part, and the patch part.\n\n   2. It takes command line arguments, which are, like \"git grep\",\n      patterns to match and instructions on how to combine the match\n      result.\n\n   3. It applies the match criteria only to the patch part of each\n      record.  A record without any match in the patch part is\n      discarded.\n\n   4. It uses the surviving record's \"commit <object name>\" lines\n      to decide what commits to show.  It does the moral equivalent\n      of invoking \"git show\" on each of them, and perhaps lets you\n      affect how the commits are shown.\n\n      Or perhaps it just lists the commit object names chosen for\n      further processing by downstream tools that read from it.\n\n\nSo the user would be able to say something like\n\n\tgit log -Ux --since=6.months |\n\tgit patch-grep \\\n\t\t--commit-names-only \\\n\t\t--all-match \\\n\t\t-e '+.*devm_request_threaded_irq(IRQF_SHARED)' \\\n                -e '-.*devm_request_threaded_irq(IRQF_ONESHOT)' |\n\txargs git show --oneline -s\n\nAs an implementor, you know that is not how your -G<pattern> thing\nworks, but coming from the end-user side, I think it is a reasonable\nmental model to expect a tool to work more like so.  And I think the\nexpectation from combining --oneline with -Ux was that the -U option\nwould apply to step 1, not step 4 (as --oneline is a clear\nindication that the user wants a very concise final result).\n\nPersonally, I think the _best_ match for the original wish would be\nto have that hypothetical \"git patch-grep\" read from \"git log -L\"\nthat is limited to the C function in the source the user is\ninterested in.\n\nAnd until \"git patch-grep\" becomes reality, I would probably have\ndone\n\n\tgit log -L<function of interest> -U<x> | less\n\nand asked \"less\" to skip to a match with\n\n\t/(IRQF_SHARED|IRQF_ONESHOT)\n\nand then kept hitting 'n' until I find what replaces them, as a\nstop-gap measure.\n\nBy the way, I think your thing is interesting regardless, even if it\ndoes not match the use case in the original thread (it actually may\nmatch---I didn't think it through).\n\nBecause in the context of diff/log family, however, the word \"raw\"\nhas a specific connotation about the \"--raw\" format (as opposed to\n\"--patch\"), I would not call this \"grep the patch output itself,\ninstead of grepping the source (guided by the patch output to tell\nwhat lines are near the lines that got replaced)\" feature anything\n\"raw\", by the way.\n\n\n"},{"id":"374444","messageId":"20190425005448.GA6466@x230","threadId":"50983","inReplyTo":"87mukfrnp3.fsf@evledraar.gmail.com","subject":"Re: [PATCH 2/2] diffcore-pickaxe: add --pickaxe-raw-diff for use with -G","fromName":"Eugeniu Rosca","fromEmail":"roscaeugeniu@gmail.com","sentAt":"2019-04-25T00:54:48Z","receivedAt":"2019-04-25T00:55:03Z","isPatch":true,"sender":{"key":"roscaeugeniu@gmail.com","avatar":null},"body":"On Thu, Apr 25, 2019 at 01:24:56AM +0200, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Thu, Apr 25 2019, Eugeniu Rosca wrote:\n> \n> > Hi Ævar,\n> >\n> > Thanks for the amazingly fast reply and for the useful feature (yay!).\n> >\n> > On Wed, Apr 24, 2019 at 05:37:10PM +0200, Ævar Arnfjörð Bjarmason wrote:\n> >>\n> >> On Wed, Apr 24 2019, Ævar Arnfjörð Bjarmason wrote:\n> >>\n> >> > Add the ability for the -G<regex> pickaxe to search only through added\n> >> > or removed lines in the diff, or even through an arbitrary amount of\n> >> > context lines when combined with -U<n>.\n> >> >\n> >> > This has been requested[1][2] a few times in the past, and isn't\n> >> > currently possible. Instead users need to do -G<regex> and then write\n> >> > their own post-parsing script to see if the <regex> matched added or\n> >> > removed lines, or both. There was no way to match the adjacent context\n> >> > lines other than running and grepping the equivalent of a \"log -p -U<n>\".\n> >> >\n> >> > 1. https://public-inbox.org/git/xmqqwoqrr8y2.fsf@gitster-ct.c.googlers.com/\n> >> > 2. https://public-inbox.org/git/20190424102609.GA19697@vmlxhi-102.adit-jv.com/\n> >>\n> >> I see now once I actually read Eugeniu Rosca's E-Mail upthread instead\n> >> of just knee-jerk sending out patches that this doesn't actually solve\n> >> his particular problem fully.\n> >>\n> >> I.e. if you want some AND/OR matching support this --pickaxe-raw-diff\n> >> won't give you that, but it *does* make it much easier to script up such\n> >> an option. Run it twice with -G\"\\+<regex>\" and -G\"-<regex>\", \"sort |\n> >> uniq -c\" the commit list, and see which things occur once or twice.\n> >>\n> >> Of course that doesn't give you more complex nested and/or cases, but if\n> >> git-log grew support for that like git-grep has the -G option could use\n> >> that, although at that point we'd probably want to spend effort on\n> >> making the underlying machinery smarter to avoid duplicate work.\n> >\n> > Purely from user's standpoint, I feel more comfortable with `git grep`\n> > and `git log --grep` particularly b/c they support '--all-match' [2],\n> > allowing more flexible multi-line searches. Based on your feedback, it\n> > looks to me that `git log -G/-S` did not have a chance to develop their\n> > features to the same level.\n> >\n> >>\n> >> Furthermore, and quoting Eugeniu upthread:\n> >>\n> >>     In the context of [1], I would like to find all Linux commits which\n> >>     replaced:\n> >>     \t'devm_request_threaded_irq(* IRQF_SHARED *)'\n> >>     by:\n> >>     \t'devm_request_threaded_irq(* IRQF_ONESHOT *)'\n> >>\n> >> Such AND/OR machinery would give you what you wanted *most* of the time,\n> >> but it would also find removed/added pairs that were \"unrelated\" as well\n> >> as \"related\". Solving *that* problem is more complex, but something the\n> >> diff machinery could in principle expose.\n> >\n> > I expect some false positives, since git is agnostic on the language\n> > used to write the versioned files (the latter sounds like a research\n> > topic to me - I hope there is somebody willing to experiment with that\n> > in future).\n> \n> I was thinking of something where the added/removed could be filtered to\n> cases that occur in the same diff hunk.\n> \n> >>\n> >> But the \"-G<regex> --pickaxe-raw-diff\" feature I have as-is is very\n> >> useful,\n> >\n> > I agree. I am a bit bothered by the fact that\n> > `git log --oneline -Ux -G<regex> --pickaxe-raw-diff` outputs the\n> > contents/patch of a commit. My expectation is that we have the\n> > `log -p` knob for that?\n> \n> This is unrelated to --pickaxe-raw-diff, -U<n> just implies -p in\n> general. See e.g. \"git log -U1\".\n\nOops. Since I use `-U<n>` mostly with `git show`, I missed the\nimplication. You are right. Then, my question is how users are\ngoing to (quote from commit description):\n\n> >> > [..] search [..] through an arbitrary amount of\n> >> > context lines when combined with -U<n>.\n\nand achieve a `git log --oneline` report, given that -U<n> unfolds\nthe commits?\n\nFTR, based on my quick experiments, --pickaxe-raw-diff does process\nseveral lines of context by default (it appears to default to -U3).\n\n> \n> >> I've had at least two people off-list ask me about a problem\n> >> that would be solved by it just in the last 1/2 year (unrelated to them\n> >> having seen the WIP patch I sent last October).\n> >>\n> >> It's more general than Junio's suggested --pickaxe-ignore-{add,del}\n> >\n> > As a user, I would be happier to freely grep in the raw commit contents\n> > rather than learning a dozen of new options which provide small subsets\n> > of the same functionality. So, I personally vote for the approach taken\n> > by --pickaxe-raw-diff. This would also reduce the complexity of my\n> > current git aliases and/or allow dropping some of them altogether.\n> >\n> > Quite off topic, but I also needed to come up with a solution to get\n> > the C functions modified/touched by a git commit [3]. It is my\n> > understanding that --pickaxe-raw-diff can't help here and I still have\n> > to rely on parsing the output of `git log -p`?\n> \n> Yeah, it doesn't help with that. When it runs we haven't generated the\n> context line or the \"@@\" line yet, that's later. You can breakpoint on\n> xdl_format_hunk_hdr and diffgrep_consume to see it in action.\n> \n> It's a waste of CPU to generate that for all possible hunks, most of\n> which we won't show at all.\n> \n> But it's of course possible to do so by running the full diff machinery\n> over every commit and matching on the result, the current pickaxe is\n> just taking shortcuts and not doing that.\n> \n> >> options[1], but those could be implemented in terms of this underlying\n> >> code if anyone cared to have those as aliases. You'd just take the\n> >> -G<regex> and prefix the <regex> with \"^\\+\" or \"^-\" as appropriate and\n> >> turn on the DIFF_PICKAXE_G_RAW_DIFF flag.\n> >>\n> >> 1. https://public-inbox.org/git/xmqqwoqrr8y2.fsf@gitster-ct.c.googlers.com/\n> >\n> > Thanks!\n> >\n> > [2] https://gitster.livejournal.com/30195.html\n> > [3] https://stackoverflow.com/questions/50707171/how-to-get-all-c-functions-modified-by-a-git-commit\n"},{"id":"374497","messageId":"87h8ams2ml.fsf@evledraar.gmail.com","threadId":"50983","inReplyTo":"20190425005448.GA6466@x230","subject":"Re: [PATCH 2/2] diffcore-pickaxe: add --pickaxe-raw-diff for use with -G","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-04-25T12:14:42Z","receivedAt":"2019-04-25T12:14:50Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Apr 25 2019, Eugeniu Rosca wrote:\n\n> On Thu, Apr 25, 2019 at 01:24:56AM +0200, Ævar Arnfjörð Bjarmason wrote:\n>>\n>> On Thu, Apr 25 2019, Eugeniu Rosca wrote:\n>>\n>> > Hi Ævar,\n>> >\n>> > Thanks for the amazingly fast reply and for the useful feature (yay!).\n>> >\n>> > On Wed, Apr 24, 2019 at 05:37:10PM +0200, Ævar Arnfjörð Bjarmason wrote:\n>> >>\n>> >> On Wed, Apr 24 2019, Ævar Arnfjörð Bjarmason wrote:\n>> >>\n>> >> > Add the ability for the -G<regex> pickaxe to search only through added\n>> >> > or removed lines in the diff, or even through an arbitrary amount of\n>> >> > context lines when combined with -U<n>.\n>> >> >\n>> >> > This has been requested[1][2] a few times in the past, and isn't\n>> >> > currently possible. Instead users need to do -G<regex> and then write\n>> >> > their own post-parsing script to see if the <regex> matched added or\n>> >> > removed lines, or both. There was no way to match the adjacent context\n>> >> > lines other than running and grepping the equivalent of a \"log -p -U<n>\".\n>> >> >\n>> >> > 1. https://public-inbox.org/git/xmqqwoqrr8y2.fsf@gitster-ct.c.googlers.com/\n>> >> > 2. https://public-inbox.org/git/20190424102609.GA19697@vmlxhi-102.adit-jv.com/\n>> >>\n>> >> I see now once I actually read Eugeniu Rosca's E-Mail upthread instead\n>> >> of just knee-jerk sending out patches that this doesn't actually solve\n>> >> his particular problem fully.\n>> >>\n>> >> I.e. if you want some AND/OR matching support this --pickaxe-raw-diff\n>> >> won't give you that, but it *does* make it much easier to script up such\n>> >> an option. Run it twice with -G\"\\+<regex>\" and -G\"-<regex>\", \"sort |\n>> >> uniq -c\" the commit list, and see which things occur once or twice.\n>> >>\n>> >> Of course that doesn't give you more complex nested and/or cases, but if\n>> >> git-log grew support for that like git-grep has the -G option could use\n>> >> that, although at that point we'd probably want to spend effort on\n>> >> making the underlying machinery smarter to avoid duplicate work.\n>> >\n>> > Purely from user's standpoint, I feel more comfortable with `git grep`\n>> > and `git log --grep` particularly b/c they support '--all-match' [2],\n>> > allowing more flexible multi-line searches. Based on your feedback, it\n>> > looks to me that `git log -G/-S` did not have a chance to develop their\n>> > features to the same level.\n>> >\n>> >>\n>> >> Furthermore, and quoting Eugeniu upthread:\n>> >>\n>> >>     In the context of [1], I would like to find all Linux commits which\n>> >>     replaced:\n>> >>     \t'devm_request_threaded_irq(* IRQF_SHARED *)'\n>> >>     by:\n>> >>     \t'devm_request_threaded_irq(* IRQF_ONESHOT *)'\n>> >>\n>> >> Such AND/OR machinery would give you what you wanted *most* of the time,\n>> >> but it would also find removed/added pairs that were \"unrelated\" as well\n>> >> as \"related\". Solving *that* problem is more complex, but something the\n>> >> diff machinery could in principle expose.\n>> >\n>> > I expect some false positives, since git is agnostic on the language\n>> > used to write the versioned files (the latter sounds like a research\n>> > topic to me - I hope there is somebody willing to experiment with that\n>> > in future).\n>>\n>> I was thinking of something where the added/removed could be filtered to\n>> cases that occur in the same diff hunk.\n>>\n>> >>\n>> >> But the \"-G<regex> --pickaxe-raw-diff\" feature I have as-is is very\n>> >> useful,\n>> >\n>> > I agree. I am a bit bothered by the fact that\n>> > `git log --oneline -Ux -G<regex> --pickaxe-raw-diff` outputs the\n>> > contents/patch of a commit. My expectation is that we have the\n>> > `log -p` knob for that?\n>>\n>> This is unrelated to --pickaxe-raw-diff, -U<n> just implies -p in\n>> general. See e.g. \"git log -U1\".\n>\n> Oops. Since I use `-U<n>` mostly with `git show`, I missed the\n> implication. You are right. Then, my question is how users are\n> going to (quote from commit description):\n>\n>> >> > [..] search [..] through an arbitrary amount of\n>> >> > context lines when combined with -U<n>.\n>\n> and achieve a `git log --oneline` report, given that -U<n> unfolds\n> the commits?\n>\n> FTR, based on my quick experiments, --pickaxe-raw-diff does process\n> several lines of context by default (it appears to default to -U3).\n\nYeah I should document this explicitly. We use the default diff context\nso if you just -G'foo.*bar' you'll find things in the 6x lines of\ncontext (3 before / 3 after), not just the \"-\" and \"+\" lines.\n\nIt's a \"feature\", but we should be really clear about it, i.e. you need\nto anchor with \"^[+-]\" if you want the same thing that -G does for you\nnow.\n\nI *do* find the default semantics really useful. Sometimes you can use\n-L, but I've often done manual greps with -U<n> for \"let's find code\nchanges anywhere in the project near places where we use some API\",\nmaybe we should pick -U0 with --pickaxe-raw-diff by default to avoid\n*that* particular surprise by default, but I think that would be even\nmore confusing...\n\n>>\n>> >> I've had at least two people off-list ask me about a problem\n>> >> that would be solved by it just in the last 1/2 year (unrelated to them\n>> >> having seen the WIP patch I sent last October).\n>> >>\n>> >> It's more general than Junio's suggested --pickaxe-ignore-{add,del}\n>> >\n>> > As a user, I would be happier to freely grep in the raw commit contents\n>> > rather than learning a dozen of new options which provide small subsets\n>> > of the same functionality. So, I personally vote for the approach taken\n>> > by --pickaxe-raw-diff. This would also reduce the complexity of my\n>> > current git aliases and/or allow dropping some of them altogether.\n>> >\n>> > Quite off topic, but I also needed to come up with a solution to get\n>> > the C functions modified/touched by a git commit [3]. It is my\n>> > understanding that --pickaxe-raw-diff can't help here and I still have\n>> > to rely on parsing the output of `git log -p`?\n>>\n>> Yeah, it doesn't help with that. When it runs we haven't generated the\n>> context line or the \"@@\" line yet, that's later. You can breakpoint on\n>> xdl_format_hunk_hdr and diffgrep_consume to see it in action.\n>>\n>> It's a waste of CPU to generate that for all possible hunks, most of\n>> which we won't show at all.\n>>\n>> But it's of course possible to do so by running the full diff machinery\n>> over every commit and matching on the result, the current pickaxe is\n>> just taking shortcuts and not doing that.\n>>\n>> >> options[1], but those could be implemented in terms of this underlying\n>> >> code if anyone cared to have those as aliases. You'd just take the\n>> >> -G<regex> and prefix the <regex> with \"^\\+\" or \"^-\" as appropriate and\n>> >> turn on the DIFF_PICKAXE_G_RAW_DIFF flag.\n>> >>\n>> >> 1. https://public-inbox.org/git/xmqqwoqrr8y2.fsf@gitster-ct.c.googlers.com/\n>> >\n>> > Thanks!\n>> >\n>> > [2] https://gitster.livejournal.com/30195.html\n>> > [3] https://stackoverflow.com/questions/50707171/how-to-get-all-c-functions-modified-by-a-git-commit\n"},{"id":"374498","messageId":"87ftq6s252.fsf@evledraar.gmail.com","threadId":"50983","inReplyTo":"xmqqsgu6zzev.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 2/2] diffcore-pickaxe: add --pickaxe-raw-diff for use with -G","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-04-25T12:25:13Z","receivedAt":"2019-04-25T12:25:21Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Apr 25 2019, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n>\n>>> I agree. I am a bit bothered by the fact that\n>>> `git log --oneline -Ux -G<regex> --pickaxe-raw-diff` outputs the\n>>> contents/patch of a commit. My expectation is that we have the\n>>> `log -p` knob for that?\n>>\n>> This is unrelated to --pickaxe-raw-diff, -U<n> just implies -p in\n>> general. See e.g. \"git log -U1\".\n>\n> The reason why I found this exchange interesting is because I think\n> it shows a noteworthy gap between end-user expectations and what the\n> implementors know.\n>\n> Stepping back (or sideways) a bit, pretend for a while that there\n> were no \"pickaxe\" feature in Git.  Instead there is the \"patch-grep\"\n> tool whose design is roughly:\n>\n>    1. It reads \"git log -p\" output from its standard input, and\n>       splits the lines into records, each of which consists of the\n>       header part (i.e. starting at the \"commit <object name>\" line,\n>       to the first blank line before the title), the log message\n>       part, and the patch part.\n>\n>    2. It takes command line arguments, which are, like \"git grep\",\n>       patterns to match and instructions on how to combine the match\n>       result.\n>\n>    3. It applies the match criteria only to the patch part of each\n>       record.  A record without any match in the patch part is\n>       discarded.\n>\n>    4. It uses the surviving record's \"commit <object name>\" lines\n>       to decide what commits to show.  It does the moral equivalent\n>       of invoking \"git show\" on each of them, and perhaps lets you\n>       affect how the commits are shown.\n>\n>       Or perhaps it just lists the commit object names chosen for\n>       further processing by downstream tools that read from it.\n>\n>\n> So the user would be able to say something like\n>\n> \tgit log -Ux --since=6.months |\n> \tgit patch-grep \\\n> \t\t--commit-names-only \\\n> \t\t--all-match \\\n> \t\t-e '+.*devm_request_threaded_irq(IRQF_SHARED)' \\\n>                 -e '-.*devm_request_threaded_irq(IRQF_ONESHOT)' |\n> \txargs git show --oneline -s\n>\n> As an implementor, you know that is not how your -G<pattern> thing\n> works, but coming from the end-user side, I think it is a reasonable\n> mental model to expect a tool to work more like so.  And I think the\n> expectation from combining --oneline with -Ux was that the -U option\n> would apply to step 1, not step 4 (as --oneline is a clear\n> indication that the user wants a very concise final result).\n>\n> Personally, I think the _best_ match for the original wish would be\n> to have that hypothetical \"git patch-grep\" read from \"git log -L\"\n> that is limited to the C function in the source the user is\n> interested in.\n>\n> And until \"git patch-grep\" becomes reality, I would probably have\n> done\n>\n> \tgit log -L<function of interest> -U<x> | less\n>\n> and asked \"less\" to skip to a match with\n>\n> \t/(IRQF_SHARED|IRQF_ONESHOT)\n>\n> and then kept hitting 'n' until I find what replaces them, as a\n> stop-gap measure.\n>\n> By the way, I think your thing is interesting regardless, even if it\n> does not match the use case in the original thread (it actually may\n> match---I didn't think it through).\n\nYeah it's definitely a bit orthagonal, should have sent it in reply to\nsomething else and actually read the E-Mail, but I think it's useful.\n\n> Because in the context of diff/log family, however, the word \"raw\"\n> has a specific connotation about the \"--raw\" format (as opposed to\n> \"--patch\"), I would not call this \"grep the patch output itself,\n> instead of grepping the source (guided by the patch output to tell\n> what lines are near the lines that got replaced)\" feature anything\n> \"raw\", by the way.\n\nI agree, brainfarted on not thinking about \"raw\". Do you or anyone have\na suggestion for a better CLI option name?\n\nMaybe --pickaxe-patch or --pickaxe-patch-format (to go with git-diff's\n-u aka --patch (i.e. not --raw) default format)? Or\n--pickaxe-G-with-context or --pickaxe-with-context or\n--with-pickaxe-context or --pickaxe-context ? All of these suck, but I'm\ncoming up blank on a better one :)\n\nProbably the least shitty of those shitty options is --pickaxe-patch,\nsince we have --patch which triggers the same format, and we can\ndocument that the default is a -G search through --no-pickaxe-patch, and\nyou can just tweak the format.\n\nIt also leaves the door open (unlike having *-G-* in the option) to\nsupport this for -S if anyone cared...\n"},{"id":"374851","messageId":"20190503031531.GA19436@sigill.intra.peff.net","threadId":"50983","inReplyTo":"20190425005448.GA6466@x230","subject":"Re: [PATCH 2/2] diffcore-pickaxe: add --pickaxe-raw-diff for use with -G","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-05-03T03:15:31Z","receivedAt":"2019-05-03T03:15:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 25, 2019 at 02:54:48AM +0200, Eugeniu Rosca wrote:\n\n> > This is unrelated to --pickaxe-raw-diff, -U<n> just implies -p in\n> > general. See e.g. \"git log -U1\".\n> \n> Oops. Since I use `-U<n>` mostly with `git show`, I missed the\n> implication. You are right. Then, my question is how users are\n> going to (quote from commit description):\n> \n> > >> > [..] search [..] through an arbitrary amount of\n> > >> > context lines when combined with -U<n>.\n> \n> and achieve a `git log --oneline` report, given that -U<n> unfolds\n> the commits?\n\nYou can use \"-s\" to suppress patch output; as long as it comes after -U\non the command-line, it will countermand the patch-format part.\n\n(Of course it doesn't matter until we have a raw-diff grep, since\notherwise the context lines do not matter at all, and you should just\nomit -U entirely).\n\n-Peff\n"},{"id":"374860","messageId":"20190503083700.GA6115@vmlxhi-102.adit-jv.com","threadId":"50983","inReplyTo":"xmqqsgu6zzev.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 2/2] diffcore-pickaxe: add --pickaxe-raw-diff for use with -G","fromName":"Eugeniu Rosca","fromEmail":"erosca@de.adit-jv.com","sentAt":"2019-05-03T08:37:00Z","receivedAt":"2019-05-03T08:37:13Z","isPatch":true,"sender":{"key":"erosca@de.adit-jv.com","avatar":null},"body":"On Thu, Apr 25, 2019 at 09:44:40AM +0900, Junio C Hamano wrote:\n[..]\n> So the user would be able to say something like\n> \n> \tgit log -Ux --since=6.months |\n> \tgit patch-grep \\\n> \t\t--commit-names-only \\\n> \t\t--all-match \\\n> \t\t-e '+.*devm_request_threaded_irq(IRQF_SHARED)' \\\n>                 -e '-.*devm_request_threaded_irq(IRQF_ONESHOT)' |\n> \txargs git show --oneline -s\n[..]\n\nJFTR/FWIW, this looks quite user friendly to me, but I believe users\nlike me can (and likely do) already handcraft their own variants of\n'git patch-grep', since the above model implies piping (as opposed to\nin-git processing done by --pickaxe-raw-diff).\n\n-- \nBest Regards,\nEugeniu.\n"},{"id":"374864","messageId":"20190503091049.GA6438@vmlxhi-102.adit-jv.com","threadId":"50983","inReplyTo":"87ftq6s252.fsf@evledraar.gmail.com","subject":"Re: [PATCH 2/2] diffcore-pickaxe: add --pickaxe-raw-diff for use with -G","fromName":"Eugeniu Rosca","fromEmail":"erosca@de.adit-jv.com","sentAt":"2019-05-03T09:10:49Z","receivedAt":"2019-05-03T09:11:07Z","isPatch":true,"sender":{"key":"erosca@de.adit-jv.com","avatar":null},"body":"On Thu, Apr 25, 2019 at 02:25:13PM +0200, Ævar Arnfjörð Bjarmason wrote:\n[..]\n> Do you or anyone have a suggestion for a better CLI option name?\n> \n> Maybe --pickaxe-patch or --pickaxe-patch-format (to go with git-diff's\n> -u aka --patch (i.e. not --raw) default format)? Or\n> --pickaxe-G-with-context or --pickaxe-with-context or\n> --with-pickaxe-context or --pickaxe-context ? All of these suck, but I'm\n> coming up blank on a better one :)\n\n'--pickaxe-patch' is shorter than '--pickaxe-raw-diff', hence more\nconvenient to me. It looks like 'pickaxe-all' and 'pickaxe-regex' are\nthe only --pickaxe-* options currently implemented. Both of them are\ntwo-worded only and easy to remember/type. I think '--pickaxe-patch'\nis more user-friendly, but I leave git people to say the final word.\n\n> \n> Probably the least shitty of those shitty options is --pickaxe-patch,\n> since we have --patch which triggers the same format, and we can\n> document that the default is a -G search through --no-pickaxe-patch, and\n> you can just tweak the format.\n> \n> It also leaves the door open (unlike having *-G-* in the option) to\n> support this for -S if anyone cared...\n\n-- \nBest Regards,\nEugeniu.\n"}]}