{"thread":{"id":"62511","subject":"Long names for `git log -S` and `git log -G`","startedAt":"2024-11-18T23:56:32Z","lastAt":"2025-02-05T07:15:25Z","messageCount":8,"participants":["Illia Bobyr","Junio C Hamano","Jeff King","Johannes Sixt"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"507553","messageId":"24458598-ebbe-41fc-8517-457fa65ed481@gmail.com","threadId":"62511","inReplyTo":null,"subject":"Long names for `git log -S` and `git log -G`","fromName":"Illia Bobyr","fromEmail":"illia.bobyr@gmail.com","sentAt":"2024-11-18T23:56:30Z","receivedAt":"2024-11-18T23:56:32Z","isPatch":false,"sender":{"key":"illia.bobyr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/694419?v=4"},"body":"Hello,\n\nI would like to add long names for the `-S` and `-G` options for `git log`.\nIt seems that most options have long versions, though there are some \nexceptions.\n\nI was wondering if there could be any objections.\nAnd also, what would be a good name for each.\n\nBoth are provided by the diff-pickaxe functionality.\n`-S` is already affected by `--pickaxe-regex` and both `-G` and `-S` are \naffected by `--pickaxe-all`.\n\nAlso,`diffcore` docs says:\n\n > \"-G\" (mnemonic: grep)\n\nI was thinking of `--pickaxe` for `-S` and `--grep` for `-G`.\nAnd it would probably make sense to discuss this before I try submitting \na patch.\n\n`--pickaxe-grep` for `-G` seems like a reasonable alternative name for `-G`.\nNot sure what would be a reasonably short alternative for `-S`.\n`--pickaxe-occurance-change` seems too long, and might not be as clear.\n`--pickaxe-occurance-count-change` is just way too long.\n\nThank you,\nIllia Bobyr\n\n"},{"id":"507565","messageId":"xmqqo72bev71.fsf@gitster.g","threadId":"62511","inReplyTo":"24458598-ebbe-41fc-8517-457fa65ed481@gmail.com","subject":"Re: Long names for `git log -S` and `git log -G`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-11-19T03:52:50Z","receivedAt":"2024-11-19T03:52:52Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Illia Bobyr writes:\n\n> Also,`diffcore` docs says:\n>\n>> \"-G\" (mnemonic: grep)\n>\n> I was thinking of `--pickaxe` for `-S` and `--grep` for `-G`.\n\nIn the context of \"git diff\", calling \"-G\" \"--grep\" would be OK, but\nin the context of \"git log\", there is \"--grep\" already, so that\nwon't fly.\n\n> `--pickaxe-grep` for `-G` seems like a reasonable alternative name for `-G`.\n\nThat is probably OK (even though \"-G\" is not exactly what the\npickaxe machinery wants to do; \"--grep-in-patch\" might be closer to\nthe intent).\n\n> Not sure what would be a reasonably short alternative for `-S`.\n> `--pickaxe-occurance-change` seems too long, and might not be as clear.\n> `--pickaxe-occurance-count-change` is just way too long.\n\nGiving a tool a meaningful name is an excellent idea. If the\nmeaningful name guides users to the right way to use the tool,\nit would be ideal. Which means that to name it right, you'd need to\nknow what it exactly is for.\n\nThe -S feature was written to become one of the building blocks of\nLinus's \"clearly superior algorithm\", described in [1]. Linus talks\nabout \"where did this _line_ come from?\", but the algorithm is more\ngenerally about a block of code. The expected use case is for -S to\nbe fed sufficiently unique block of text so that we can efficiently\ndetect the transition of occurence count from 1 (because wee start\nfrom sufficiently unique block of code) down to 0 (which is the\nboundary in history where the block of code was first introduced in\nits current form). It detects any occurence count change, but its\nprimary focus is to find a transition from 1 to 0 (when going\nbackwards in history). Its spirit is more about \"finding where it\nappeared in its current shape\".\n\n\n[Footnote]\n\n*1* https://lore.kernel.org/git/Pine.LNX.4.58.0504150753440.7211@ppc970.osdl.org/\n"},{"id":"507638","messageId":"20241119185817.GC15723@coredump.intra.peff.net","threadId":"62511","inReplyTo":"xmqqo72bev71.fsf@gitster.g","subject":"Re: Long names for `git log -S` and `git log -G`","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-11-19T18:58:17Z","receivedAt":"2024-11-19T18:58:19Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Nov 19, 2024 at 12:52:50PM +0900, Junio C Hamano wrote:\n\n> \n> > `--pickaxe-grep` for `-G` seems like a reasonable alternative name for `-G`.\n> \n> That is probably OK (even though \"-G\" is not exactly what the\n> pickaxe machinery wants to do; \"--grep-in-patch\" might be closer to\n> the intent).\n\nFWIW, I like --grep-in-patch. Saying just \"--pickaxe-grep\" does not\nhighlight that it is about looking in the patch. I.e., it is not clear\nfrom the name that is different from \"-Sfoo --pickaxe-regex\".\n\n> The -S feature was written to become one of the building blocks of\n> Linus's \"clearly superior algorithm\", described in [1]. Linus talks\n> about \"where did this _line_ come from?\", but the algorithm is more\n> generally about a block of code. The expected use case is for -S to\n> be fed sufficiently unique block of text so that we can efficiently\n> detect the transition of occurence count from 1 (because wee start\n> from sufficiently unique block of code) down to 0 (which is the\n> boundary in history where the block of code was first introduced in\n> its current form). It detects any occurence count change, but its\n> primary focus is to find a transition from 1 to 0 (when going\n> backwards in history). Its spirit is more about \"finding where it\n> appeared in its current shape\".\n\nHeh. I do not disagree that was the original focus, but I find that I\nuse \"-S\" most frequently to find mentions of a particular function or\nother token across the code base. E.g., understanding the historical\nuses of a function, or why it has no callers anymore, and so on. So I am\noften looking for many appearances or disappearances (though of course\nthey may or may not happen in a single file).\n\n-Peff\n"},{"id":"507865","messageId":"470fe577-b26d-4393-8fa6-8f73ca4302de@gmail.com","threadId":"62511","inReplyTo":"20241119185817.GC15723@coredump.intra.peff.net","subject":"Re: Long names for `git log -S` and `git log -G`","fromName":"Illia Bobyr","fromEmail":"illia.bobyr@gmail.com","sentAt":"2024-11-21T23:31:00Z","receivedAt":"2024-11-21T23:31:02Z","isPatch":false,"sender":{"key":"illia.bobyr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/694419?v=4"},"body":"On 11/18/24 19:52, Junio C Hamano wrote:\n>> `--pickaxe-grep` for `-G` seems like a reasonable alternative name for `-G`.\n> That is probably OK (even though \"-G\" is not exactly what the\n> pickaxe machinery wants to do; \"--grep-in-patch\" might be closer to\n> the intent).\nImagining, that I am starting from scratch for this functionality, I think I\nwould also consider \"patch\".  Though, as we have 4 related argument names, I\nwonder if using it as a prefix would create a more consistent UX.\n\nSomething like:\n\n\"--patch-grep\" for \"-G\"\n\"--patch-modifies\" for \"-S\"\n\"--patch-search-show-all\"/\"--patch-show-all\" for \"--pickaxe-all\"\n\"--patch-search-regex\"/\"--patch-regex\" for \"--pickaxe-regex\"\n\nI'm not too sure about the later two, and also they already have long names.\nBut it seems quite easy to understand what this command would do:\n\n   git log --patch-grep sometext ...\n\nOr this\n\n   git log --patch-modifies function_call --patch-search-regex ...\n\n>> Not sure what would be a reasonably short alternative for `-S`.\n>> `--pickaxe-occurance-change` seems too long, and might not be as clear.\n>> `--pickaxe-occurance-count-change` is just way too long.\n> Giving a tool a meaningful name is an excellent idea. If the\n> meaningful name guides users to the right way to use the tool,\n> it would be ideal. Which means that to name it right, you'd need to\n> know what it exactly is for.\n\nGlad we are on the same page here :)\nI would really appreciate yours and Jeff input.\nI wrote a simple patch to see how much work is it to add long names [1].\nBut I would change it based on whatever names are agreed upon.\n\n[1]: \nhttps://lore.kernel.org/git/20241119032755.3360365-1-illia.bobyr@gmail.com/\n\n> The -S feature was written to become one of the building blocks of\n> Linus's \"clearly superior algorithm\", described in [1]. Linus talks\n> about \"where did this _line_ come from?\", but the algorithm is more\n> generally about a block of code. The expected use case is for -S to\n> be fed sufficiently unique block of text so that we can efficiently\n> detect the transition of occurence count from 1 (because wee start\n> from sufficiently unique block of code) down to 0 (which is the\n> boundary in history where the block of code was first introduced in\n> its current form). It detects any occurence count change, but its\n> primary focus is to find a transition from 1 to 0 (when going\n> backwards in history). Its spirit is more about \"finding where it\n> appeared in its current shape\".\n>\n>\n> [Footnote]\n>\n> *1* https://lore.kernel.org/git/Pine.LNX.4.58.0504150753440.7211@ppc970.osdl.org/\n\nThis is pretty interesting.  Thank you for pointing it out.  I guess, it \nmeans\nthat the \"counting\" in the porposed long name of \"-S\" arguments is more \nof an\nimplementation detail than the actually intended functionality.  Do you \nthink\nthere is a word that better reflects the intent here? \"--patch-modifies\" is\nprobably also not really hitting it 100%.\n\nOn 11/19/24 10:58, Jeff King wrote:\n> On Tue, Nov 19, 2024 at 12:52:50PM +0900, Junio C Hamano wrote:\n>>> `--pickaxe-grep` for `-G` seems like a reasonable alternative name for `-G`.\n>> That is probably OK (even though \"-G\" is not exactly what the\n>> pickaxe machinery wants to do; \"--grep-in-patch\" might be closer to\n>> the intent).\n> FWIW, I like --grep-in-patch. Saying just \"--pickaxe-grep\" does not\n> highlight that it is about looking in the patch. I.e., it is not clear\n> from the name that is different from \"-Sfoo --pickaxe-regex\".\n\nI agree.  Do you think \"--patch-grep\" is a better name?  Or do you think \nthat\nthe grep and the occurrence counting functionality should not share a common\nprefix, as they are somewhat different in nature?\n\n\"git log\" docs talk about both \"-S\" and \"-G\" as if they are pretty \nclose.  There\nis a note in \"git diffcore\" doc, \"diff-pickaxe\" section that says that \ngrep is\nactually quite different.  I wonder if this is important for the users \nto think\nabout.  I often use \"-G\" followed by \"-S\" or the other way around, just \nto see\nwhich view is better for my particular problem.\n\n"},{"id":"507907","messageId":"xmqqserjsfrq.fsf@gitster.g","threadId":"62511","inReplyTo":"470fe577-b26d-4393-8fa6-8f73ca4302de@gmail.com","subject":"Re: Long names for `git log -S` and `git log -G`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-11-22T10:51:21Z","receivedAt":"2024-11-22T10:51:24Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Illia Bobyr writes:\n\n> On 11/18/24 19:52, Junio C Hamano wrote:\n>>> `--pickaxe-grep` for `-G` seems like a reasonable alternative name for `-G`.\n>> That is probably OK (even though \"-G\" is not exactly what the\n>> pickaxe machinery wants to do; \"--grep-in-patch\" might be closer to\n>> the intent).\n> Imagining, that I am starting from scratch for this functionality, I think I\n> would also consider \"patch\".  Though, as we have 4 related argument names, I\n> wonder if using it as a prefix would create a more consistent UX.\n>\n> Something like:\n>\n> \"--patch-grep\" for \"-G\"\n> \"--patch-modifies\" for \"-S\"\n\nAhh, \"modifies\" is a great verb. It sounds quite logical, but \"-S\"\ndoes not have to genereate a patch internally for it to work, so\n\"--patch-modifies\" is a bit of white lie.\n\n> \"--patch-search-show-all\"/\"--patch-show-all\" for \"--pickaxe-all\"\n> \"--patch-search-regex\"/\"--patch-regex\" for \"--pickaxe-regex\"\n\nThese already have their own established long names, so it is\noutside the scope of this topic, and I doubt it is worth giving\nthese additional aliases (as you seem to agree).\n"},{"id":"511844","messageId":"20250205022422.2019929-1-illia.bobyr@gmail.com","threadId":"62511","inReplyTo":"xmqqserjsfrq.fsf@gitster.g","subject":"[PATCH v2 0/1] Long names for `git log -S` and `git log -G`","fromName":"Illia Bobyr","fromEmail":"illia.bobyr@gmail.com","sentAt":"2025-02-05T02:24:19Z","receivedAt":"2025-02-05T02:24:46Z","isPatch":true,"sender":{"key":"illia.bobyr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/694419?v=4"},"body":"Sorry, I dropped the ball on this.\nI still think it would be good to get it done.\n\n> On 11/22/24 02:51, Junio C Hamano wrote:\n> >>> `--pickaxe-grep` for `-G` seems like a reasonable alternative name for `-G`.\n> >> That is probably OK (even though \"-G\" is not exactly what the\n> >> pickaxe machinery wants to do; \"--grep-in-patch\" might be closer to\n> >> the intent).\n> > Imagining, that I am starting from scratch for this functionality, I think I\n> > would also consider \"patch\". Though, as we have 4 related argument names, I\n> > wonder if using it as a prefix would create a more consistent UX.\n> >\n> > Something like:\n> >\n> > \"--patch-grep\" for \"-G\"\n> > \"--patch-modifies\" for \"-S\"\n>\n> Ahh, \"modifies\" is a great verb. It sounds quite logical, but \"-S\"\n> does not have to generate a patch internally for it to work, so\n> \"--patch-modifies\" is a bit of white lie.\n\nUsed \"--patch-modifies\" for the second version of the patch.\n\nI think most people (certainly me) think about patches first, when they look at\nthese commands. They are unlikely to realize right away that generating a patch\nis more expensive than counting the string/regex occurrences in the pre- and\npost- images. So, if you consider the \"it behaves as if\" point of view rather\nthan \"this is how it works\" point of view, it is not even a lie :)\n\n> > \"--patch-search-show-all\"/\"--patch-show-all\" for \"--pickaxe-all\"\n> > \"--patch-search-regex\"/\"--patch-regex\" for \"--pickaxe-regex\"\n>\n> These already have their own established long names, so it is\n> outside the scope of this topic, and I doubt it is worth giving\n> these additional aliases (as you seem to agree).\n\nI do not have a strong feeling on this one. If \"-S\" and/or \"-G\" would get names\nthat do not start with a \"--pickaxe\" it might be a bit confusing that the flags\nthat affect their behavior do have the \"--pickaxe\" prefix. If this is a valid\nconcern, I could probably create a separate patch to add alternative names.\n\n---\n\nI've updated the patch with the following names:\n\n\"--patch-grep\" for \"-G\"\n\"--patch-modifies\" for \"-S\"\n\nOn 11/19/24 10:58, Jeff King wrote:\n> FWIW, I like --grep-in-patch. Saying just \"--pickaxe-grep\" does not\n> highlight that it is about looking in the patch. I.e., it is not clear\n> from the name that is different from \"-Sfoo --pickaxe-regex\".\n\nIs \"--patch-grep\" a good alternative? I think, using the same prefix for a\nfunctionality that looks quite similar from the user standpoint (\"-G\" and \"-S\")\nseems nice. Using \"--grep-in-patch\" for \"-G\", \"--patch-modifies\" for \"-S\" and\n\"--pickaxe-regex\"/\"--pickaxe-all\" all at the same time seems less consistent,\nbut I can change it if you insist.\n\nIllia Bobyr (1):\n diff: --patch{-modifies,grep} arg names for -S and -G\n\n Documentation/diff-options.txt | 36 +++++------\n Documentation/git-blame.txt | 2 +-\n Documentation/gitdiffcore.txt | 48 ++++++++-------\n diff.c | 18 +++---\n diff.h | 11 +++-\n gitk-git/gitk | 10 +++-\n t/t4062-diff-pickaxe.sh | 8 +--\n t/t4209-log-pickaxe.sh | 106 +++++++++++++++++++++++----------\n 8 files changed, 151 insertions(+), 88 deletions(-)\n\n-- \n2.45.2\n\n"},{"id":"511845","messageId":"20250205022422.2019929-2-illia.bobyr@gmail.com","threadId":"62511","inReplyTo":"xmqqserjsfrq.fsf@gitster.g","subject":"[PATCH v2 1/1] diff: --patch{-modifies,grep} arg names for -S and -G","fromName":"Illia Bobyr","fromEmail":"illia.bobyr@gmail.com","sentAt":"2025-02-05T02:24:20Z","receivedAt":"2025-02-05T02:24:54Z","isPatch":true,"sender":{"key":"illia.bobyr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/694419?v=4"},"body":"Most arguments have both short and long versions. Long versions are\neasier to read, especially in scripts and command history.\n\nTests that check just the option parsing are duplicated to check both\nshort and long argument options. But more complex tests are updated to\nuse the long argument in order to improve the test readability.\nAssuming that the usage tests have already verified that both arguments\ninvoke the same underlying functionality.\n\nSigned-off-by: Illia Bobyr \n---\n Documentation/diff-options.txt | 36 +++++------\n Documentation/git-blame.txt | 2 +-\n Documentation/gitdiffcore.txt | 48 ++++++++-------\n diff.c | 18 +++---\n diff.h | 11 +++-\n gitk-git/gitk | 10 +++-\n t/t4062-diff-pickaxe.sh | 8 +--\n t/t4209-log-pickaxe.sh | 106 +++++++++++++++++++++++----------\n 8 files changed, 151 insertions(+), 88 deletions(-)\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 640eb..c9f7c 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -650,6 +650,7 @@ Note that not all diffs can feature all types. For instance, copied and\n renamed entries cannot appear if detection for those types is disabled.\n \n `-S`::\n+`--patch-modifies=`::\n \tLook for differences that change the number of occurrences of\n \tthe specified __ (i.e. addition/deletion) in a file.\n \tIntended for the scripter's use.\n@@ -657,18 +658,19 @@ renamed entries cannot appear if detection for those types is disabled.\n It is useful when you're looking for an exact block of code (like a\n struct), and want to know the history of that block since it first\n came into being: use the feature iteratively to feed the interesting\n-block in the preimage back into `-S`, and keep going until you get the\n-very first version of the block.\n+block in the preimage back into `--patch-modifies`, and keep going until\n+you get the very first version of the block.\n +\n Binary files are searched as well.\n \n `-G`::\n+`--patch-grep=`::\n \tLook for differences whose patch text contains added/removed\n \tlines that match __.\n +\n-To illustrate the difference between `-S` `--pickaxe-regex` and\n-`-G`, consider a commit with the following diff in the same\n-file:\n+To illustrate the difference between `--patch-modifies=\n+--pickaxe-regex` and `--patch-grep=`, consider a commit with the\n+following diff in the same file:\n +\n ----\n + return frotz(nitfol, two->ptr, 1, 0);\n@@ -676,9 +678,9 @@ file:\n - hit = frotz(nitfol, mf2.ptr, 1, 0);\n ----\n +\n-While `git log -G\"frotz\\(nitfol\"` will show this commit, `git log\n--S\"frotz\\(nitfol\" --pickaxe-regex` will not (because the number of\n-occurrences of that string did not change).\n+While `git log --patch-grep=\"frotz\\(nitfol\"` will show this commit, `git\n+log --patch-modifies=\"frotz\\(nitfol\" --pickaxe-regex` will not (because the\n+number of occurrences of that string did not change).\n +\n Unless `--text` is supplied patches of binary files without a textconv\n filter will be ignored.\n@@ -687,22 +689,22 @@ See the 'pickaxe' entry in linkgit:gitdiffcore[7] for more\n information.\n \n `--find-object=`::\n-\tLook for differences that change the number of occurrences of\n-\tthe specified object. Similar to `-S`, just the argument is different\n-\tin that it doesn't search for a specific string but for a specific\n-\tobject id.\n+\tLook for differences that change the number of occurrences of the\n+\tspecified object. Similar to `--patch-modifies`, just the argument\n+\tis different in that it doesn't search for a specific string but\n+\tfor a specific object id.\n +\n The object can be a blob or a submodule commit. It implies the `-t` option in\n `git-log` to also find trees.\n \n `--pickaxe-all`::\n-\tWhen `-S` or `-G` finds a change, show all the changes in that\n-\tchangeset, not just the files that contain the change\n-\tin __.\n+\tWhen `--patch-modifies` or `--patch-grep` finds a change, show all\n+\tthe changes in that changeset, not just the files that contain the\n+\tchange in __.\n \n `--pickaxe-regex`::\n-\tTreat the __ given to `-S` as an extended POSIX regular\n-\texpression to match.\n+\tTreat the __ given to `--patch-modifies` as an extended\n+\tPOSIX regular expression to match.\n \n endif::git-format-patch[]\n \ndiff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt\nindex b1d7fb..0f21d3 100644\n--- a/Documentation/git-blame.txt\n+++ b/Documentation/git-blame.txt\n@@ -41,7 +41,7 @@ a text string in the diff. A small example of the pickaxe interface\n that searches for `blame_usage`:\n \n -----------------------------------------------------------------------------\n-$ git log --pretty=oneline -S'blame_usage'\n+$ git log --pretty=oneline --patch-modifies='blame_usage'\n 5040f17eba15504bad66b14a645bddd9b015ebb7 blame -S \n ea4c7f9bf69e781dd0cd88d2bccb2bf5cc15c9a7 git-blame: Make the output\n -----------------------------------------------------------------------------\ndiff --git a/Documentation/gitdiffcore.txt b/Documentation/gitdiffcore.txt\nindex 642c5..e4b18 100644\n--- a/Documentation/gitdiffcore.txt\n+++ b/Documentation/gitdiffcore.txt\n@@ -245,33 +245,35 @@ diffcore-pickaxe: For Detecting Addition/Deletion of Specified String\n \n This transformation limits the set of filepairs to those that change\n specified strings between the preimage and the postimage in a certain\n-way. -S and -G options are used to\n-specify different ways these strings are sought.\n-\n-\"-S\" detects filepairs whose preimage and postimage\n-have different number of occurrences of the specified block of text.\n-By definition, it will not detect in-file moves. Also, when a\n-changeset moves a file wholesale without affecting the interesting\n-string, diffcore-rename kicks in as usual, and `-S` omits the filepair\n-(since the number of occurrences of that string didn't change in that\n+way. --patch-modifies= and\n+--patch-grep= options are used to specify\n+different ways these strings are sought.\n+\n+\"-S\", or \"--patch-modifies=\" detects\n+filepairs whose preimage and postimage have different number of\n+occurrences of the specified block of text. By definition, it will\n+not detect in-file moves. Also, when a changeset moves a file\n+wholesale without affecting the interesting string, diffcore-rename\n+kicks in as usual, and `--patch-modifies` omits the filepair (since\n+the number of occurrences of that string didn't change in that\n rename-detected filepair). When used with `--pickaxe-regex`, treat\n the as an extended POSIX regular expression to match,\n instead of a literal string.\n \n-\"-G\" (mnemonic: grep) detects filepairs whose\n-textual diff has an added or a deleted line that matches the given\n-regular expression. This means that it will detect in-file (or what\n-rename-detection considers the same file) moves, which is noise. The\n-implementation runs diff twice and greps, and this can be quite\n-expensive. To speed things up, binary files without textconv filters\n-will be ignored.\n-\n-When `-S` or `-G` are used without `--pickaxe-all`, only filepairs\n-that match their respective criterion are kept in the output. When\n-`--pickaxe-all` is used, if even one filepair matches their respective\n-criterion in a changeset, the entire changeset is kept. This behavior\n-is designed to make reviewing changes in the context of the whole\n-changeset easier.\n+\"-G\", or \"--patch-grep=\"\n+(mnemonic: grep) detects filepairs whose textual diff has an added or\n+a deleted line that matches the given regular expression. This means\n+that it will detect in-file (or what rename-detection considers the\n+same file) moves, which is noise. The implementation runs diff twice\n+and greps, and this can be quite expensive. To speed things up,\n+binary files without textconv filters will be ignored.\n+\n+When `--patch-modifies` or `--patch-grep` are used without\n+`--pickaxe-all`, only filepairs that match their respective criterion\n+are kept in the output. When `--pickaxe-all` is used, if even one\n+filepair matches their respective criterion in a changeset, the entire\n+changeset is kept. This behavior is designed to make reviewing\n+changes in the context of the whole changeset easier.\n \n diffcore-order: For Sorting the Output Based on Filenames\n ---------------------------------------------------------\ndiff --git a/diff.c b/diff.c\nindex d28b41..09beb 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -4877,15 +4877,17 @@ void diff_setup_done(struct diff_options *options)\n \n \tif (HAS_MULTI_BITS(options->pickaxe_opts & DIFF_PICKAXE_KINDS_MASK))\n \t\tdie(_(\"options '%s', '%s', and '%s' cannot be used together\"),\n-\t\t\t\"-G\", \"-S\", \"--find-object\");\n+\t\t\t\"-G/--patch-grep\", \"-S/--patch-modifies\", \"--find-object\");\n \n \tif (HAS_MULTI_BITS(options->pickaxe_opts & DIFF_PICKAXE_KINDS_G_REGEX_MASK))\n \t\tdie(_(\"options '%s' and '%s' cannot be used together, use '%s' with '%s'\"),\n-\t\t\t\"-G\", \"--pickaxe-regex\", \"--pickaxe-regex\", \"-S\");\n+\t\t\t\"-G/--patch-grep\", \"--pickaxe-regex\",\n+ \"--pickaxe-regex\", \"-S/--patch-modifies\");\n \n \tif (HAS_MULTI_BITS(options->pickaxe_opts & DIFF_PICKAXE_KINDS_ALL_OBJFIND_MASK))\n \t\tdie(_(\"options '%s' and '%s' cannot be used together, use '%s' with '%s' and '%s'\"),\n-\t\t\t\"--pickaxe-all\", \"--find-object\", \"--pickaxe-all\", \"-G\", \"-S\");\n+\t\t\t\"--pickaxe-all\", \"--find-object\",\n+ \"--pickaxe-all\", \"-G/--patch-grep\", \"-S/--patch-modifies\");\n \n \t/*\n \t * Most of the time we can say \"there are changes\"\n@@ -5862,17 +5864,17 @@ struct option *add_diff_options(const struct option *opts,\n \t\tOPT_SET_INT_F(0, \"ita-visible-in-index\", &options->ita_invisible_in_index,\n \t\t\t N_(\"treat 'git add -N' entries as real in the index\"),\n \t\t\t 0, PARSE_OPT_NONEG),\n-\t\tOPT_CALLBACK_F('S', NULL, options, N_(\"\"),\n+\t\tOPT_CALLBACK_F('S', \"patch-modifies\", options, N_(\"\"),\n \t\t\t N_(\"look for differences that change the number of occurrences of the specified string\"),\n \t\t\t 0, diff_opt_pickaxe_string),\n-\t\tOPT_CALLBACK_F('G', NULL, options, N_(\"\"),\n-\t\t\t N_(\"look for differences that change the number of occurrences of the specified regex\"),\n+\t\tOPT_CALLBACK_F('G', \"patch-grep\", options, N_(\"\"),\n+\t\t\t N_(\"look for differences where a patch contains the specified regex\"),\n \t\t\t 0, diff_opt_pickaxe_regex),\n \t\tOPT_BIT_F(0, \"pickaxe-all\", &options->pickaxe_opts,\n-\t\t\t N_(\"show all changes in the changeset with -S or -G\"),\n+\t\t\t N_(\"show all changes in the changeset with -S/--patch-modifies or -G/--patch-grep\"),\n \t\t\t DIFF_PICKAXE_ALL, PARSE_OPT_NONEG),\n \t\tOPT_BIT_F(0, \"pickaxe-regex\", &options->pickaxe_opts,\n-\t\t\t N_(\"treat in -S as extended POSIX regular expression\"),\n+\t\t\t N_(\"treat in -S/--patch-modifies as extended POSIX regular expression\"),\n \t\t\t DIFF_PICKAXE_REGEX, 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\")),\ndiff --git a/diff.h b/diff.h\nindex 6e6007..247ac 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -598,9 +598,16 @@ void diffcore_fix_diff_index(void);\n \" try unchanged files as candidate for copy detection.\\n\" \\\n \" -l limit rename attempts up to paths.\\n\" \\\n \" -O reorder diffs according to the .\\n\" \\\n-\" -S find filepair whose only one side contains the string.\\n\" \\\n+\" -G\\n\" \\\n+\" --patch-grep=\\n\" \\\n+\" find differences whose patch contains the regex.\\n\" \\\n+\" -S\\n\" \\\n+\" --patch-modifies=\\n\" \\\n+\" find filepair who differ in the number of occurrences of string.\\n\" \\\n+\" --pickaxe-grep\\n\" \\\n+\" treat as regex in the -S/--patch-modifies argument.\\n\" \\\n \" --pickaxe-all\\n\" \\\n-\" show all files diff when -S is used and hit is found.\\n\" \\\n+\" show all files diff for -G/--patch-grep and -S/--patch-modifies.\\n\" \\\n \" -a --text treat all files as text.\\n\"\n \n int diff_queue_is_empty(struct diff_options *o);\ndiff --git a/gitk-git/gitk b/gitk-git/gitk\nindex 47a7c1..52516 100755\n--- a/gitk-git/gitk\n+++ b/gitk-git/gitk\n@@ -228,7 +228,15 @@ proc parseviewargs {n arglist} {\n \"--until=*\" - \"--before=*\" - \"--max-age=*\" - \"--min-age=*\" -\n \"--author=*\" - \"--committer=*\" - \"--grep=*\" - \"-[iE]\" -\n \"--remove-empty\" - \"--first-parent\" - \"--cherry-pick\" -\n- \"-S*\" - \"-G*\" - \"--pickaxe-all\" - \"--pickaxe-regex\" -\n+ \"-S*\" - \"--patch-modifies=*\" -\n+ \"--patch-modifies\" {\n+ set nextisval 1\n+ }\n+ \"-G*\" - \"--patch-grep=*\" -\n+ \"--patch-grep\" {\n+ set nextisval 1\n+ }\n+ \"--pickaxe-all\" - \"--pickaxe-regex\" -\n \"--simplify-by-decoration\" {\n # These mean that we get a subset of the commits\n set filtered 1\ndiff --git a/t/t4062-diff-pickaxe.sh b/t/t4062-diff-pickaxe.sh\nindex 8ad3d7..805e0f 100755\n--- a/t/t4062-diff-pickaxe.sh\n+++ b/t/t4062-diff-pickaxe.sh\n@@ -16,13 +16,13 @@ test_expect_success setup '\n '\n \n # OpenBSD only supports up to 255 repetitions, so repeat twice for 64*64=4096.\n-test_expect_success '-G matches' '\n-\tgit diff --name-only -G \"^(0{64}){64}$\" HEAD^ >out &&\n+test_expect_success '--patch-grep matches' '\n+\tgit diff --name-only --patch-grep \"^(0{64}){64}$\" HEAD^ >out &&\n \ttest 4096-zeroes.txt = \"$(cat out)\"\n '\n \n-test_expect_success '-S --pickaxe-regex' '\n-\tgit diff --name-only -S0 --pickaxe-regex HEAD^ >out &&\n+test_expect_success '--patch-modifies --pickaxe-regex' '\n+\tgit diff --name-only --patch-modifies 0 --pickaxe-regex HEAD^ >out &&\n \ttest 4096-zeroes.txt = \"$(cat out)\"\n '\n \ndiff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh\nindex a675ac..5f4d6 100755\n--- a/t/t4209-log-pickaxe.sh\n+++ b/t/t4209-log-pickaxe.sh\n@@ -1,6 +1,6 @@\n #!/bin/sh\n \n-test_description='log --grep/--author/--regexp-ignore-case/-S/-G'\n+test_description='log --grep/--author/--regexp-ignore-case/--patch-{modifies,grep}'\n \n . ./test-lib.sh\n \n@@ -60,24 +60,48 @@ test_expect_success 'usage' '\n \ttest_expect_code 129 git log -S 2>err &&\n \ttest_grep \"switch.*requires a value\" err &&\n \n+\ttest_expect_code 129 git log --patch-modifies 2>err &&\n+\ttest_grep \"option.*requires a value\" err &&\n+\n \ttest_expect_code 129 git log -G 2>err &&\n \ttest_grep \"switch.*requires a value\" err &&\n \n+\ttest_expect_code 129 git log --patch-grep 2>err &&\n+\ttest_grep \"option.*requires a value\" err &&\n+\n \ttest_expect_code 128 git log -Gregex -Sstring 2>err &&\n \tgrep \"cannot be used together\" err &&\n \n+\ttest_expect_code 128 git log -Gregex --patch-modifies string 2>err &&\n+\tgrep \"cannot be used together\" err &&\n+\n+\ttest_expect_code 128 git log --patch-grep regex -Sstring 2>err &&\n+\tgrep \"cannot be used together\" err &&\n+\n+\ttest_expect_code 128 git log --patch-grep regex --patch-modifies string 2>err &&\n+\tgrep \"cannot be used together\" err &&\n+\n \ttest_expect_code 128 git log -Gregex --find-object=HEAD 2>err &&\n \tgrep \"cannot be used together\" err &&\n \n+\ttest_expect_code 128 git log --patch-grep regex --find-object=HEAD 2>err &&\n+\tgrep \"cannot be used together\" err &&\n+\n \ttest_expect_code 128 git log -Sstring --find-object=HEAD 2>err &&\n \tgrep \"cannot be used together\" err &&\n \n+\ttest_expect_code 128 git log --patch-modifies string --find-object=HEAD 2>err &&\n+\tgrep \"cannot be used together\" err &&\n+\n \ttest_expect_code 128 git log --pickaxe-all --find-object=HEAD 2>err &&\n \tgrep \"cannot be used together\" err\n '\n \n test_expect_success 'usage: --pickaxe-regex' '\n \ttest_expect_code 128 git log -Gregex --pickaxe-regex 2>err &&\n+\tgrep \"cannot be used together\" err &&\n+\n+\ttest_expect_code 128 git log --patch-grep regex --pickaxe-regex 2>err &&\n \tgrep \"cannot be used together\" err\n '\n \n@@ -89,7 +113,13 @@ test_expect_success 'usage: --no-pickaxe-regex' '\n \ttest_expect_code 128 git log -Sstring --no-pickaxe-regex 2>actual &&\n \ttest_cmp expect actual &&\n \n-\ttest_expect_code 128 git log -Gstring --no-pickaxe-regex 2>err &&\n+\ttest_expect_code 128 git log --patch-modifies string --no-pickaxe-regex 2>actual &&\n+\ttest_cmp expect actual &&\n+\n+\ttest_expect_code 128 git log -Gregex --no-pickaxe-regex 2>err &&\n+\ttest_cmp expect actual &&\n+\n+\ttest_expect_code 128 git log --patch-grep regex --no-pickaxe-regex 2>err &&\n \ttest_cmp expect actual\n '\n \n@@ -104,47 +134,59 @@ test_log_icase\texpect_second\t--author person\n test_log_icase\texpect_nomatch\t--author spreon\n \n test_log\texpect_nomatch\t-G picked\n+test_log\texpect_nomatch\t--patch-grep picked\n test_log\texpect_second\t-G Picked\n+test_log\texpect_second\t--patch-grep Picked\n test_log_icase\texpect_nomatch\t-G pickle\n+test_log_icase\texpect_nomatch\t--patch-grep pickle\n test_log_icase\texpect_second\t-G picked\n+test_log_icase\texpect_second\t--patch-grep picked\n \n-test_expect_success 'log -G --textconv (missing textconv tool)' '\n+test_expect_success 'log --patch-grep --textconv (missing textconv tool)' '\n \techo \"* diff=test\" >.gitattributes &&\n-\ttest_must_fail git -c diff.test.textconv=missing log -Gfoo &&\n+\ttest_must_fail git -c diff.test.textconv=missing log --patch-grep foo &&\n \trm .gitattributes\n '\n \n-test_expect_success 'log -G --no-textconv (missing textconv tool)' '\n+test_expect_success 'log --patch-grep --no-textconv (missing textconv tool)' '\n \techo \"* diff=test\" >.gitattributes &&\n-\tgit -c diff.test.textconv=missing log -Gfoo --no-textconv >actual &&\n+\tgit -c diff.test.textconv=missing log --patch-grep foo --no-textconv >actual &&\n \ttest_cmp expect_nomatch actual &&\n \trm .gitattributes\n '\n \n test_log\texpect_nomatch\t-S picked\n+test_log\texpect_nomatch\t--patch-modifies picked\n test_log\texpect_second\t-S Picked\n+test_log\texpect_second\t--patch-modifies Picked\n test_log_icase\texpect_second\t-S picked\n+test_log_icase\texpect_second\t--patch-modifies picked\n test_log_icase\texpect_nomatch\t-S pickle\n+test_log_icase\texpect_nomatch\t--patch-modifies pickle\n \n test_log\texpect_nomatch\t-S p.cked --pickaxe-regex\n+test_log\texpect_nomatch\t--patch-modifies p.cked --pickaxe-regex\n test_log\texpect_second\t-S P.cked --pickaxe-regex\n+test_log\texpect_second\t--patch-modifies P.cked --pickaxe-regex\n test_log_icase\texpect_second\t-S p.cked --pickaxe-regex\n+test_log_icase\texpect_second\t--patch-modifies p.cked --pickaxe-regex\n test_log_icase\texpect_nomatch\t-S p.ckle --pickaxe-regex\n+test_log_icase\texpect_nomatch\t--patch-modifies p.ckle --pickaxe-regex\n \n-test_expect_success 'log -S --textconv (missing textconv tool)' '\n+test_expect_success 'log --patch-modifies --textconv (missing textconv tool)' '\n \techo \"* diff=test\" >.gitattributes &&\n-\ttest_must_fail git -c diff.test.textconv=missing log -Sfoo &&\n+\ttest_must_fail git -c diff.test.textconv=missing log --patch-modifies foo &&\n \trm .gitattributes\n '\n \n-test_expect_success 'log -S --no-textconv (missing textconv tool)' '\n+test_expect_success 'log --patch-modifies --no-textconv (missing textconv tool)' '\n \techo \"* diff=test\" >.gitattributes &&\n-\tgit -c diff.test.textconv=missing log -Sfoo --no-textconv >actual &&\n+\tgit -c diff.test.textconv=missing log --patch-modifies foo --no-textconv >actual &&\n \ttest_cmp expect_nomatch actual &&\n \trm .gitattributes\n '\n \n-test_expect_success 'setup log -[GS] plain & regex' '\n+test_expect_success 'setup log --patch{-modifies,-grep} plain & regex' '\n \ttest_create_repo GS-plain &&\n \ttest_commit -C GS-plain --append A data.txt \"a\" &&\n \ttest_commit -C GS-plain --append B data.txt \"a a\" &&\n@@ -159,31 +201,31 @@ test_expect_success 'setup log -[GS] plain & regex' '\n \tgit -C GS-plain log >full-log\n '\n \n-test_expect_success 'log -G trims diff new/old [-+]' '\n-\tgit -C GS-plain log -G\"[+-]a\" >log &&\n+test_expect_success 'log --patch-grep trims diff new/old [-+]' '\n+\tgit -C GS-plain log --patch-grep \"[+-]a\" >log &&\n \ttest_must_be_empty log &&\n-\tgit -C GS-plain log -G\"^a\" >log &&\n+\tgit -C GS-plain log --patch-grep \"^a\" >log &&\n \ttest_cmp log A-to-B-then-E-log\n '\n \n-test_expect_success 'log -S is not a regex, but -S --pickaxe-regex is' '\n-\tgit -C GS-plain log -S\"a\" >log &&\n+test_expect_success 'log --patch-modifies is not a regex, but --patch-modifies --pickaxe-regex is' '\n+\tgit -C GS-plain log --patch-modifies \"a\" >log &&\n \ttest_cmp log A-to-B-then-E-log &&\n \n-\tgit -C GS-plain log -S\"[a]\" >log &&\n+\tgit -C GS-plain log --patch-modifies \"[a]\" >log &&\n \ttest_must_be_empty log &&\n \n-\tgit -C GS-plain log -S\"[a]\" --pickaxe-regex >log &&\n+\tgit -C GS-plain log --patch-modifies \"[a]\" --pickaxe-regex >log &&\n \ttest_cmp log A-to-B-then-E-log &&\n \n-\tgit -C GS-plain log -S\"[b]\" >log &&\n+\tgit -C GS-plain log --patch-modifies \"[b]\" >log &&\n \ttest_cmp log D-then-E-log &&\n \n-\tgit -C GS-plain log -S\"[b]\" --pickaxe-regex >log &&\n+\tgit -C GS-plain log --patch-modifies \"[b]\" --pickaxe-regex >log &&\n \ttest_cmp log C-to-D-then-E-log\n '\n \n-test_expect_success 'setup log -[GS] binary & --text' '\n+test_expect_success 'setup log --patch{-modifies,-grep} binary & --text' '\n \ttest_create_repo GS-bin-txt &&\n \ttest_commit -C GS-bin-txt --printf A data.bin \"a\\na\\0a\\n\" &&\n \ttest_commit -C GS-bin-txt --append --printf B data.bin \"a\\na\\0a\\n\" &&\n@@ -191,36 +233,36 @@ test_expect_success 'setup log -[GS] binary & --text' '\n \tgit -C GS-bin-txt log >full-log\n '\n \n-test_expect_success 'log -G ignores binary files' '\n-\tgit -C GS-bin-txt log -Ga >log &&\n+test_expect_success 'log --patch-grep ignores binary files' '\n+\tgit -C GS-bin-txt log --patch-grep a >log &&\n \ttest_must_be_empty log\n '\n \n-test_expect_success 'log -G looks into binary files with -a' '\n-\tgit -C GS-bin-txt log -a -Ga >log &&\n+test_expect_success 'log --patch-grep looks into binary files with -a' '\n+\tgit -C GS-bin-txt log -a --patch-grep a >log &&\n \ttest_cmp log full-log\n '\n \n-test_expect_success 'log -G looks into binary files with textconv filter' '\n+test_expect_success 'log --patch-grep looks into binary files with textconv filter' '\n \ttest_when_finished \"rm GS-bin-txt/.gitattributes\" &&\n \t(\n \t\tcd GS-bin-txt &&\n \t\techo \"* diff=bin\" >.gitattributes &&\n-\t\tgit -c diff.bin.textconv=cat log -Ga >../log\n+\t\tgit -c diff.bin.textconv=cat log --patch-grep a >../log\n \t) &&\n \ttest_cmp log full-log\n '\n \n-test_expect_success 'log -S looks into binary files' '\n-\tgit -C GS-bin-txt log -Sa >log &&\n+test_expect_success 'log --patch-modifies looks into binary files' '\n+\tgit -C GS-bin-txt log --patch-modifies a >log &&\n \ttest_cmp log full-log\n '\n \n-test_expect_success 'log -S --pickaxe-regex looks into binary files' '\n-\tgit -C GS-bin-txt log --pickaxe-regex -Sa >log &&\n+test_expect_success 'log --patch-modifies --pickaxe-regex looks into binary files' '\n+\tgit -C GS-bin-txt log --pickaxe-regex --patch-modifies a >log &&\n \ttest_cmp log full-log &&\n \n-\tgit -C GS-bin-txt log --pickaxe-regex -S\"[a]\" >log &&\n+\tgit -C GS-bin-txt log --pickaxe-regex --patch-modifies \"[a]\" >log &&\n \ttest_cmp log full-log\n '\n \n-- \n2.45.2\n\n"},{"id":"511855","messageId":"b2038430-62dc-41fa-86c2-c0a14bd25e0f@kdbg.org","threadId":"62511","inReplyTo":"20250205022422.2019929-2-illia.bobyr@gmail.com","subject":"Re: [PATCH v2 1/1] diff: --patch{-modifies,grep} arg names for -S and -G","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2025-02-05T07:15:09Z","receivedAt":"2025-02-05T07:15:25Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 05.02.25 um 03:24 schrieb Illia Bobyr:\n> diff --git a/gitk-git/gitk b/gitk-git/gitk\n> index 47a7c1..52516 100755\n> --- a/gitk-git/gitk\n> +++ b/gitk-git/gitk\n\nPlease exclude this part related to gitk from this submission and send a\nfollow-up patch with only this part. The subject line should then start\nwith \"gitk:\".\n\n-- Hannes\n\n"}]}