On Wed, Apr 30, 2014 at 7:40 PM, Jonathan Nieder <jrnie...@gmail.com> wrote: > Hi, > > Nathan Collins wrote: > >> Patches created with 'diff.noprefix=true' don't 'git apply' without >> specifying '-p0'. >> >> I'm not sure this is a bug -- the 'man git-apply' just says "Reads the >> supplied diff output (i.e. "a patch") and applies it to files" -- but >> I would expect patches I create locally to apply cleanly locally. > > Sounds like a documentation bug, at least. Any ideas for clearer > wording?
Hmmm. Maybe a warning that the patch is expected to be in '-p1' format, and that setting 'diff.noprefix=true' makes some commands generate '-p0' patches? But I worry this would just confuse / distract the people that don't have 'diff.noprefix=true' set, which I expect is the majority of users. Better I think would be for 'git apply' to be smarter, as you suggest below. >> In >> real life the 'diff.noprefix=true' is in my ~/.gitconfig, so this was >> pretty confusing. > > I personally think setting diff.noprefix is not very sane (it also > breaks "patch -p1"), and I suppose I should have been louder about > that when it was introduced. > > Can you say more about the workflow you use that requires > diff.noprefix? Maybe we can make other changes to improve it, too. I have 'diff.noprefix=true' set so I can copy and paste paths from the 'git diff' output easily. I like to create small, logically independent commits, usually comprising a subset of my current changes. So, I do 'git diff' in one terminal, and then 'git add <path>' or 'git add --patch <path>' in another terminal to build up a commit (I suppose this is the work flow that 'git add --interactive' is designed for ...), where I get '<path>' from the diff by copying and pasting. With 'diff.noprefix=true', I can copy with double left click and paste with middle click; with 'diff.noprefix=false', to copy I instead have to carefully highlight the non-prefix part of the path in the diff, which is less convenient. > At first glance I don't suspect making diff.noprefix imply -p0 for > "git am" would be great, since that would generate the the opposite > problem when applying patches from the outside world. But maybe we > need better autodetection and maybe noprefix is a good signal about > when to use it. Autodetecting the lack or presence of the 'a/' and 'b/' prefixes seems like a great solution to me: externally user friendly and easy to implement internally. > Another complication is that unlike 'git diff', 'git apply' is > plumbing that is meant to be useful and reliable for scripts. And > unlike most plumbing, there is no higher-level command with similar > functionality for which we can experiment more freely with the UI. > Adding a new command to fix that might be a good direction toward > handling noprefix patches better. Related to 'git apply' being a scriptable plumbing command: naively I would expect there to be a "scripting mode" for Git commands which ignored the local configuration entirely (e.g. ~/.gitconfig). I've wanted this a few times and was surprised I could find no very sane way to achieve it. In fact, here's the corresponding question I posted on Stack Overflow while I was composing my original email (I wanted to be sure that 'diff.noprefix=true' was the only relevant part of my ~/.gitconfig, so I wanted disable my ~/.gitconfig entirely): http://stackoverflow.com/questions/23400449/how-to-make-git-temporarily-ignore-gitconfig > [...] >> git show | git apply --reverse > > The following which only uses plumbing commands should work: > > git diff-tree -p HEAD^! | > git apply --reverse Nice! However, I don't now how to generalize this solution to other (probably insane) use cases, e.g. git log -S<string> --patch | git apply --reverse (Context: http://stackoverflow.com/a/23401018/470844). > Thanks for some food for thought, > Jonathan Thanks for your reply. I didn't see it until today because a GMail filter ate it :P -nathan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
