{"thread":{"id":"33818","subject":"[PATCH] diffcore-pickaxe doc: document -S and -G properly","startedAt":"2013-05-14T14:12:39Z","lastAt":"2013-05-14T21:13:08Z","messageCount":20,"participants":["Ramkumar Ramachandra","Junio C Hamano","Phil Hord","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"217327","messageId":"1368540759-25598-1-git-send-email-artagnon@gmail.com","threadId":"33818","inReplyTo":null,"subject":"[PATCH] diffcore-pickaxe doc: document -S and -G properly","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-14T14:12:39Z","receivedAt":"2013-05-14T14:12:39Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"The documentation of -S and -G is very sketchy. Completely rewrite the\nsections in Documentation/diff-options.txt and\nDocumentation/gitdiffcore.txt.\n\nReferences:\n52e9578 ([PATCH] Introducing software archaeologist's tool \"pickaxe\".)\nf506b8e (git log/diff: add -G that greps in the patch text)\n\nSigned-off-by: Ramkumar Ramachandra \n---\n I spent some time reading the code and history to figure out what -S\n and -G really do. I hope I've done justice.\n\n Documentation/diff-options.txt | 35 +++++++++++++++++++++++++++-------\n Documentation/gitdiffcore.txt | 43 +++++++++++++++++++++++-------------------\n 2 files changed, 52 insertions(+), 26 deletions(-)\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 104579d..765abc5 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -383,14 +383,35 @@ ifndef::git-format-patch[]\n \tthat matches other criteria, nothing is selected.\n \n -S::\n-\tLook for differences that introduce or remove an instance of\n-\t. Note that this is different than the string simply\n-\tappearing in diff output; see the 'pickaxe' entry in\n-\tlinkgit:gitdiffcore[7] for more details.\n+\tLook for commits where the specified string was added or\n+\tremoved. More precisely, find commits that change the number\n+\tof occurrences of the specified string.\n++\n+It is often useful when you're looking for an exact string (like a\n+function prototype), and want to know the history of that string since\n+it first came into being.\n \n -G::\n-\tLook for differences whose added or removed line matches\n-\tthe given .\n+\tGrep through the patch text of commits for added/removed lines\n+\tthat match . `--pickaxe-regex` is implied in this\n+\tmode.\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++\n+----\n++ return !regexec(regexp, two->ptr, 1, ®match, 0);\n+...\n+- hit = !regexec(regexp, mf2.ptr, 1, ®match, 0);\n+----\n++\n+While `git log -G\"regexec\\(regexp\"` will show this commit, `git log\n+-S\"regexec\\(regexp\" --pickaxe-regex` will not (because the number of\n+occurrences of that string didn't change).\n++\n+See the 'pickaxe' entry in linkgit:gitdiffcore[7] for more\n+information.\n \n --pickaxe-all::\n \tWhen `-S` or `-G` finds a change, show all the changes in that\n@@ -399,7 +420,7 @@ ifndef::git-format-patch[]\n \n --pickaxe-regex::\n \tMake the not a plain string but an extended POSIX\n-\tregex to match.\n+\tregex to match. Implied when using `-G`.\n endif::git-format-patch[]\n \n -O::\ndiff --git a/Documentation/gitdiffcore.txt b/Documentation/gitdiffcore.txt\nindex 568d757..39b9c51 100644\n--- a/Documentation/gitdiffcore.txt\n+++ b/Documentation/gitdiffcore.txt\n@@ -222,25 +222,30 @@ version prefixed with '+'.\n diffcore-pickaxe: For Detecting Addition/Deletion of Specified String\n ---------------------------------------------------------------------\n \n-This transformation is used to find filepairs that represent\n-changes that touch a specified string, and is controlled by the\n--S option and the `--pickaxe-all` option to the 'git diff-*'\n-commands.\n-\n-When diffcore-pickaxe is in use, it checks if there are\n-filepairs whose \"result\" side and whose \"origin\" side have\n-different number of specified string. Such a filepair represents\n-\"the string appeared in this changeset\". It also checks for the\n-opposite case that loses the specified string.\n-\n-When `--pickaxe-all` is not in effect, diffcore-pickaxe leaves\n-only such filepairs that touch the specified string in its\n-output. When `--pickaxe-all` is used, diffcore-pickaxe leaves all\n-filepairs intact if there is such a filepair, or makes the\n-output empty otherwise. The latter behaviour is designed to\n-make reviewing of the changes in the context of the whole\n-changeset easier.\n-\n+There are two kinds of pickaxe: the S kind (corresponding to 'git log\n+-S') and the G kind (corresponding to 'git log -G').\n+\n+The S kind detects filepairs whose \"result\" side and \"origin\" side\n+have different number of occurrences of specified string. While\n+rename detection works as usual, 'git log -S' cannot omit commits\n+where a the small string being looked for is moved verbatim from one\n+file to another (since the number of occurrences of that string\n+changed in each of those two filepairs). The implementation\n+essentially runs a count, and is significantly cheaper than the G\n+kind.\n+\n+The G kind detects filepairs whose patch text has an added or a\n+deleted line that matches the given regexp. This means that it can\n+detect in-file (or what rename-detection considers the same file)\n+moves. The implementation of 'git log -G' runs diff twice and greps,\n+and this can be quite expensive.\n+\n+A diffcore-pickaxe option worth mentioning: `--pickaxe-all`. When not\n+in effect, diffcore-pickaxe leaves only such filepairs that touch the\n+specified string in its output. When in effect, diffcore-pickaxe\n+leaves all filepairs intact if there is such a filepair, or makes the\n+output empty otherwise. The latter behavior is designed to make\n+reviewing of the changes in the context of the whole changeset easier.\n \n diffcore-order: For Sorting the Output Based on Filenames\n ---------------------------------------------------------\n-- \n1.8.3.rc1.61.g2cacfff\n"},{"id":"217331","messageId":"7vwqr1jw67.fsf@alter.siamese.dyndns.org","threadId":"33818","inReplyTo":"1368540759-25598-1-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH] diffcore-pickaxe doc: document -S and -G properly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-14T17:41:52Z","receivedAt":"2013-05-14T17:41:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> -S::\n> -\tLook for differences that introduce or remove an instance of\n> -\t. Note that this is different than the string simply\n> -\tappearing in diff output; see the 'pickaxe' entry in\n> -\tlinkgit:gitdiffcore[7] for more details.\n> +\tLook for commits where the specified string was added or\n> +\tremoved. More precisely, find commits that change the number\n> +\tof occurrences of the specified string.\n\nAny time you say \"This means that\", \"More precisely\", etc. please\ncheck if you can rewrite it to lose everything before them (i.e. a\nvague sentence that needs to be clarified may not have to be there\nat all).\n\n> ++\n> +It is often useful when you're looking for an exact string (like a\n> +function prototype), and want to know the history of that string since\n> +it first came into being.\n\nI think you should remind that the most useful case (and indeed the\nintended one) is for \"an exact string\" to be a multi-line \"block of\ntext\". People often get a (wrong) impression from the word \"string\"\nthat it is meant to be used with a single-liner.\n"},{"id":"217332","messageId":"CABURp0r808KeSRwhgPw98vUC+JSErvSYHFyAvn-=8JhQzn8e1Q@mail.gmail.com","threadId":"33818","inReplyTo":"1368540759-25598-1-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH] diffcore-pickaxe doc: document -S and -G properly","fromName":"Phil Hord","fromEmail":"phil.hord@gmail.com","sentAt":"2013-05-14T17:44:58Z","receivedAt":"2013-05-14T17:44:58Z","isPatch":true,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"On Tue, May 14, 2013 at 10:12 AM, Ramkumar Ramachandra\n wrote:\n> The documentation of -S and -G is very sketchy. Completely rewrite the\n> sections in Documentation/diff-options.txt and\n> Documentation/gitdiffcore.txt.\n>\n> References:\n> 52e9578 ([PATCH] Introducing software archaeologist's tool \"pickaxe\".)\n> f506b8e (git log/diff: add -G that greps in the patch text)\n>\n> Signed-off-by: Ramkumar Ramachandra \n> ---\n> I spent some time reading the code and history to figure out what -S\n> and -G really do. I hope I've done justice.\n>\n> Documentation/diff-options.txt | 35 +++++++++++++++++++++++++++-------\n> Documentation/gitdiffcore.txt | 43 +++++++++++++++++++++++-------------------\n> 2 files changed, 52 insertions(+), 26 deletions(-)\n>\n> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\n> index 104579d..765abc5 100644\n> --- a/Documentation/diff-options.txt\n> +++ b/Documentation/diff-options.txt\n> @@ -383,14 +383,35 @@ ifndef::git-format-patch[]\n> that matches other criteria, nothing is selected.\n>\n> -S::\n> - Look for differences that introduce or remove an instance of\n> - . Note that this is different than the string simply\n> - appearing in diff output; see the 'pickaxe' entry in\n> - linkgit:gitdiffcore[7] for more details.\n> + Look for commits where the specified string was added or\n> + removed. More precisely, find commits that change the number\n> + of occurrences of the specified string.\n> ++\n> +It is often useful when you're looking for an exact string (like a\n> +function prototype), and want to know the history of that string since\n> +it first came into being.\n>\n> -G::\n\nIt looks like you have deleted the -S and -G references here. Am I\nreading this right?\n\n> - Look for differences whose added or removed line matches\n> - the given .\n> + Grep through the patch text of commits for added/removed lines\n> + that match . `--pickaxe-regex` is implied in this\n> + mode.\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> ++\n> +----\n> ++ return !regexec(regexp, two->ptr, 1, ®match, 0);\n> +...\n> +- hit = !regexec(regexp, mf2.ptr, 1, ®match, 0);\n> +----\n> ++\n> +While `git log -G\"regexec\\(regexp\"` will show this commit, `git log\n> +-S\"regexec\\(regexp\" --pickaxe-regex` will not (because the number of\n> +occurrences of that string didn't change).\n\nReferences to git-log seem out of place to me here in git-diffcore. I\nknow it's only an example, but it seems that Git normally describes\nthese 'reference selectors' more generically. The generic description\nmay be more confusing to new users, but this patch is not the place to\nconsider whether it should change.\n\n> ++\n> +See the 'pickaxe' entry in linkgit:gitdiffcore[7] for more\n> +information.\n>\n> --pickaxe-all::\n> When `-S` or `-G` finds a change, show all the changes in that\n> @@ -399,7 +420,7 @@ ifndef::git-format-patch[]\n>\n> --pickaxe-regex::\n> Make the not a plain string but an extended POSIX\n> - regex to match.\n> + regex to match. Implied when using `-G`.\n> endif::git-format-patch[]\n>\n> -O::\n> diff --git a/Documentation/gitdiffcore.txt b/Documentation/gitdiffcore.txt\n> index 568d757..39b9c51 100644\n> --- a/Documentation/gitdiffcore.txt\n> +++ b/Documentation/gitdiffcore.txt\n> @@ -222,25 +222,30 @@ version prefixed with '+'.\n> diffcore-pickaxe: For Detecting Addition/Deletion of Specified String\n> ---------------------------------------------------------------------\n>\n> -This transformation is used to find filepairs that represent\n> -changes that touch a specified string, and is controlled by the\n> --S option and the `--pickaxe-all` option to the 'git diff-*'\n> -commands.\n> -\n> -When diffcore-pickaxe is in use, it checks if there are\n> -filepairs whose \"result\" side and whose \"origin\" side have\n> -different number of specified string. Such a filepair represents\n> -\"the string appeared in this changeset\". It also checks for the\n> -opposite case that loses the specified string.\n> -\n> -When `--pickaxe-all` is not in effect, diffcore-pickaxe leaves\n> -only such filepairs that touch the specified string in its\n> -output. When `--pickaxe-all` is used, diffcore-pickaxe leaves all\n> -filepairs intact if there is such a filepair, or makes the\n> -output empty otherwise. The latter behaviour is designed to\n> -make reviewing of the changes in the context of the whole\n> -changeset easier.\n> -\n> +There are two kinds of pickaxe: the S kind (corresponding to 'git log\n> +-S') and the G kind (corresponding to 'git log -G').\n\nWhile the switches are called -S and -G, I do not think it is helpful\nto name the two pickaxe options as \"the S kind\" and \"the G kind\".\n\n> +\n> +The S kind detects filepairs whose \"result\" side and \"origin\" side\n> +have different number of occurrences of specified string. While\n> +rename detection works as usual, 'git log -S' cannot omit commits\n\nThe \"cannot omit\" feels like a confusing double-negative. How about\n\"includes\" instead?\n\n> +where a the small string being looked for is moved verbatim from one\n\ns/a the/the/\n\n> +file to another (since the number of occurrences of that string\n> +changed in each of those two filepairs). The implementation\n> +essentially runs a count, and is significantly cheaper than the G\n> +kind.\n> +\n> +The G kind detects filepairs whose patch text has an added or a\n> +deleted line that matches the given regexp. This means that it can\n> +detect in-file (or what rename-detection considers the same file)\n> +moves. The implementation of 'git log -G' runs diff twice and greps,\n> +and this can be quite expensive.\n> +\n> +A diffcore-pickaxe option worth mentioning: `--pickaxe-all`. When not\n\nIs it worth mentioning that something in the documentation is \"worth\nmentioning\"?\n\n> +in effect, diffcore-pickaxe leaves only such filepairs that touch the\n> +specified string in its output. When in effect, diffcore-pickaxe\n> +leaves all filepairs intact if there is such a filepair, or makes the\n> +output empty otherwise. The latter behavior is designed to make\n> +reviewing of the changes in the context of the whole changeset easier.\n\nI find this description (from the original code, not from this commit)\nsomewhat confusing. It is written from the implementer's POV. Does\nthis seem clearer to you?\n\n Normally the pickaxe options limit the diff output to those files which\n contained the changes being searched; that is, those files which\n had modifications including the search string. With the --pickaxe-all\n option, the diff of the entire commit will be shown including files\n which did not have modifications including the search string. This\n is designed to make it easier to review the changes in the context\n of the whole commit.\n\n>\n> diffcore-order: For Sorting the Output Based on Filenames\n> ---------------------------------------------------------\n> --\n> 1.8.3.rc1.61.g2cacfff\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n"},{"id":"217334","messageId":"CABURp0p+2XUtFGW+7hEzna9qvANZGxTRw0oULfo+iM2UFrwqSA@mail.gmail.com","threadId":"33818","inReplyTo":"CABURp0r808KeSRwhgPw98vUC+JSErvSYHFyAvn-=8JhQzn8e1Q@mail.gmail.com","subject":"Re: [PATCH] diffcore-pickaxe doc: document -S and -G properly","fromName":"Phil Hord","fromEmail":"phil.hord@gmail.com","sentAt":"2013-05-14T17:47:34Z","receivedAt":"2013-05-14T17:47:34Z","isPatch":true,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"On Tue, May 14, 2013 at 1:44 PM, Phil Hord wrote:\n> On Tue, May 14, 2013 at 10:12 AM, Ramkumar Ramachandra\n> wrote:\n>>\n>> -S::\n>> - Look for differences that introduce or remove an instance of\n>> - . Note that this is different than the string simply\n>> - appearing in diff output; see the 'pickaxe' entry in\n>> - linkgit:gitdiffcore[7] for more details.\n>> + Look for commits where the specified string was added or\n>> + removed. More precisely, find commits that change the number\n>> + of occurrences of the specified string.\n>> ++\n>> +It is often useful when you're looking for an exact string (like a\n>> +function prototype), and want to know the history of that string since\n>> +it first came into being.\n>>\n>> -G::\n>\n> It looks like you have deleted the -S and -G references here. Am I\n> reading this right?\n\nDoy! Yes, I was reading it wrong. Sorry for the noise there.\n\nPhil\n"},{"id":"217339","messageId":"CALkWK0kJ7aksRbtjOesEPvsdx6wZyxxdVHUm_F-jS+Nb3WLSxg@mail.gmail.com","threadId":"33818","inReplyTo":"7vwqr1jw67.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] diffcore-pickaxe doc: document -S and -G properly","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-14T18:20:42Z","receivedAt":"2013-05-14T18:20:42Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> Any time you say \"This means that\", \"More precisely\", etc. please\n> check if you can rewrite it to lose everything before them (i.e. a\n> vague sentence that needs to be clarified may not have to be there\n> at all).\n\nRight. I thought both are necessary in this case: the first sentence\ngives easy information to a first-timer. For someone who has played\nwith it a bit, and wants to know more: the second line.\n\n>> ++\n>> +It is often useful when you're looking for an exact string (like a\n>> +function prototype), and want to know the history of that string since\n>> +it first came into being.\n>\n> I think you should remind that the most useful case (and indeed the\n> intended one) is for \"an exact string\" to be a multi-line \"block of\n> text\". People often get a (wrong) impression from the word \"string\"\n> that it is meant to be used with a single-liner.\n\nYes, I've been meaning to discuss that. I've been having some trouble\nwith multi-line strings: zsh doesn't insert a TAB in the next line.\nThe workaround I have is to write a shell script and execute that.\nHow do you do it?\n"},{"id":"217341","messageId":"CALkWK0nR1=Pgv0AY78p7n17C-VVvbc6BfJ4y_Df1ciZoc-xR-Q@mail.gmail.com","threadId":"33818","inReplyTo":"CABURp0r808KeSRwhgPw98vUC+JSErvSYHFyAvn-=8JhQzn8e1Q@mail.gmail.com","subject":"Re: [PATCH] diffcore-pickaxe doc: document -S and -G properly","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-14T18:44:08Z","receivedAt":"2013-05-14T18:44:08Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Phil Hord wrote:\n> References to git-log seem out of place to me here in git-diffcore. I\n> know it's only an example, but it seems that Git normally describes\n> these 'reference selectors' more generically. The generic description\n> may be more confusing to new users, but this patch is not the place to\n> consider whether it should change.\n\nIt's not for new users at all. The most useful application of -S and\n-G is in log. The translation from a log -G to a diffcore -G is not\nobvious at all, and warrants an explanation.\n\nOh, and for the user. No user is going to type out `man gitdiffcore`\nout of the blue: she's most probably led there from log, and we're\nconnecting the dots for her.\n\n> While the switches are called -S and -G, I do not think it is helpful\n> to name the two pickaxe options as \"the S kind\" and \"the G kind\".\n\nHow do you describe something precisely without loss of meaning? You\nstop abstracting unnecessarily. Read the sources: you will literally\nsee DIFF_PICKAXE_KIND_G there.\n\n>> +\n>> +The S kind detects filepairs whose \"result\" side and \"origin\" side\n>> +have different number of occurrences of specified string. While\n>> +rename detection works as usual, 'git log -S' cannot omit commits\n>\n> The \"cannot omit\" feels like a confusing double-negative. How about\n> \"includes\" instead?\n\nIntended. Omission is expected.\n\n> Is it worth mentioning that something in the documentation is \"worth\n> mentioning\"?\n\nYou don't have to nitpick style. We can allow this much creative\nfreedom in documentation.\n\n>> +in effect, diffcore-pickaxe leaves only such filepairs that touch the\n>> +specified string in its output. When in effect, diffcore-pickaxe\n>> +leaves all filepairs intact if there is such a filepair, or makes the\n>> +output empty otherwise. The latter behavior is designed to make\n>> +reviewing of the changes in the context of the whole changeset easier.\n>\n> I find this description (from the original code, not from this commit)\n> somewhat confusing. It is written from the implementer's POV.\n\nI explained the entire -S and -G thing in terms of filepairs (and yes,\nthat's implementation detail). Why would I want to explain this in\nany other terms?\n\n> Does\n> this seem clearer to you?\n> [...]\n\n>From diff-options.txt (the more end-user side):\n\nWhen -S or -G finds a change, show all the changes in that changeset,\nnot just the files that contain the change in .\n\nNot clear enough?\n"},{"id":"217342","messageId":"7v8v3hjt57.fsf@alter.siamese.dyndns.org","threadId":"33818","inReplyTo":"CALkWK0kJ7aksRbtjOesEPvsdx6wZyxxdVHUm_F-jS+Nb3WLSxg@mail.gmail.com","subject":"Re: [PATCH] diffcore-pickaxe doc: document -S and -G properly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-14T18:47:16Z","receivedAt":"2013-05-14T18:47:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> Junio C Hamano wrote:\n>> Any time you say \"This means that\", \"More precisely\", etc. please\n>> check if you can rewrite it to lose everything before them (i.e. a\n>> vague sentence that needs to be clarified may not have to be there\n>> at all).\n>\n> Right. I thought both are necessary in this case: the first sentence\n> gives easy information to a first-timer. For someone who has played\n> with it a bit, and wants to know more: the second line.\n>\n>>> ++\n>>> +It is often useful when you're looking for an exact string (like a\n>>> +function prototype), and want to know the history of that string since\n>>> +it first came into being.\n>>\n>> I think you should remind that the most useful case (and indeed the\n>> intended one) is for \"an exact string\" to be a multi-line \"block of\n>> text\". People often get a (wrong) impression from the word \"string\"\n>> that it is meant to be used with a single-liner.\n>\n> Yes, I've been meaning to discuss that. I've been having some trouble\n> with multi-line strings: zsh doesn't insert a TAB in the next line.\n> The workaround I have is to write a shell script and execute that.\n> How do you do it?\n\nI do not use zsh but with bash+readline the old tradition lnext can\nbe used (see \"stty -a\" output and it typically is set to ^V), i.e.\n\\C-v followed by \\C-i should give you a literal HT.\n"},{"id":"217344","messageId":"CALkWK0na1pDi5DnKUcWRnt0v9iQmeaLdrs7uQMDZzCKAazaNFg@mail.gmail.com","threadId":"33818","inReplyTo":"7v8v3hjt57.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] diffcore-pickaxe doc: document -S and -G properly","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-14T18:57:15Z","receivedAt":"2013-05-14T18:57:15Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> I do not use zsh but with bash+readline the old tradition lnext can\n> be used (see \"stty -a\" output and it typically is set to ^V), i.e.\n> \\C-v followed by \\C-i should give you a literal HT.\n\nJust looked it up: zsh has quoted-insert (^V) after which I can TAB.\nStill doesn't solve my problem though: I don't type out structs; I\npaste them the X clipboard (Emacs). And that doesn't work either on\nbash or zsh.\n\nWhat can we do to improve the interface?\n"},{"id":"217343","messageId":"7v4ne5jsno.fsf@alter.siamese.dyndns.org","threadId":"33818","inReplyTo":"CABURp0r808KeSRwhgPw98vUC+JSErvSYHFyAvn-=8JhQzn8e1Q@mail.gmail.com","subject":"Re: [PATCH] diffcore-pickaxe doc: document -S and -G properly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-14T18:57:47Z","receivedAt":"2013-05-14T18:57:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phil Hord writes:\n\n> Normally the pickaxe options limit the diff output to those files which\n> contained the changes being searched; that is, those files which\n> had modifications including the search string. With the --pickaxe-all\n> option, the diff of the entire commit will be shown including files\n> which did not have modifications including the search string. This\n> is designed to make it easier to review the changes in the context\n> of the whole commit.\n\nI find this very readable, even though \"diff output\" might be\nsomewhat misleading (it is not \"output for the end user\", but is\n\"passing to the next stage in the pipeline\").\n"},{"id":"217346","messageId":"20130514191757.GP3657@google.com","threadId":"33818","inReplyTo":"CALkWK0na1pDi5DnKUcWRnt0v9iQmeaLdrs7uQMDZzCKAazaNFg@mail.gmail.com","subject":"Re: [PATCH] diffcore-pickaxe doc: document -S and -G properly","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-05-14T19:17:57Z","receivedAt":"2013-05-14T19:17:57Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> What can we do to improve the interface?\n\nWrite a better shell? Teach \"git gui blame\" to blame on arbitrary\nregions instead of single lines? I'm not sure what you're asking,\nmostly because I'm not sure who \"we\" is.\n"},{"id":"217347","messageId":"7vppwtid3n.fsf@alter.siamese.dyndns.org","threadId":"33818","inReplyTo":"CALkWK0nR1=Pgv0AY78p7n17C-VVvbc6BfJ4y_Df1ciZoc-xR-Q@mail.gmail.com","subject":"Re: [PATCH] diffcore-pickaxe doc: document -S and -G properly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-14T19:19:08Z","receivedAt":"2013-05-14T19:19:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n>>> +The S kind detects filepairs whose \"result\" side and \"origin\" side\n>>> +have different number of occurrences of specified string. While\n>>> +rename detection works as usual, 'git log -S' cannot omit commits\n>>\n>> The \"cannot omit\" feels like a confusing double-negative. How about\n>> \"includes\" instead?\n>\n> Intended. Omission is expected.\n\nI think what makes this paragraph unnecessarily hard to read is the\n\"While rename works\".\n\nWith that, you are implying \"if you rename a file as a whole without\nchanging the block of text you identify with the -S parameter, then\nsuch a change is not interesting as far as pickaxe is concerned\".\nwhile that statement is logically correct, normal people are not\nthat generous to read that much between your lines.\n\nI think that is one of the reasons why \"If you moved a string from\nfile A to file B, log -S will flag that change as worth inspecting\"\ndoes not seem to logically follow and made Phil find your\ndescription confusing.\n\nFinding such a change indeed is a feature [*1*]; we need to flag\nsuch a change as worth inspecting to find where the code came from\nin order to dig deeper, so at least this \"cannot omit\" should be\n\"does not omit\".\n\n\n[Footnote]\n\n*1* I suspect that your confusion may stem from not understanding\n what pickaxe was invented for. It is _not_ about finding the\n final answer, but is about stopping at a commit that is worth\n investigating further. \n\n It may help to read\n http://article.gmane.org/gmane.comp.version-control.git/217 and\n then its follow-up http://gitster.livejournal.com/35628.html\n"},{"id":"217348","messageId":"CABURp0qHb_nYsSHad8E_0s-n-VeZN2tqQjeApV4ssBXrqCQKeQ@mail.gmail.com","threadId":"33818","inReplyTo":"CALkWK0nR1=Pgv0AY78p7n17C-VVvbc6BfJ4y_Df1ciZoc-xR-Q@mail.gmail.com","subject":"Re: [PATCH] diffcore-pickaxe doc: document -S and -G properly","fromName":"Phil Hord","fromEmail":"phil.hord@gmail.com","sentAt":"2013-05-14T19:23:08Z","receivedAt":"2013-05-14T19:23:08Z","isPatch":true,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"On Tue, May 14, 2013 at 2:44 PM, Ramkumar Ramachandra\n wrote:\n> Phil Hord wrote:\n>> References to git-log seem out of place to me here in git-diffcore. I\n>> know it's only an example, but it seems that Git normally describes\n>> these 'reference selectors' more generically. The generic description\n>> may be more confusing to new users, but this patch is not the place to\n>> consider whether it should change.\n>\n> It's not for new users at all. The most useful application of -S and\n> -G is in log. The translation from a log -G to a diffcore -G is not\n> obvious at all, and warrants an explanation.\n>\n> Oh, and for the user. No user is going to type out `man gitdiffcore`\n> out of the blue: she's most probably led there from log, and we're\n> connecting the dots for her.\n\nShe may have been led here by some other help topic, too. Maybe the\ngit log examples belong in the git-log documentation.\n\n>> While the switches are called -S and -G, I do not think it is helpful\n>> to name the two pickaxe options as \"the S kind\" and \"the G kind\".\n>\n> How do you describe something precisely without loss of meaning? You\n> stop abstracting unnecessarily. Read the sources: you will literally\n> see DIFF_PICKAXE_KIND_G there.\n\nS and G are abstractions.\n\nDIFF_PICKAXE_KIND_G is an implementer's distinction.\n\nI agree this is a tricky subject. When writing technical\ndocumentation you must be precise and clear. It is easy to forget\nboth. It is common to forget to be either clear or precise. It is\nvery difficult to be both clear and precise.\n\nSo, my suggestion is to use some meaningful names for the action of -S\nand -G. Relating these names to -S and -G in a clear way is likely to\nbe difficult.\n\n>>> +\n>>> +The S kind detects filepairs whose \"result\" side and \"origin\" side\n>>> +have different number of occurrences of specified string. While\n>>> +rename detection works as usual, 'git log -S' cannot omit commits\n>>\n>> The \"cannot omit\" feels like a confusing double-negative. How about\n>> \"includes\" instead?\n>\n> Intended. Omission is expected.\n\nThis is precision at the expense of clarity. Omission is not expected\nfor the user who wants to ask this question: \"Where is that commit\nthat added 'foo'?\" To Git she wants to ask \"Show me the commit that\nadded or removed 'foo'.\" You and I both know that Git does this in\nreverse. Git translates this question into \"Show all the commits, but\nhide the ones which do not add or remove 'foo'.\"\n\nAnd so we say Git 'cannot omit' commits\n\n\n>\n>> Is it worth mentioning that something in the documentation is \"worth\n>> mentioning\"?\n>\n> You don't have to nitpick style. We can allow this much creative\n> freedom in documentation.\n\nI think I do. The concepts in here are complicated enough without\nloading the language with excess verbiage. I am chronically afflicted\nwith this disease where I am always adding extra decorations to my\nsentences. I work hard to combat that, especially when dealing with\ntechnical writings. This makes it easy for me to recognize in other\ntechnical writing.\n\nI didn't look closely, but these unnecessary introductory clause\nappeared to be the only change in this paragraph. I do not think it\nadds anything, which is why I mentioned it.\n\nI could have been more clear and less flippant about it, though. I'm\nsorry if my manner was offensive. This is another chronic problem I\nseem to have.\n\n\n>>> +in effect, diffcore-pickaxe leaves only such filepairs that touch the\n>>> +specified string in its output. When in effect, diffcore-pickaxe\n>>> +leaves all filepairs intact if there is such a filepair, or makes the\n>>> +output empty otherwise. The latter behavior is designed to make\n>>> +reviewing of the changes in the context of the whole changeset easier.\n>>\n>> I find this description (from the original code, not from this commit)\n>> somewhat confusing. It is written from the implementer's POV.\n>\n> I explained the entire -S and -G thing in terms of filepairs (and yes,\n> that's implementation detail). Why would I want to explain this in\n> any other terms?\n\nClarity.\n\nWhat is a 'filepair' when I am getting a short-log? Internally there\nwas a diff, and the diff involves pairs of files. But it's a tedious\ndetail to the user which might send her off needlessly trying to\nunderstand the meaning of the term.\n\nBut you are right; gitdiffcore.txt is about precision and\nimplementation. The goal is to address the technical user who is\ntrying to understand these operations in general, not just for log.\n'filepairs' is a useful concept to lean on.\n\nThe clearer description probably belongs in git-log.\n\n>> Does\n>> this seem clearer to you?\n>> [...]\n>\n> From diff-options.txt (the more end-user side):\n>\n> When -S or -G finds a change, show all the changes in that changeset,\n> not just the files that contain the change in .\n>\n> Not clear enough?\n\nYes, it was clear enough to me in git-log. Perhaps it is not worth\nmentioning here.\n\nPhil\n"},{"id":"217349","messageId":"CALkWK0=fr6413jY0wEf061y6Q_1MOksuWWBFuKhgdhphPHY-Cw@mail.gmail.com","threadId":"33818","inReplyTo":"20130514191757.GP3657@google.com","subject":"Re: [PATCH] diffcore-pickaxe doc: document -S and -G properly","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-14T19:25:43Z","receivedAt":"2013-05-14T19:25:43Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Jonathan Nieder wrote:\n> Write a better shell?\n\nShell, editor. Both are very hard problems, and the successful\nprojects last many years (emacs, zsh are over 20 years old).\n\n> Teach \"git gui blame\" to blame on arbitrary\n> regions instead of single lines?\n\nOr in my case: get magit to do log -S.\n\nShould we mention in the -S documentation that temporary shell script\nis the way to get multi-line input?\n"},{"id":"217350","messageId":"20130514192909.GQ3657@google.com","threadId":"33818","inReplyTo":"CALkWK0=fr6413jY0wEf061y6Q_1MOksuWWBFuKhgdhphPHY-Cw@mail.gmail.com","subject":"Re: [PATCH] diffcore-pickaxe doc: document -S and -G properly","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-05-14T19:29:09Z","receivedAt":"2013-05-14T19:29:09Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> Should we mention in the -S documentation that temporary shell script\n> is the way to get multi-line input?\n\nNo, because for almost everyone it isn't.\n\nAn example in the EXAMPLES section including an aside that you might\nhave to hit ^V to enter a tab could be useful, though.\n"},{"id":"217351","messageId":"7vli7hicjr.fsf@alter.siamese.dyndns.org","threadId":"33818","inReplyTo":"CALkWK0na1pDi5DnKUcWRnt0v9iQmeaLdrs7uQMDZzCKAazaNFg@mail.gmail.com","subject":"Re: [PATCH] diffcore-pickaxe doc: document -S and -G properly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-14T19:31:04Z","receivedAt":"2013-05-14T19:31:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> Junio C Hamano wrote:\n>> I do not use zsh but with bash+readline the old tradition lnext can\n>> be used (see \"stty -a\" output and it typically is set to ^V), i.e.\n>> \\C-v followed by \\C-i should give you a literal HT.\n>\n> Just looked it up: zsh has quoted-insert (^V) after which I can TAB.\n> Still doesn't solve my problem though: I don't type out structs; I\n> paste them the X clipboard (Emacs). And that doesn't work either on\n> bash or zsh.\n>\n> What can we do to improve the interface?\n\nHeh, I seem to have just found a volunteer to finish the Linus's\ndream by following up on http://gitster.livejournal.com/35628.html\n(see section #5; I do not do GUI, neither Linus, so this has been a\nfour-year old dream).\n"},{"id":"217353","messageId":"7vhai5ic2k.fsf@alter.siamese.dyndns.org","threadId":"33818","inReplyTo":"20130514192909.GQ3657@google.com","subject":"Re: [PATCH] diffcore-pickaxe doc: document -S and -G properly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-14T19:41:23Z","receivedAt":"2013-05-14T19:41:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Ramkumar Ramachandra wrote:\n>\n>> Should we mention in the -S documentation that temporary shell script\n>> is the way to get multi-line input?\n>\n> No, because for almost everyone it isn't.\n>\n> An example in the EXAMPLES section including an aside that you might\n> have to hit ^V to enter a tab could be useful, though.\n\nOr document it as \"for scripter's use\", which it is.\n"},{"id":"217354","messageId":"CALkWK0=U8hySDc=OhQu6PjDNJ_fk3UBW8tmNowntghHS2-VbmQ@mail.gmail.com","threadId":"33818","inReplyTo":"7vppwtid3n.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] diffcore-pickaxe doc: document -S and -G properly","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-14T19:46:01Z","receivedAt":"2013-05-14T19:46:01Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> I think what makes this paragraph unnecessarily hard to read is the\n> \"While rename works\".\n>\n> With that, you are implying \"if you rename a file as a whole without\n> changing the block of text you identify with the -S parameter, then\n> such a change is not interesting as far as pickaxe is concerned\".\n> while that statement is logically correct, normal people are not\n> that generous to read that much between your lines.\n\nYes, I'm exactly implying that. But I don't want to lose meaning: in\nthe previous sentence, I talk about filepairs. I want to point out\nthat rename detection working at the filepair level is a perfectly\nnormal thing.\n\n> I think that is one of the reasons why \"If you moved a string from\n> file A to file B, log -S will flag that change as worth inspecting\"\n> does not seem to logically follow and made Phil find your\n> description confusing.\n\nSure, we can elaborate a bit more.\n\n> Finding such a change indeed is a feature [*1*]; we need to flag\n> such a change as worth inspecting to find where the code came from\n> in order to dig deeper, so at least this \"cannot omit\" should be\n> \"does not omit\".\n\nWhat I was trying to say is that it's an accidental feature: the\nreason this \"feature\" exists is because diffcore is tied to filepairs\n(and rename detection works at the filepair level). You may argue\nthat there's nothing wrong with this design, but consider what happens\nif you rebase on top of a big code move: it's completely broken. If\ngit were a true content tracker, and file boundaries really did not\nmatter, isn't this feature actually a deficiency?\n\nOfcourse, the user doesn't need to know all this, and \"does not omit\"\nis a good suggestion.\n"},{"id":"217356","messageId":"7vd2stibix.fsf@alter.siamese.dyndns.org","threadId":"33818","inReplyTo":"CALkWK0=U8hySDc=OhQu6PjDNJ_fk3UBW8tmNowntghHS2-VbmQ@mail.gmail.com","subject":"Re: [PATCH] diffcore-pickaxe doc: document -S and -G properly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-14T19:53:10Z","receivedAt":"2013-05-14T19:53:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> What I was trying to say is that it's an accidental feature\n\nThere is nothing accidental about it. It was a very conscious\ndesign decision.\n\nWhen a commit moves a file wholesale without affecting the block of\ncode you are interested in, you know that whole block came from the\nfile in the old tree at pre-rename location without looking at\nanywhere else. That is why renamed but pickaxe-uninteresting\nfilepairs are dropped.\n\nWhen a commit moves (some lines of) the block of code you are\ninterested in from one file to another, it may have been a single\ninstance moving to another place, but it may well have been multiple\ncopies consolidated into one (the new copy, pickaxe digging from\nfuture to past may see \"disappearing\"). That is a significant event\nworth digging into further by first stopping there and then\ninspecting the whole change with --pickaxe-all to see what changes\nthat are similar to the change in question exist in the other parts\n(notice the multiple) of the tree.\n\nIf you do not understand it, then you really should re-read\n$gmane/217 and then its explanation I wrote 4 years ago (both of\nwhich I already gave you URLs to in this thread).\n"},{"id":"217358","messageId":"CALkWK0nT-0LG2jaM+Y2R6+brYeFTuPsoDT+R17Ag_jnEwee32Q@mail.gmail.com","threadId":"33818","inReplyTo":"7vd2stibix.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] diffcore-pickaxe doc: document -S and -G properly","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-14T20:02:45Z","receivedAt":"2013-05-14T20:02:45Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> When a commit moves a file wholesale without affecting the block of\n> code you are interested in, you know that whole block came from the\n> file in the old tree at pre-rename location without looking at\n> anywhere else. That is why renamed but pickaxe-uninteresting\n> filepairs are dropped.\n>\n> When a commit moves (some lines of) the block of code you are\n> interested in from one file to another, it may have been a single\n> instance moving to another place, but it may well have been multiple\n> copies consolidated into one (the new copy, pickaxe digging from\n> future to past may see \"disappearing\"). That is a significant event\n> worth digging into further by first stopping there and then\n> inspecting the whole change with --pickaxe-all to see what changes\n> that are similar to the change in question exist in the other parts\n> (notice the multiple) of the tree.\n\nMakes sense. I wasn't looking at it from a macro perspective.\n\nYeah, I constantly re-read 217 and your follow-up.\n"},{"id":"217376","messageId":"CABURp0oJ96bYY=Wvn7ih1nPZWOea9O+tB6xTe0TViXf4rKDf2A@mail.gmail.com","threadId":"33818","inReplyTo":"CALkWK0na1pDi5DnKUcWRnt0v9iQmeaLdrs7uQMDZzCKAazaNFg@mail.gmail.com","subject":"Re: [PATCH] diffcore-pickaxe doc: document -S and -G properly","fromName":"Phil Hord","fromEmail":"phil.hord@gmail.com","sentAt":"2013-05-14T21:13:08Z","receivedAt":"2013-05-14T21:13:08Z","isPatch":true,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"On Tue, May 14, 2013 at 2:57 PM, Ramkumar Ramachandra\n wrote:\n> Junio C Hamano wrote:\n>> I do not use zsh but with bash+readline the old tradition lnext can\n>> be used (see \"stty -a\" output and it typically is set to ^V), i.e.\n>> \\C-v followed by \\C-i should give you a literal HT.\n>\n> Just looked it up: zsh has quoted-insert (^V) after which I can TAB.\n> Still doesn't solve my problem though: I don't type out structs; I\n> paste them the X clipboard (Emacs). And that doesn't work either on\n> bash or zsh.\n>\n> What can we do to improve the interface?\n\nDon't use tabs in your code?\n\nP\n"}]}