Re: Fixing the p4merge launch shell script
On Wed, Oct 3, 2012 at 1:34 AM, Jeremy Morton ad...@game-point.net wrote: Junio C Hamano gitster at pobox.com writes: Jeremy Morton admin at game-point.net writes: I've noticed that the p4merge shell script could do with some improvement when it comes to merging. Because p4merge throws up an error when one of the files it's given to diff is /dev/null, git needs to create a temporary empty file and pass that to p4merge when diffing a file that has been created/deleted (eg. create file, git add ., git diff --cached). ... Thoughts? Is there an easier way to do this? Which version of git? Perhaps you do not have ec245ba (mergetool: Provide an empty file when needed, 2012-01-19) yet? That patch fixes the mergetool part, but the part I was referring to was the difftool part, which still has this problem. Best regards, Jeremy Morton (Jez) Thanks for the heads-up, Jez. I should have some time to take a look this weekend. If someone beats me to it I'll be happy to review the patches before then (Jeremy? ;-) Is `git diff` giving us /dev/null as the path here, and p4merge doesn't like that, or is something else going on? If that's indeed happening, then I would imagine a sensible thing for git-difftool--helper to do would be to check for /dev/null and create some temporary empty file. Does that sound like the right way to go? Another option would be for git-diff's external diff code to handle it at a much earlier point, perhaps guarded behind an option that git-difftool knows to pass (changing existing behavior is off the table). These both sound like pretty big changes given that we're getting ready for 1.8.0. If it's just p4merge that doesn't like this then perhaps the right decision is to paper over it in p4merge's mergetools/ scriptlet. It's certainly the least-impactful option. Thoughts? -- David -- 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: Fixing the p4merge launch shell script
Junio C Hamano gitster at pobox.com writes: Jeremy Morton admin at game-point.net writes: I've noticed that the p4merge shell script could do with some improvement when it comes to merging. Because p4merge throws up an error when one of the files it's given to diff is /dev/null, git needs to create a temporary empty file and pass that to p4merge when diffing a file that has been created/deleted (eg. create file, git add ., git diff --cached). ... Thoughts? Is there an easier way to do this? Which version of git? Perhaps you do not have ec245ba (mergetool: Provide an empty file when needed, 2012-01-19) yet? That patch fixes the mergetool part, but the part I was referring to was the difftool part, which still has this problem. Best regards, Jeremy Morton (Jez) -- 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
Fixing the p4merge launch shell script
Hi, I've noticed that the p4merge shell script could do with some improvement when it comes to merging. Because p4merge throws up an error when one of the files it's given to diff is /dev/null, git needs to create a temporary empty file and pass that to p4merge when diffing a file that has been created/deleted (eg. create file, git add ., git diff --cached). At the moment, the hack I'm using, which works, is to create a c:/blank.txt file which is an empty file, and modify libexec/git-core/mergetools/p4merge to start with this: diff_cmd () { if [ ! -f $LOCAL ] then LOCAL=C:/blank.txt fi if [ ! -f $REMOTE ] then REMOTE=C:/blank.txt fi $merge_tool_path $LOCAL $REMOTE } Obviously, this isn't good enough because c:/blank.txt probably won't exist on the system. It would be nice for the temporary blank file to have the same extension as the one that's been added, so we could diff (say) c:/users/jez/Temp/abc123_newFile.foo c:/development/bar/newfile.foo. However, I can't see a way to do this purely from within the shell script (even `mktemp` doesn't exist on all systems so that isn't good enough). I believe git needs to create this temporary blank file itself, much like it creates a temporary file for the previous version of a file that's been modified when it's being diff'd. It then needs to expose the location of this temporary file as a variable; say $LOCALBLANK. So, we would be able to modify the shell script to this: diff_cmd () { if [ ! -f $LOCAL ] then LOCAL=$LOCALBLANK fi if [ ! -f $REMOTE ] then REMOTE=$REMOTEBLANK fi $merge_tool_path $LOCAL $REMOTE } Thoughts? Is there an easier way to do this? -- Best regards, Jeremy Morton (Jez) -- 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: Fixing the p4merge launch shell script
Jeremy Morton ad...@game-point.net writes: I've noticed that the p4merge shell script could do with some improvement when it comes to merging. Because p4merge throws up an error when one of the files it's given to diff is /dev/null, git needs to create a temporary empty file and pass that to p4merge when diffing a file that has been created/deleted (eg. create file, git add ., git diff --cached). ... Thoughts? Is there an easier way to do this? Which version of git? Perhaps you do not have ec245ba (mergetool: Provide an empty file when needed, 2012-01-19) yet? -- 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