{"thread":{"id":"36560","subject":"[BUG?] Patches created with 'diff.noprefix=true' don't 'git apply'.","startedAt":"2014-05-01T01:36:22Z","lastAt":"2014-05-08T16:56:19Z","messageCount":13,"participants":["Nathan Collins","Jonathan Nieder","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"240395","messageId":"CAO8RVvdgN3U5hUEsJjYY9urfeVUDWwHiEur4NQp=H93W37RRnw@mail.gmail.com","threadId":"36560","inReplyTo":null,"subject":"[BUG?] Patches created with 'diff.noprefix=true' don't 'git apply'.","fromName":"Nathan Collins","fromEmail":"nathan.collins@gmail.com","sentAt":"2014-05-01T01:36:22Z","receivedAt":"2014-05-01T01:36:22Z","isPatch":false,"sender":{"key":"nathan.collins@gmail.com","avatar":"https://gravatar.com/avatar/ecec917e18a103ea8fa2cddf99d766caa0f52d0aef155ba715a9ada837b5c1d2?d=mp&s=160"},"body":"Bug?\n====\n\nPatches created with 'diff.noprefix=true' don't 'git apply' without\nspecifying '-p0'.\n\nI'm not sure this is a bug -- the 'man git-apply' just says \"Reads the\nsupplied diff output (i.e. \"a patch\") and applies it to files\" -- but\nI would expect patches I create locally to apply cleanly locally. In\nreal life the 'diff.noprefix=true' is in my ~/.gitconfig, so this was\npretty confusing.\n\nHere's an old bug that's kind of related:\nhttps://bugs.debian.org/cgi-bin/bugreport.cgi?bug=607044\n\nI'm using Git 1.9.2.\n\nExample\n=======\n\nCreate a repo with a test commit:\n\n git init bug.git\n cd bug.git\n git add test\n git commit test -m Test\n\nRevert the test commit in a contrived way (like\n'git revert HEAD --no-commit; git reset'). This works:\n\n git -c diff.noprefix=false show | git -c diff.noprefix=false apply --reverse\n\nAnd this works:\n\n git reset --hard\n git -c diff.noprefix=true show | git -c diff.noprefix=true apply -p0 --reverse\n\nBut this fails:\n\n git reset --hard\n git -c diff.noprefix=true show | git -c diff.noprefix=true apply --reverse\n\n fatal: git diff header lacks filename information when removing 1\nleading pathname component (line 12)\n\nUse Case\n========\n\nPartially reverting a commit:\n\nhttp://git.661346.n2.nabble.com/Revert-a-single-commit-in-a-single-file-td6064050.html#a6064406\n\nCheers,\n\n-nathan\n"},{"id":"240396","messageId":"20140501024042.GX9218@google.com","threadId":"36560","inReplyTo":"CAO8RVvdgN3U5hUEsJjYY9urfeVUDWwHiEur4NQp=H93W37RRnw@mail.gmail.com","subject":"Re: [BUG?] Patches created with 'diff.noprefix=true' don't 'git apply'.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-05-01T02:40:42Z","receivedAt":"2014-05-01T02:40:42Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nNathan Collins wrote:\n\n> Patches created with 'diff.noprefix=true' don't 'git apply' without\n> specifying '-p0'.\n>\n> I'm not sure this is a bug -- the 'man git-apply' just says \"Reads the\n> supplied diff output (i.e. \"a patch\") and applies it to files\" -- but\n> I would expect patches I create locally to apply cleanly locally.\n\nSounds like a documentation bug, at least. Any ideas for clearer\nwording?\n\n> In\n> real life the 'diff.noprefix=true' is in my ~/.gitconfig, so this was\n> pretty confusing.\n\nI personally think setting diff.noprefix is not very sane (it also\nbreaks \"patch -p1\"), and I suppose I should have been louder about\nthat when it was introduced.\n\nCan you say more about the workflow you use that requires\ndiff.noprefix? Maybe we can make other changes to improve it, too.\n\nAt first glance I don't suspect making diff.noprefix imply -p0 for\n\"git am\" would be great, since that would generate the the opposite\nproblem when applying patches from the outside world. But maybe we\nneed better autodetection and maybe noprefix is a good signal about\nwhen to use it.\n\nAnother complication is that unlike 'git diff', 'git apply' is\nplumbing that is meant to be useful and reliable for scripts. And\nunlike most plumbing, there is no higher-level command with similar\nfunctionality for which we can experiment more freely with the UI.\nAdding a new command to fix that might be a good direction toward\nhandling noprefix patches better.\n\n[...]\n> git show | git apply --reverse\n\nThe following which only uses plumbing commands should work:\n\n\tgit diff-tree -p HEAD^! |\n\tgit apply --reverse\n\nThanks for some food for thought,\nJonathan\n"},{"id":"240746","messageId":"CAO8RVveV4SgXmJzWpYnyB3rnQALQkrrLNtLL5Ej5Y9Mjjekmmw@mail.gmail.com","threadId":"36560","inReplyTo":"20140501024042.GX9218@google.com","subject":"Re: [BUG?] Patches created with 'diff.noprefix=true' don't 'git apply'.","fromName":"Nathan Collins","fromEmail":"nathan.collins@gmail.com","sentAt":"2014-05-06T01:33:28Z","receivedAt":"2014-05-06T01:33:28Z","isPatch":false,"sender":{"key":"nathan.collins@gmail.com","avatar":"https://gravatar.com/avatar/ecec917e18a103ea8fa2cddf99d766caa0f52d0aef155ba715a9ada837b5c1d2?d=mp&s=160"},"body":"On Wed, Apr 30, 2014 at 7:40 PM, Jonathan Nieder wrote:\n> Hi,\n>\n> Nathan Collins wrote:\n>\n>> Patches created with 'diff.noprefix=true' don't 'git apply' without\n>> specifying '-p0'.\n>>\n>> I'm not sure this is a bug -- the 'man git-apply' just says \"Reads the\n>> supplied diff output (i.e. \"a patch\") and applies it to files\" -- but\n>> I would expect patches I create locally to apply cleanly locally.\n>\n> Sounds like a documentation bug, at least. Any ideas for clearer\n> wording?\n\nHmmm. Maybe a warning that the patch is expected to be in '-p1'\nformat, and that setting 'diff.noprefix=true' makes some commands\ngenerate '-p0' patches? But I worry this would just confuse / distract\nthe people that don't have 'diff.noprefix=true' set, which I expect is\nthe majority of users. Better I think would be for 'git apply' to be\nsmarter, as you suggest below.\n\n>> In\n>> real life the 'diff.noprefix=true' is in my ~/.gitconfig, so this was\n>> pretty confusing.\n>\n> I personally think setting diff.noprefix is not very sane (it also\n> breaks \"patch -p1\"), and I suppose I should have been louder about\n> that when it was introduced.\n>\n> Can you say more about the workflow you use that requires\n> diff.noprefix? Maybe we can make other changes to improve it, too.\n\nI have 'diff.noprefix=true' set so I can copy and paste paths from the\n'git diff' output easily. I like to create small, logically\nindependent commits, usually comprising a subset of my current\nchanges. So, I do 'git diff' in one terminal, and then 'git add\n' or 'git add --patch ' in another terminal to build up a\ncommit (I suppose this is the work flow that 'git add --interactive'\nis designed for ...), where I get '' from the diff by copying\nand pasting. With 'diff.noprefix=true', I can copy with double left\nclick and paste with middle click; with 'diff.noprefix=false', to copy\nI instead have to carefully highlight the non-prefix part of the path\nin the diff, which is less convenient.\n\n> At first glance I don't suspect making diff.noprefix imply -p0 for\n> \"git am\" would be great, since that would generate the the opposite\n> problem when applying patches from the outside world. But maybe we\n> need better autodetection and maybe noprefix is a good signal about\n> when to use it.\n\nAutodetecting the lack or presence of the 'a/' and 'b/' prefixes seems\nlike a great solution to me: externally user friendly and easy to\nimplement internally.\n\n> Another complication is that unlike 'git diff', 'git apply' is\n> plumbing that is meant to be useful and reliable for scripts. And\n> unlike most plumbing, there is no higher-level command with similar\n> functionality for which we can experiment more freely with the UI.\n> Adding a new command to fix that might be a good direction toward\n> handling noprefix patches better.\n\nRelated to 'git apply' being a scriptable plumbing command: naively I\nwould expect there to be a \"scripting mode\" for Git commands which\nignored the local configuration entirely (e.g. ~/.gitconfig). I've\nwanted this a few times and was surprised I could find no very sane\nway to achieve it. In fact, here's the corresponding question I posted\non Stack Overflow while I was composing my original email (I wanted to\nbe sure that 'diff.noprefix=true' was the only relevant part of my\n~/.gitconfig, so I wanted disable my ~/.gitconfig entirely):\n\nhttp://stackoverflow.com/questions/23400449/how-to-make-git-temporarily-ignore-gitconfig\n\n> [...]\n>> git show | git apply --reverse\n>\n> The following which only uses plumbing commands should work:\n>\n> git diff-tree -p HEAD^! |\n> git apply --reverse\n\nNice! However, I don't now how to generalize this solution to other\n(probably insane) use cases, e.g.\n\n git log -S --patch | git apply --reverse\n\n(Context: http://stackoverflow.com/a/23401018/470844).\n\n> Thanks for some food for thought,\n> Jonathan\n\nThanks for your reply. I didn't see it until today because a GMail\nfilter ate it :P\n\n-nathan\n"},{"id":"240786","messageId":"20140506015927.GV9218@google.com","threadId":"36560","inReplyTo":"CAO8RVveV4SgXmJzWpYnyB3rnQALQkrrLNtLL5Ej5Y9Mjjekmmw@mail.gmail.com","subject":"Re: [BUG?] Patches created with 'diff.noprefix=true' don't 'git apply'.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-05-06T01:59:27Z","receivedAt":"2014-05-06T01:59:27Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Nathan Collins wrote:\n> On Wed, Apr 30, 2014 at 7:40 PM, Jonathan Nieder wrote:\n>> Nathan Collins wrote:\n\n>>> git show | git apply --reverse\n>>\n>> The following which only uses plumbing commands should work:\n>>\n>> git diff-tree -p HEAD^! |\n>> git apply --reverse\n>\n> Nice! However, I don't now how to generalize this solution to other\n> (probably insane) use cases, e.g.\n>\n> git log -S --patch | git apply --reverse\n\nThis should do it:\n\n\tgit rev-list HEAD |\n\tgit diff-tree --no-commit-id -p -S --stdin |\n\tgit apply --reverse\n\nMore generally, when scripting plumbing commands tend to do the right\nthing.\n\nWill think more about the documentation and other parts (or if someone\nelse sends a patch before I can, I won't mind).\n\nThanks,\nJonathan\n"},{"id":"240811","messageId":"xmqqa9aug4my.fsf@gitster.dls.corp.google.com","threadId":"36560","inReplyTo":"CAO8RVveV4SgXmJzWpYnyB3rnQALQkrrLNtLL5Ej5Y9Mjjekmmw@mail.gmail.com","subject":"Re: [BUG?] Patches created with 'diff.noprefix=true' don't 'git apply'.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-06T18:10:13Z","receivedAt":"2014-05-06T18:10:13Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nathan Collins writes:\n\n> Hmmm. Maybe a warning that the patch is expected to be in '-p1'\n> format, and that setting 'diff.noprefix=true' makes some commands\n> generate '-p0' patches?\n\n\"some\"? Do you have exceptions in mind?\n\n> But I worry this would just confuse / distract\n> the people that don't have 'diff.noprefix=true' set,\n\nProbably. But that would suggest that the place to improve the doc\nis for diff.noprefix configuration variable, no?\n\n> Better I think would be for 'git apply' to be\n> smarter, as you suggest below.\n\nAs it is a plumbing command behind \"add -p\", \"am\", and friends, I\nwould hate to see \"git apply\" pretend to be smarter than its users.\nWhen the user tells it to use -p0, it shouldn't guess, and when the\nuser tells it to use -p1 by not giving any -p$n, it shouldn't guess.\n\nAs long as we make it clear \"git apply\" without any explicit -p$n\nmeans the user is telling it to do -p1 in its documentation, I think\nit would be fine.\n\n>> I personally think setting diff.noprefix is not very sane (it also\n>> breaks \"patch -p1\"), and I suppose I should have been louder about\n>> that when it was introduced.\n\nI share the same feeling ;-) But the boat has sailed, so the best\nwe could do is to warn in its doc (i.e. where diff.noprefix is\ndescribed) about its pitfalls.\n"},{"id":"240828","messageId":"CAO8RVvfHVBG6QZ5M1DJK4kOX8GMDrvEw4_L=LsEc4G6E0pP7ug@mail.gmail.com","threadId":"36560","inReplyTo":"xmqqa9aug4my.fsf@gitster.dls.corp.google.com","subject":"Re: [BUG?] Patches created with 'diff.noprefix=true' don't 'git apply'.","fromName":"Nathan Collins","fromEmail":"nathan.collins@gmail.com","sentAt":"2014-05-06T19:36:58Z","receivedAt":"2014-05-06T19:36:58Z","isPatch":false,"sender":{"key":"nathan.collins@gmail.com","avatar":"https://gravatar.com/avatar/ecec917e18a103ea8fa2cddf99d766caa0f52d0aef155ba715a9ada837b5c1d2?d=mp&s=160"},"body":"On Tue, May 6, 2014 at 11:10 AM, Junio C Hamano wrote:\n> Nathan Collins writes:\n>\n>> Hmmm. Maybe a warning that the patch is expected to be in '-p1'\n>> format, and that setting 'diff.noprefix=true' makes some commands\n>> generate '-p0' patches?\n>\n> \"some\"? Do you have exceptions in mind?\n\nAs Jonathan pointed out in his first reply, 'git diff-tree' ignores\nthe 'diff.noprefix=true' setting. Compare\n\n git -c diff.noprefix=true diff HEAD~\n\nwith\n\n git -c diff.noprefix=true diff-tree -p HEAD\n\n(E.g.\n\n diff <(git -c diff.noprefix=true diff HEAD~) <(git -c\ndiff.noprefix=true diff-tree -p HEAD)\n\n)\n\n>> But I worry this would just confuse / distract\n>> the people that don't have 'diff.noprefix=true' set,\n>\n> Probably. But that would suggest that the place to improve the doc\n> is for diff.noprefix configuration variable, no?\n\nI don't think that would actually help much in practice. The problem\nis that a person (like me) that set 'diff.noprefix=true' in their\n~/.gitconfig months or years ago is unlikely to do 'man git-config'\nwhen 'git apply' fails. Having the warning in 'man git-apply' is\nbetter than (only) in 'man git-config', if making 'git apply' smarter\nis not an option.\n\n>> Better I think would be for 'git apply' to be\n>> smarter, as you suggest below.\n>\n> As it is a plumbing command behind \"add -p\", \"am\", and friends, I\n> would hate to see \"git apply\" pretend to be smarter than its users.\n> When the user tells it to use -p0, it shouldn't guess, and when the\n> user tells it to use -p1 by not giving any -p$n, it shouldn't guess.\n\nIs there a non-plumbing command for applying patches not in mailboxes?\nI don't see how to replace '| git apply --reverse' with '| git am ???'\nhere.\n\n> As long as we make it clear \"git apply\" without any explicit -p$n\n> means the user is telling it to do -p1 in its documentation, I think\n> it would be fine.\n\nOK, then how about a smarter error message? Right now I get\n\n git -c diff.noprefix=true diff HEAD~ | git -c diff.noprefix=true\napply --reverse\n error: Data/Function/Decorator/Memoizer/Unsafe.hs: No such file or directory\n\nvs\n\n git -c diff.noprefix=true diff HEAD~ | patch --reverse\n can't find file to patch at input line 5\n Perhaps you should have used the -p or --strip option?\n [...]\n\nBut 'git apply' could be much more helpful than 'patch' even, since\nthe presence or absence of the 'a/' and 'b/' prefixes in the patch,\nand the 'diff.noprefix' setting, give Git enough info to be very\nhelpful to the user.\n\nCheers,\n\n-nathan\n"},{"id":"240840","messageId":"xmqqbnvaehm3.fsf@gitster.dls.corp.google.com","threadId":"36560","inReplyTo":"CAO8RVvfHVBG6QZ5M1DJK4kOX8GMDrvEw4_L=LsEc4G6E0pP7ug@mail.gmail.com","subject":"Re: [BUG?] Patches created with 'diff.noprefix=true' don't 'git apply'.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-06T21:12:52Z","receivedAt":"2014-05-06T21:12:52Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nathan Collins writes:\n\n> But 'git apply' could be much more helpful than 'patch' even, since\n> the presence or absence of the 'a/' and 'b/' prefixes in the patch,\n> and the 'diff.noprefix' setting, give Git enough info to be very\n> helpful to the user.\n\nThe prefix would be unreliable as the generator may be using the\nmnemonicprefix option to use i/ and w/ prefixes. Worse yet, the\nconfiguration variables are for people who generated the diff you\nare feeding \"git apply\", and there is nothing that tells \"apply\"\nthat the patch is generated with _your_ setting.\n\nSo that is not workable.\n\nHowever, Before issuing this error message\n\n> git -c diff.noprefix=true diff HEAD~ | git apply --reverse\n> error: Data/Function/Decorator/Memoizer/Unsafe.hs: No such file or directory\n\nwe _could_ check that there is Data/ directory in the target tree\nthe patch is being applied and suggest to:\n\n * \"use -p0\", if noprefix, which I agree with Jonathan is an insane\n thing to use by default, is common enough; or\n\n * \"use different setting for -p$n\", if noprefix is not common.\n\nin the error message. Extra computation necessary to do so would\nhappen only in the error codepath, and we wouldn't mind spending\nsome cycles if they help the end user.\n"},{"id":"240863","messageId":"CAO8RVveF6Oh0mqqyiVRHmnqQ+vXX7riDW_=zn0g=t88cQjCVrg@mail.gmail.com","threadId":"36560","inReplyTo":"xmqqbnvaehm3.fsf@gitster.dls.corp.google.com","subject":"Re: [BUG?] Patches created with 'diff.noprefix=true' don't 'git apply'.","fromName":"Nathan Collins","fromEmail":"nathan.collins@gmail.com","sentAt":"2014-05-07T01:16:30Z","receivedAt":"2014-05-07T01:16:30Z","isPatch":false,"sender":{"key":"nathan.collins@gmail.com","avatar":"https://gravatar.com/avatar/ecec917e18a103ea8fa2cddf99d766caa0f52d0aef155ba715a9ada837b5c1d2?d=mp&s=160"},"body":"On Tue, May 6, 2014 at 2:12 PM, Junio C Hamano wrote:\n> Nathan Collins writes:\n>\n>> But 'git apply' could be much more helpful than 'patch' even, since\n>> the presence or absence of the 'a/' and 'b/' prefixes in the patch,\n>> and the 'diff.noprefix' setting, give Git enough info to be very\n>> helpful to the user.\n>\n> The prefix would be unreliable as the generator may be using the\n> mnemonicprefix option to use i/ and w/ prefixes. Worse yet, the\n> configuration variables are for people who generated the diff you\n> are feeding \"git apply\", and there is nothing that tells \"apply\"\n> that the patch is generated with _your_ setting.\n\nMore concretely, what I had in mind was that if 'diff.noprefix=true'\nis set in the user's config, and the patch is in '-p0' format, then\nGit could suggest to the user that the 'diff.noprefix' setting *might*\nbe causing them to generate '-p0' patches. If the user had in fact not\ngenerated the patch themselves, then they could safely ignore the\nsuggestion.\n\nBut this may just be an overcomplicated solution to my and others'\nmisuse of the 'diff.noprefix' option; see below.\n\n> So that is not workable.\n>\n> However, Before issuing this error message\n>\n>> git -c diff.noprefix=true diff HEAD~ | git apply --reverse\n>> error: Data/Function/Decorator/Memoizer/Unsafe.hs: No such file or directory\n>\n> we _could_ check that there is Data/ directory in the target tree\n> the patch is being applied and suggest to:\n\nTo clarify, the actual path was\n\n src/Data/Function/Decorator/Memoizer/Unsafe.hs\n\nThe path in the error message,\n\n Data/Function/Decorator/Memoizer/Unsafe.hs\n\nwas the '-p1' version of that path. This is extra confusing if the\nuser is unfamiliar with the '-p' option for patch and unaware that\n'git apply' is assuming '-p1'.\n\n> * \"use -p0\", if noprefix, which I agree with Jonathan is an insane\n> thing to use by default, is common enough; or\n>\n> * \"use different setting for -p$n\", if noprefix is not common.\n>\n> in the error message. Extra computation necessary to do so would\n> happen only in the error codepath, and we wouldn't mind spending\n> some cycles if they help the end user.\n\nI'm starting to think there are really two separate issues here:\n\n1. 'git apply' doesn't give a very helpful error message when the\npatch does not apply due to not being in '-p1' format.\n\n2. the 'diff.noprefix=true' option is used for two unrelated things in\npractice. One of them is related to diffing -- namely, making Git\ngenerate '-p0' patches -- and the other is unrelated to diffing --\nnamely, users want file names that can be easily copied with\ndouble-click.\n\nFor (1), I think the solution is check if the patch makes sense as\n'-p0', in the error case, and tell the user about this in the error\nmessage as you suggested above. In fact, in case the '-p1' path\ndoesn't exist, Git could just try all possible '-p$n' values, and\nreport the first that yields valid paths, if any. Mentioning to the\nuser that they have 'diff.noprefix=true' set in case '-p0' is\ndiscovered might be helpful, but a better solution to (2) might\neliminate this problem in practice.\n\nFor (2), the solution may be to add a separate\n'diff.add-clickable-paths' option (probably there is a better name?\n'diff.add-copyable-paths'? ...), which makes Git insert clickable\npaths in the comments in the diff output. This handles the\nclickable-paths use case which lead me and others to abuse\n'diff.noprefix=true'. Examples where people suggest using\n'diff.noprefix=true' to make it easier to double-click copy paths\ninclude [1 - 5]. Examples where people suggest using 'noprefix' to\ngenerate '-p0' patches include [6 - 10].\n\nConcretely, if 'diff.add-clickable-paths' is set, then instead of e.g.\n\n diff --git a/src/Data/Function/Decorator/Memoizer/Unsafe.hs\nb/src/Data/Function/Decorator/Memoizer\n index 3ef17da..a0586d3 100644\n --- a/src/Data/Function/Decorator/Memoizer/Unsafe.hs\n +++ b/src/Data/Function/Decorator/Memoizer/Unsafe.hs\n\nwe get e.g.\n\n # src/Data/Function/Decorator/Memoizer/Unsafe.hs\n diff --git a/src/Data/Function/Decorator/Memoizer/Unsafe.hs\nb/src/Data/Function/Decorator/Memoizer\n index 3ef17da..a0586d3 100644\n --- a/src/Data/Function/Decorator/Memoizer/Unsafe.hs\n +++ b/src/Data/Function/Decorator/Memoizer/Unsafe.hs\n\nas the diff header.\n\nIn any case, warning the user in the 'diff.noprefix' docs that the\npoint of the option is to create '-p0' patches, and that setting this\npermanently will cause bad interactions with other Git commands (like\n'git apply') seems like a great idea -- you suggested this in your\nfirst email, but I hadn't really understood why my use of\n'diff.noprefix' was \"insane\" yet. This probably won't help people that\nblindly use 'noprefix' in the \"insane\" way based on a suggestion they\nfound with Google, but it can't hurt. If there were a\n'diff.add-clickable-paths' option, then the 'diff.noprefix' docs could\nalso mention this, and suggest the user use that instead if their use\ncase is the \"easy copy\" use case.\n\nThank you both for pointing out that my usage of 'diff.noprefix=true'\nis insane. Sorry if my suggestion for the 'diff.add-clickable-paths'\noption is also insane; making the 'git apply' error message better --\nkeeping in mind that users who use 'noprefix' to create clickable\npaths may not even know what \"'-p$n' patch\" is -- is probably\nsufficient in practice.\n\nCheers,\n\n-nathan\n\nReferences for copy and paste:\n\n[1] http://stackoverflow.com/a/20370519/470844: answer to SO question\nabout why there are 'a/' and 'b/' prefixes. Last comment says that\n'--no-prefix' makes copy and paste easier.\n\n[2] https://coderwall.com/p/oaekvg: points out that 'noprefix' makes\ncopy and paste easier.\n\n[3] http://git.661346.n2.nabble.com/PATCH-git-jump-ignore-custom-prefix-in-diff-mode-tp7567162p7567308.html:\npost to Git mailing list with user mentioning they use 'noprefix' to\nmake copy and paste easier.\n\n[4] http://hugogiraudel.com/2014/03/17/git-tips-and-tricks-part-2/:\nblog post suggesting you can set 'noprefix' to make it easier to copy\nand paste file names.\n\n[5] http://www.databasesandlife.com/why-subversion-is-better-than-git/:\npage arguing that SVN is better than Git (hahaha) because in SVN you\ncan copy file names from diff output with double click, but in Git\nthere is an annoying leading 'a/' or 'b/'. A Git defender replies\nsuggesting that 'noprefix' can be used to make copy/paste work.\n\nReferences for creating '-p0' patches:\n\n[6] http://tamsler.blogspot.com/2009/02/patching-with-git-diff.html\n\n[7] http://stackoverflow.com/questions/4610744/can-i-get-a-patch-compatible-output-from-git-diff\n\n[8] http://lists.drupal.org/pipermail/development/2011-April/038006.html\n\n[9] http://www.lullabot.com/blog/article/git-best-practices-upgrading-patch-process\n\n[10]\nhttp://scribu.net/wordpress/contributing-to-wordpress-using-github.html:\nSVN uses '-p0' patches, so they suggest using 'noprefix' to generate\nSVN compatible patches from Git.\n"},{"id":"240931","messageId":"xmqqzjita0sn.fsf@gitster.dls.corp.google.com","threadId":"36560","inReplyTo":"CAO8RVveF6Oh0mqqyiVRHmnqQ+vXX7riDW_=zn0g=t88cQjCVrg@mail.gmail.com","subject":"Re: [BUG?] Patches created with 'diff.noprefix=true' don't 'git apply'.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-07T18:42:00Z","receivedAt":"2014-05-07T18:42:00Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nathan Collins writes:\n\n> More concretely, what I had in mind was that if 'diff.noprefix=true'\n> is set in the user's config, and the patch is in '-p0' format, then\n> Git could suggest to the user that the 'diff.noprefix' setting *might*\n> be causing them to generate '-p0' patches. If the user had in fact not\n> generated the patch themselves, then they could safely ignore the\n> suggestion.\n\nIssuing a suggestion that can be ignored too often sounds like\ncrying wolf to train users to ignore all other more useful\nsuggestions, so I would advise to be very cautious before doing so.\nIf you cannot come up with a way to make the heuristics very sure\nand foolproof, I do not think it would help more than it hurt.\n\nOne possibility I considered briefly before discarding it was to see\nif the input is coming from a pipe *and* these configuration is set.\n\n> But this may just be an overcomplicated solution to my and others'\n> misuse of the 'diff.noprefix' option; see below.\n>\n>> So that is not workable.\n\nYes.\n\n>> However, Before issuing this error message\n>>\n>>> git -c diff.noprefix=true diff HEAD~ | git apply --reverse\n>>> error: Data/Function/Decorator/Memoizer/Unsafe.hs: No such file or directory\n>>\n>> we _could_ check that there is Data/ directory in the target tree\n>> the patch is being applied and suggest to:\n>\n> To clarify, the actual path was\n>\n> src/Data/Function/Decorator/Memoizer/Unsafe.hs\n\nYeah, and I meant to suggest checking that path is plausible. It\nwould not help when adding a new file, but we can issue the original\nerror message and that is perfectly readable, so catching only paths\nthat are modified or deleted and suggesting -p$n is still making\nthings better.\n\n> 1. 'git apply' doesn't give a very helpful error message when the\n> patch does not apply due to not being in '-p1' format.\n\nThat \"Data/Function/...\" path in the error message is helpful\nenough, once you learn that -p$n strips the leading directory and\nthat -p1 is the default. For new people, however, -p1 being the\ndefault might be unexpected, but the learning curve is not that\nsteep, I would think. So this is a one-time thing.\n\n> 2. the 'diff.noprefix=true' option is used for two unrelated things in\n> practice. One of them is related to diffing -- namely, making Git\n> generate '-p0' patches -- and the other is unrelated to diffing --\n> namely, users want file names that can be easily copied with\n> double-click.\n\nI do not share this observation, as I never double-click. I let my\nshell to complete instead ;-)\n\n> For (2), the solution may be to add a separate\n> 'diff.add-clickable-paths' option (probably there is a better name?\n> 'diff.add-copyable-paths'? ...),...\n> ...\n> Concretely, if 'diff.add-clickable-paths' is set, then instead of e.g.\n>\n> diff --git a/src/Data/Function/Decorator/Memoizer/Unsafe.hs\n> b/src/Data/Function/Decorator/Memoizer\n> index 3ef17da..a0586d3 100644\n> --- a/src/Data/Function/Decorator/Memoizer/Unsafe.hs\n> +++ b/src/Data/Function/Decorator/Memoizer/Unsafe.hs\n\nIf you do something along that line, perhaps\n\n\tIndex: src/Data/Function/Decorator/Memoizer/Unsafe.hs\n diff --git a/src/Data/Function/Decorator/Memoizer/Unsafe.hs ...\n index 3ef17da..a0586d3 100644\n --- a/src/Data/Function/Decorator/Memoizer/Unsafe.hs\n +++ b/src/Data/Function/Decorator/Memoizer/Unsafe.hs\n\nto imitate what \"cvs diff\" does may be more familar to people.\n\nWhat would you propose to make clickable in a renaming diff, though?\n\n \n"},{"id":"240976","messageId":"CAO8RVvfzP_nc9=nwr6JSK9s6u7DEctMpdn_5yca9h7G4TOd2Bw@mail.gmail.com","threadId":"36560","inReplyTo":"xmqqzjita0sn.fsf@gitster.dls.corp.google.com","subject":"Re: [BUG?] Patches created with 'diff.noprefix=true' don't 'git apply'.","fromName":"Nathan Collins","fromEmail":"nathan.collins@gmail.com","sentAt":"2014-05-07T23:39:45Z","receivedAt":"2014-05-07T23:39:45Z","isPatch":false,"sender":{"key":"nathan.collins@gmail.com","avatar":"https://gravatar.com/avatar/ecec917e18a103ea8fa2cddf99d766caa0f52d0aef155ba715a9ada837b5c1d2?d=mp&s=160"},"body":"On Wed, May 7, 2014 at 11:42 AM, Junio C Hamano wrote:\n> Nathan Collins writes:\n\n>> For (2), the solution may be to add a separate\n>> 'diff.add-clickable-paths' option (probably there is a better name?\n>> 'diff.add-copyable-paths'? ...),...\n>> ...\n>> Concretely, if 'diff.add-clickable-paths' is set, then instead of e.g.\n>>\n>> diff --git a/src/Data/Function/Decorator/Memoizer/Unsafe.hs\n>> b/src/Data/Function/Decorator/Memoizer\n>> index 3ef17da..a0586d3 100644\n>> --- a/src/Data/Function/Decorator/Memoizer/Unsafe.hs\n>> +++ b/src/Data/Function/Decorator/Memoizer/Unsafe.hs\n>\n> If you do something along that line, perhaps\n>\n> Index: src/Data/Function/Decorator/Memoizer/Unsafe.hs\n> diff --git a/src/Data/Function/Decorator/Memoizer/Unsafe.hs ...\n> index 3ef17da..a0586d3 100644\n> --- a/src/Data/Function/Decorator/Memoizer/Unsafe.hs\n> +++ b/src/Data/Function/Decorator/Memoizer/Unsafe.hs\n>\n> to imitate what \"cvs diff\" does may be more familar to people.\n>\n> What would you propose to make clickable in a renaming diff, though?\n\nYour 'Index' header looks good, and I would expect a renaming diff to\nhave something like\n\n Index: foo -> bar\n\nas in 'git status', but I just realized that a \"clickable paths\"\noption already exists in some sense! There is a '--patch-with-raw'\noption (which is \"short\" for '--patch' and '--raw', hahaha) which\ninserts clickable file names in the patch, above each diff. Moreover,\nit respects the '--relative' option, so you can get relative or\nabsolute (relative repo root) clickable paths. It handles renaming by\ninserting the old and new paths separated by space.\n\nSo then, having a way to make '--patch-with-raw' the default for all\nnon-plumbing patch-producing commands would solve the clickable paths\nproblem.\n\nIn a summary, a possible complete solution:\n\n1. improve Git apply error message: mention '-p$n' and '-p1' default,\n and report if path in patch makes sense for some non-default '-p'\n value.\n\n2. improve 'diff.noprefix' documentation: tell user that this option\n is for producing '-p0' patches, and that using it to produce\n clickable paths is insane and may cause problems with generated\n patches. Suggest the user use '--patch-with-raw', and possibly\n '--relative', instead, or refer to (3).\n\n3. add a Git config for making '--patch-with-raw' and optionally\n '--relative' the default for non-plumbing patch-producing commands.\n\nCheers,\n\n-nathan\n"},{"id":"240990","messageId":"CAO8RVvcyi6KvrZqueBA+PKCTocL=s7NJGann4CzU0WAf6DxWcA@mail.gmail.com","threadId":"36560","inReplyTo":"CAO8RVvfzP_nc9=nwr6JSK9s6u7DEctMpdn_5yca9h7G4TOd2Bw@mail.gmail.com","subject":"Re: [BUG?] Patches created with 'diff.noprefix=true' don't 'git apply'.","fromName":"Nathan Collins","fromEmail":"nathan.collins@gmail.com","sentAt":"2014-05-08T04:38:51Z","receivedAt":"2014-05-08T04:38:51Z","isPatch":false,"sender":{"key":"nathan.collins@gmail.com","avatar":"https://gravatar.com/avatar/ecec917e18a103ea8fa2cddf99d766caa0f52d0aef155ba715a9ada837b5c1d2?d=mp&s=160"},"body":"On Wed, May 7, 2014 at 4:39 PM, Nathan Collins wrote:\n> On Wed, May 7, 2014 at 11:42 AM, Junio C Hamano wrote:\n>> Nathan Collins writes:\n>\n>>> For (2), the solution may be to add a separate\n>>> 'diff.add-clickable-paths' option (probably there is a better name?\n>>> 'diff.add-copyable-paths'? ...),...\n>>> ...\n>>> Concretely, if 'diff.add-clickable-paths' is set, then instead of e.g.\n>>>\n>>> diff --git a/src/Data/Function/Decorator/Memoizer/Unsafe.hs\n>>> b/src/Data/Function/Decorator/Memoizer\n>>> index 3ef17da..a0586d3 100644\n>>> --- a/src/Data/Function/Decorator/Memoizer/Unsafe.hs\n>>> +++ b/src/Data/Function/Decorator/Memoizer/Unsafe.hs\n>>\n>> If you do something along that line, perhaps\n>>\n>> Index: src/Data/Function/Decorator/Memoizer/Unsafe.hs\n>> diff --git a/src/Data/Function/Decorator/Memoizer/Unsafe.hs ...\n>> index 3ef17da..a0586d3 100644\n>> --- a/src/Data/Function/Decorator/Memoizer/Unsafe.hs\n>> +++ b/src/Data/Function/Decorator/Memoizer/Unsafe.hs\n>>\n>> to imitate what \"cvs diff\" does may be more familar to people.\n>>\n>> What would you propose to make clickable in a renaming diff, though?\n>\n> Your 'Index' header looks good, and I would expect a renaming diff to\n> have something like\n>\n> Index: foo -> bar\n>\n> as in 'git status', but I just realized that a \"clickable paths\"\n> option already exists in some sense! There is a '--patch-with-raw'\n> option (which is \"short\" for '--patch' and '--raw', hahaha) which\n> inserts clickable file names in the patch, above each diff.\n\nOr not: I stupidly only tested this with a single file modified: it\nturns out that all the clickable file names appear at the top of the\npatch, not as one file name above each corresponding diff as I\nclaimed.\n\nD'oh,\n\n-nathan\n"},{"id":"240991","messageId":"CAO8RVvdfEPrtNNr1bFAA-aUigz67swS-rzUhTkZE76DJO3TBaA@mail.gmail.com","threadId":"36560","inReplyTo":"CAO8RVvcyi6KvrZqueBA+PKCTocL=s7NJGann4CzU0WAf6DxWcA@mail.gmail.com","subject":"Re: [BUG?] Patches created with 'diff.noprefix=true' don't 'git apply'.","fromName":"Nathan Collins","fromEmail":"nathan.collins@gmail.com","sentAt":"2014-05-08T04:53:57Z","receivedAt":"2014-05-08T04:53:57Z","isPatch":false,"sender":{"key":"nathan.collins@gmail.com","avatar":"https://gravatar.com/avatar/ecec917e18a103ea8fa2cddf99d766caa0f52d0aef155ba715a9ada837b5c1d2?d=mp&s=160"},"body":"On Wed, May 7, 2014 at 9:38 PM, Nathan Collins wrote:\n> On Wed, May 7, 2014 at 4:39 PM, Nathan Collins wrote:\n>> On Wed, May 7, 2014 at 11:42 AM, Junio C Hamano wrote:\n>>> Nathan Collins writes:\n>>\n>>>> For (2), the solution may be to add a separate\n>>>> 'diff.add-clickable-paths' option (probably there is a better name?\n>>>> 'diff.add-copyable-paths'? ...),...\n>>>> ...\n>>>> Concretely, if 'diff.add-clickable-paths' is set, then instead of e.g.\n>>>>\n>>>> diff --git a/src/Data/Function/Decorator/Memoizer/Unsafe.hs\n>>>> b/src/Data/Function/Decorator/Memoizer\n>>>> index 3ef17da..a0586d3 100644\n>>>> --- a/src/Data/Function/Decorator/Memoizer/Unsafe.hs\n>>>> +++ b/src/Data/Function/Decorator/Memoizer/Unsafe.hs\n>>>\n>>> If you do something along that line, perhaps\n>>>\n>>> Index: src/Data/Function/Decorator/Memoizer/Unsafe.hs\n>>> diff --git a/src/Data/Function/Decorator/Memoizer/Unsafe.hs ...\n>>> index 3ef17da..a0586d3 100644\n>>> --- a/src/Data/Function/Decorator/Memoizer/Unsafe.hs\n>>> +++ b/src/Data/Function/Decorator/Memoizer/Unsafe.hs\n>>>\n>>> to imitate what \"cvs diff\" does may be more familar to people.\n>>>\n>>> What would you propose to make clickable in a renaming diff, though?\n>>\n>> Your 'Index' header looks good, and I would expect a renaming diff to\n>> have something like\n>>\n>> Index: foo -> bar\n>>\n>> as in 'git status', but I just realized that a \"clickable paths\"\n>> option already exists in some sense! There is a '--patch-with-raw'\n>> option (which is \"short\" for '--patch' and '--raw', hahaha) which\n>> inserts clickable file names in the patch, above each diff.\n>\n> Or not: I stupidly only tested this with a single file modified: it\n> turns out that all the clickable file names appear at the top of the\n> patch, not as one file name above each corresponding diff as I\n> claimed.\n\nThe following may be a non-option, since presumably many tools depend\non the current Git patch format.\n\nThe paths in the \"extended header lines\" in Git patches are clickable\nby default, and respect the '--relative' option. So, adding a path to\nthe extended header lines that don't already have one would solve the\n\"clickable paths\" problem.\n\nE.g.\n\n index .. \n\nbecomes\n\n index .. \n\nThe 'man git-diff' description of extended header lines in the\n\"Generating Patches with -p\" section:\n\n 2. It is followed by one or more extended header lines:\n\n old mode \n new mode \n deleted file mode \n new file mode \n copy from \n copy to \n rename from \n rename to \n similarity index \n dissimilarity index \n index .. \n\n File modes are printed as 6-digit octal numbers including the\nfile type and file\n permission bits.\n\n Path names in extended headers do not include the a/ and b/ prefixes.\n\nCheers,\n\n-nathan\n"},{"id":"241008","messageId":"xmqqa9as5hvw.fsf@gitster.dls.corp.google.com","threadId":"36560","inReplyTo":"CAO8RVvfzP_nc9=nwr6JSK9s6u7DEctMpdn_5yca9h7G4TOd2Bw@mail.gmail.com","subject":"Re: [BUG?] Patches created with 'diff.noprefix=true' don't 'git apply'.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-08T16:56:19Z","receivedAt":"2014-05-08T16:56:19Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nathan Collins writes:\n\n>> What would you propose to make clickable in a renaming diff, though?\n>\n> Your 'Index' header looks good, and I would expect a renaming diff to\n> have something like\n>\n> Index: foo -> bar\n>\n> as in 'git status',\n\nHeh, please don't call \"Index:\" *mine* --- It is a CVS abomination\n;-).\n\nFor renames and copies, we do have separate \"rename from\" and\n\"rename to\" in the extended header part, so there is no reason to\nworry about them at all. I would suggest showing the name _after_\nthe change (unless it is a deletion---instead of showing /dev/null\nto signal that it was deleted, show the original filename) for\nconsistency so that users can do \"show -p | grep '^Index: ' to see\nwhat resulting paths there are without missing the renamed ones.\n\n> but I just realized that a \"clickable paths\"\n> option already exists in some sense! There is a '--patch-with-raw'\n> option...\n\nI do not think that would be useful (neither --stat which would be\nmore commonly used for other reasosn), because these come at the top\nand by the time you see individual patch, they may be long scrolled\noff the top of the screen.\n\nOf course, the CVS \"Index:\" or \"rename to\" would be the same thing\nif a file is heavily modified, so it may not be too big a deal, but\nas I said, I never felt any need to double-click, so I wouldn't be\nthe best judge.\n"}]}