Re: [BUG?] Patches created with 'diff.noprefix=true' don't 'git apply'.
Nathan Collins nathan.coll...@gmail.com writes: What would you propose to make clickable in a renaming diff, though? Your 'Index' header looks good, and I would expect a renaming diff to have something like Index: foo - bar as in 'git status', Heh, please don't call Index: *mine* --- It is a CVS abomination ;-). For renames and copies, we do have separate rename from and rename to in the extended header part, so there is no reason to worry about them at all. I would suggest showing the name _after_ the change (unless it is a deletion---instead of showing /dev/null to signal that it was deleted, show the original filename) for consistency so that users can do show -p | grep '^Index: ' to see what resulting paths there are without missing the renamed ones. but I just realized that a clickable paths option already exists in some sense! There is a '--patch-with-raw' option... I do not think that would be useful (neither --stat which would be more commonly used for other reasosn), because these come at the top and by the time you see individual patch, they may be long scrolled off the top of the screen. Of course, the CVS Index: or rename to would be the same thing if a file is heavily modified, so it may not be too big a deal, but as I said, I never felt any need to double-click, so I wouldn't be the best judge. -- 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
Re: [BUG?] Patches created with 'diff.noprefix=true' don't 'git apply'.
Nathan Collins nathan.coll...@gmail.com writes: More concretely, what I had in mind was that if 'diff.noprefix=true' is set in the user's config, and the patch is in '-p0' format, then Git could suggest to the user that the 'diff.noprefix' setting *might* be causing them to generate '-p0' patches. If the user had in fact not generated the patch themselves, then they could safely ignore the suggestion. Issuing a suggestion that can be ignored too often sounds like crying wolf to train users to ignore all other more useful suggestions, so I would advise to be very cautious before doing so. If you cannot come up with a way to make the heuristics very sure and foolproof, I do not think it would help more than it hurt. One possibility I considered briefly before discarding it was to see if the input is coming from a pipe *and* these configuration is set. But this may just be an overcomplicated solution to my and others' misuse of the 'diff.noprefix' option; see below. So that is not workable. Yes. However, Before issuing this error message git -c diff.noprefix=true diff HEAD~ | git apply --reverse error: Data/Function/Decorator/Memoizer/Unsafe.hs: No such file or directory we _could_ check that there is Data/ directory in the target tree the patch is being applied and suggest to: To clarify, the actual path was src/Data/Function/Decorator/Memoizer/Unsafe.hs Yeah, and I meant to suggest checking that path is plausible. It would not help when adding a new file, but we can issue the original error message and that is perfectly readable, so catching only paths that are modified or deleted and suggesting -p$n is still making things better. 1. 'git apply' doesn't give a very helpful error message when the patch does not apply due to not being in '-p1' format. That Data/Function/... path in the error message is helpful enough, once you learn that -p$n strips the leading directory and that -p1 is the default. For new people, however, -p1 being the default might be unexpected, but the learning curve is not that steep, I would think. So this is a one-time thing. 2. the 'diff.noprefix=true' option is used for two unrelated things in practice. One of them is related to diffing -- namely, making Git generate '-p0' patches -- and the other is unrelated to diffing -- namely, users want file names that can be easily copied with double-click. I do not share this observation, as I never double-click. I let my shell to complete instead ;-) For (2), the solution may be to add a separate 'diff.add-clickable-paths' option (probably there is a better name? 'diff.add-copyable-paths'? ...),... ... Concretely, if 'diff.add-clickable-paths' is set, then instead of e.g. diff --git a/src/Data/Function/Decorator/Memoizer/Unsafe.hs b/src/Data/Function/Decorator/Memoizer index 3ef17da..a0586d3 100644 --- a/src/Data/Function/Decorator/Memoizer/Unsafe.hs +++ b/src/Data/Function/Decorator/Memoizer/Unsafe.hs If you do something along that line, perhaps Index: src/Data/Function/Decorator/Memoizer/Unsafe.hs diff --git a/src/Data/Function/Decorator/Memoizer/Unsafe.hs ... index 3ef17da..a0586d3 100644 --- a/src/Data/Function/Decorator/Memoizer/Unsafe.hs +++ b/src/Data/Function/Decorator/Memoizer/Unsafe.hs to imitate what cvs diff does may be more familar to people. What would you propose to make clickable in a renaming diff, though? -- 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
Re: [BUG?] Patches created with 'diff.noprefix=true' don't 'git apply'.
On Wed, May 7, 2014 at 11:42 AM, Junio C Hamano gits...@pobox.com wrote: Nathan Collins nathan.coll...@gmail.com writes: For (2), the solution may be to add a separate 'diff.add-clickable-paths' option (probably there is a better name? 'diff.add-copyable-paths'? ...),... ... Concretely, if 'diff.add-clickable-paths' is set, then instead of e.g. diff --git a/src/Data/Function/Decorator/Memoizer/Unsafe.hs b/src/Data/Function/Decorator/Memoizer index 3ef17da..a0586d3 100644 --- a/src/Data/Function/Decorator/Memoizer/Unsafe.hs +++ b/src/Data/Function/Decorator/Memoizer/Unsafe.hs If you do something along that line, perhaps Index: src/Data/Function/Decorator/Memoizer/Unsafe.hs diff --git a/src/Data/Function/Decorator/Memoizer/Unsafe.hs ... index 3ef17da..a0586d3 100644 --- a/src/Data/Function/Decorator/Memoizer/Unsafe.hs +++ b/src/Data/Function/Decorator/Memoizer/Unsafe.hs to imitate what cvs diff does may be more familar to people. What would you propose to make clickable in a renaming diff, though? Your 'Index' header looks good, and I would expect a renaming diff to have something like Index: foo - bar as in 'git status', but I just realized that a clickable paths option already exists in some sense! There is a '--patch-with-raw' option (which is short for '--patch' and '--raw', hahaha) which inserts clickable file names in the patch, above each diff. Moreover, it respects the '--relative' option, so you can get relative or absolute (relative repo root) clickable paths. It handles renaming by inserting the old and new paths separated by space. So then, having a way to make '--patch-with-raw' the default for all non-plumbing patch-producing commands would solve the clickable paths problem. In a summary, a possible complete solution: 1. improve Git apply error message: mention '-p$n' and '-p1' default, and report if path in patch makes sense for some non-default '-p' value. 2. improve 'diff.noprefix' documentation: tell user that this option is for producing '-p0' patches, and that using it to produce clickable paths is insane and may cause problems with generated patches. Suggest the user use '--patch-with-raw', and possibly '--relative', instead, or refer to (3). 3. add a Git config for making '--patch-with-raw' and optionally '--relative' the default for non-plumbing patch-producing commands. Cheers, -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
Re: [BUG?] Patches created with 'diff.noprefix=true' don't 'git apply'.
On Wed, May 7, 2014 at 4:39 PM, Nathan Collins nathan.coll...@gmail.com wrote: On Wed, May 7, 2014 at 11:42 AM, Junio C Hamano gits...@pobox.com wrote: Nathan Collins nathan.coll...@gmail.com writes: For (2), the solution may be to add a separate 'diff.add-clickable-paths' option (probably there is a better name? 'diff.add-copyable-paths'? ...),... ... Concretely, if 'diff.add-clickable-paths' is set, then instead of e.g. diff --git a/src/Data/Function/Decorator/Memoizer/Unsafe.hs b/src/Data/Function/Decorator/Memoizer index 3ef17da..a0586d3 100644 --- a/src/Data/Function/Decorator/Memoizer/Unsafe.hs +++ b/src/Data/Function/Decorator/Memoizer/Unsafe.hs If you do something along that line, perhaps Index: src/Data/Function/Decorator/Memoizer/Unsafe.hs diff --git a/src/Data/Function/Decorator/Memoizer/Unsafe.hs ... index 3ef17da..a0586d3 100644 --- a/src/Data/Function/Decorator/Memoizer/Unsafe.hs +++ b/src/Data/Function/Decorator/Memoizer/Unsafe.hs to imitate what cvs diff does may be more familar to people. What would you propose to make clickable in a renaming diff, though? Your 'Index' header looks good, and I would expect a renaming diff to have something like Index: foo - bar as in 'git status', but I just realized that a clickable paths option already exists in some sense! There is a '--patch-with-raw' option (which is short for '--patch' and '--raw', hahaha) which inserts clickable file names in the patch, above each diff. Or not: I stupidly only tested this with a single file modified: it turns out that all the clickable file names appear at the top of the patch, not as one file name above each corresponding diff as I claimed. D'oh, -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
Re: [BUG?] Patches created with 'diff.noprefix=true' don't 'git apply'.
On Wed, May 7, 2014 at 9:38 PM, Nathan Collins nathan.coll...@gmail.com wrote: On Wed, May 7, 2014 at 4:39 PM, Nathan Collins nathan.coll...@gmail.com wrote: On Wed, May 7, 2014 at 11:42 AM, Junio C Hamano gits...@pobox.com wrote: Nathan Collins nathan.coll...@gmail.com writes: For (2), the solution may be to add a separate 'diff.add-clickable-paths' option (probably there is a better name? 'diff.add-copyable-paths'? ...),... ... Concretely, if 'diff.add-clickable-paths' is set, then instead of e.g. diff --git a/src/Data/Function/Decorator/Memoizer/Unsafe.hs b/src/Data/Function/Decorator/Memoizer index 3ef17da..a0586d3 100644 --- a/src/Data/Function/Decorator/Memoizer/Unsafe.hs +++ b/src/Data/Function/Decorator/Memoizer/Unsafe.hs If you do something along that line, perhaps Index: src/Data/Function/Decorator/Memoizer/Unsafe.hs diff --git a/src/Data/Function/Decorator/Memoizer/Unsafe.hs ... index 3ef17da..a0586d3 100644 --- a/src/Data/Function/Decorator/Memoizer/Unsafe.hs +++ b/src/Data/Function/Decorator/Memoizer/Unsafe.hs to imitate what cvs diff does may be more familar to people. What would you propose to make clickable in a renaming diff, though? Your 'Index' header looks good, and I would expect a renaming diff to have something like Index: foo - bar as in 'git status', but I just realized that a clickable paths option already exists in some sense! There is a '--patch-with-raw' option (which is short for '--patch' and '--raw', hahaha) which inserts clickable file names in the patch, above each diff. Or not: I stupidly only tested this with a single file modified: it turns out that all the clickable file names appear at the top of the patch, not as one file name above each corresponding diff as I claimed. The following may be a non-option, since presumably many tools depend on the current Git patch format. The paths in the extended header lines in Git patches are clickable by default, and respect the '--relative' option. So, adding a path to the extended header lines that don't already have one would solve the clickable paths problem. E.g. index hash..hash mode becomes index hash..hash mode path The 'man git-diff' description of extended header lines in the Generating Patches with -p section: 2. It is followed by one or more extended header lines: old mode mode new mode mode deleted file mode mode new file mode mode copy from path copy to path rename from path rename to path similarity index number dissimilarity index number index hash..hash mode File modes are printed as 6-digit octal numbers including the file type and file permission bits. Path names in extended headers do not include the a/ and b/ prefixes. Cheers, -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
Re: [BUG?] Patches created with 'diff.noprefix=true' don't 'git apply'.
Nathan Collins nathan.coll...@gmail.com writes: 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? some? Do you have exceptions in mind? But I worry this would just confuse / distract the people that don't have 'diff.noprefix=true' set, Probably. But that would suggest that the place to improve the doc is for diff.noprefix configuration variable, no? Better I think would be for 'git apply' to be smarter, as you suggest below. As it is a plumbing command behind add -p, am, and friends, I would hate to see git apply pretend to be smarter than its users. When the user tells it to use -p0, it shouldn't guess, and when the user tells it to use -p1 by not giving any -p$n, it shouldn't guess. As long as we make it clear git apply without any explicit -p$n means the user is telling it to do -p1 in its documentation, I think it would be fine. 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. I share the same feeling ;-) But the boat has sailed, so the best we could do is to warn in its doc (i.e. where diff.noprefix is described) about its pitfalls. -- 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
Re: [BUG?] Patches created with 'diff.noprefix=true' don't 'git apply'.
On Tue, May 6, 2014 at 11:10 AM, Junio C Hamano gits...@pobox.com wrote: Nathan Collins nathan.coll...@gmail.com writes: 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? some? Do you have exceptions in mind? As Jonathan pointed out in his first reply, 'git diff-tree' ignores the 'diff.noprefix=true' setting. Compare git -c diff.noprefix=true diff HEAD~ with git -c diff.noprefix=true diff-tree -p HEAD (E.g. diff (git -c diff.noprefix=true diff HEAD~) (git -c diff.noprefix=true diff-tree -p HEAD) ) But I worry this would just confuse / distract the people that don't have 'diff.noprefix=true' set, Probably. But that would suggest that the place to improve the doc is for diff.noprefix configuration variable, no? I don't think that would actually help much in practice. The problem is that a person (like me) that set 'diff.noprefix=true' in their ~/.gitconfig months or years ago is unlikely to do 'man git-config' when 'git apply' fails. Having the warning in 'man git-apply' is better than (only) in 'man git-config', if making 'git apply' smarter is not an option. Better I think would be for 'git apply' to be smarter, as you suggest below. As it is a plumbing command behind add -p, am, and friends, I would hate to see git apply pretend to be smarter than its users. When the user tells it to use -p0, it shouldn't guess, and when the user tells it to use -p1 by not giving any -p$n, it shouldn't guess. Is there a non-plumbing command for applying patches not in mailboxes? I don't see how to replace '| git apply --reverse' with '| git am ???' here. As long as we make it clear git apply without any explicit -p$n means the user is telling it to do -p1 in its documentation, I think it would be fine. OK, then how about a smarter error message? Right now I get git -c diff.noprefix=true diff HEAD~ | git -c diff.noprefix=true apply --reverse error: Data/Function/Decorator/Memoizer/Unsafe.hs: No such file or directory vs git -c diff.noprefix=true diff HEAD~ | patch --reverse can't find file to patch at input line 5 Perhaps you should have used the -p or --strip option? [...] But 'git apply' could be much more helpful than 'patch' even, since the presence or absence of the 'a/' and 'b/' prefixes in the patch, and the 'diff.noprefix' setting, give Git enough info to be very helpful to the user. Cheers, -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
Re: [BUG?] Patches created with 'diff.noprefix=true' don't 'git apply'.
Nathan Collins nathan.coll...@gmail.com writes: But 'git apply' could be much more helpful than 'patch' even, since the presence or absence of the 'a/' and 'b/' prefixes in the patch, and the 'diff.noprefix' setting, give Git enough info to be very helpful to the user. The prefix would be unreliable as the generator may be using the mnemonicprefix option to use i/ and w/ prefixes. Worse yet, the configuration variables are for people who generated the diff you are feeding git apply, and there is nothing that tells apply that the patch is generated with _your_ setting. So that is not workable. However, Before issuing this error message git -c diff.noprefix=true diff HEAD~ | git apply --reverse error: Data/Function/Decorator/Memoizer/Unsafe.hs: No such file or directory we _could_ check that there is Data/ directory in the target tree the patch is being applied and suggest to: * use -p0, if noprefix, which I agree with Jonathan is an insane thing to use by default, is common enough; or * use different setting for -p$n, if noprefix is not common. in the error message. Extra computation necessary to do so would happen only in the error codepath, and we wouldn't mind spending some cycles if they help the end user. -- 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
Re: [BUG?] Patches created with 'diff.noprefix=true' don't 'git apply'.
On Tue, May 6, 2014 at 2:12 PM, Junio C Hamano gits...@pobox.com wrote: Nathan Collins nathan.coll...@gmail.com writes: But 'git apply' could be much more helpful than 'patch' even, since the presence or absence of the 'a/' and 'b/' prefixes in the patch, and the 'diff.noprefix' setting, give Git enough info to be very helpful to the user. The prefix would be unreliable as the generator may be using the mnemonicprefix option to use i/ and w/ prefixes. Worse yet, the configuration variables are for people who generated the diff you are feeding git apply, and there is nothing that tells apply that the patch is generated with _your_ setting. More concretely, what I had in mind was that if 'diff.noprefix=true' is set in the user's config, and the patch is in '-p0' format, then Git could suggest to the user that the 'diff.noprefix' setting *might* be causing them to generate '-p0' patches. If the user had in fact not generated the patch themselves, then they could safely ignore the suggestion. But this may just be an overcomplicated solution to my and others' misuse of the 'diff.noprefix' option; see below. So that is not workable. However, Before issuing this error message git -c diff.noprefix=true diff HEAD~ | git apply --reverse error: Data/Function/Decorator/Memoizer/Unsafe.hs: No such file or directory we _could_ check that there is Data/ directory in the target tree the patch is being applied and suggest to: To clarify, the actual path was src/Data/Function/Decorator/Memoizer/Unsafe.hs The path in the error message, Data/Function/Decorator/Memoizer/Unsafe.hs was the '-p1' version of that path. This is extra confusing if the user is unfamiliar with the '-p' option for patch and unaware that 'git apply' is assuming '-p1'. * use -p0, if noprefix, which I agree with Jonathan is an insane thing to use by default, is common enough; or * use different setting for -p$n, if noprefix is not common. in the error message. Extra computation necessary to do so would happen only in the error codepath, and we wouldn't mind spending some cycles if they help the end user. I'm starting to think there are really two separate issues here: 1. 'git apply' doesn't give a very helpful error message when the patch does not apply due to not being in '-p1' format. 2. the 'diff.noprefix=true' option is used for two unrelated things in practice. One of them is related to diffing -- namely, making Git generate '-p0' patches -- and the other is unrelated to diffing -- namely, users want file names that can be easily copied with double-click. For (1), I think the solution is check if the patch makes sense as '-p0', in the error case, and tell the user about this in the error message as you suggested above. In fact, in case the '-p1' path doesn't exist, Git could just try all possible '-p$n' values, and report the first that yields valid paths, if any. Mentioning to the user that they have 'diff.noprefix=true' set in case '-p0' is discovered might be helpful, but a better solution to (2) might eliminate this problem in practice. For (2), the solution may be to add a separate 'diff.add-clickable-paths' option (probably there is a better name? 'diff.add-copyable-paths'? ...), which makes Git insert clickable paths in the comments in the diff output. This handles the clickable-paths use case which lead me and others to abuse 'diff.noprefix=true'. Examples where people suggest using 'diff.noprefix=true' to make it easier to double-click copy paths include [1 - 5]. Examples where people suggest using 'noprefix' to generate '-p0' patches include [6 - 10]. Concretely, if 'diff.add-clickable-paths' is set, then instead of e.g. diff --git a/src/Data/Function/Decorator/Memoizer/Unsafe.hs b/src/Data/Function/Decorator/Memoizer index 3ef17da..a0586d3 100644 --- a/src/Data/Function/Decorator/Memoizer/Unsafe.hs +++ b/src/Data/Function/Decorator/Memoizer/Unsafe.hs we get e.g. # src/Data/Function/Decorator/Memoizer/Unsafe.hs diff --git a/src/Data/Function/Decorator/Memoizer/Unsafe.hs b/src/Data/Function/Decorator/Memoizer index 3ef17da..a0586d3 100644 --- a/src/Data/Function/Decorator/Memoizer/Unsafe.hs +++ b/src/Data/Function/Decorator/Memoizer/Unsafe.hs as the diff header. In any case, warning the user in the 'diff.noprefix' docs that the point of the option is to create '-p0' patches, and that setting this permanently will cause bad interactions with other Git commands (like 'git apply') seems like a great idea -- you suggested this in your first email, but I hadn't really understood why my use of 'diff.noprefix' was insane yet. This probably won't help people that blindly use 'noprefix' in the insane way based on a suggestion they found with Google, but it can't hurt. If there were a 'diff.add-clickable-paths' option, then the 'diff.noprefix' docs could also mention this, and suggest the user use that instead if their use case is the easy copy use case. Thank you
Re: [BUG?] Patches created with 'diff.noprefix=true' don't 'git apply'.
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 -Sstring --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
Re: [BUG?] Patches created with 'diff.noprefix=true' don't 'git apply'.
Nathan Collins wrote: On Wed, Apr 30, 2014 at 7:40 PM, Jonathan Nieder jrnie...@gmail.com wrote: Nathan Collins wrote: 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 -Sstring --patch | git apply --reverse This should do it: git rev-list HEAD | git diff-tree --no-commit-id -p -Sstring --stdin | git apply --reverse More generally, when scripting plumbing commands tend to do the right thing. Will think more about the documentation and other parts (or if someone else sends a patch before I can, I won't mind). Thanks, Jonathan -- 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
Re: [BUG?] Patches created with 'diff.noprefix=true' don't 'git apply'.
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? 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. 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. 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. [...] git show | git apply --reverse The following which only uses plumbing commands should work: git diff-tree -p HEAD^! | git apply --reverse Thanks for some food for thought, Jonathan -- 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
