{"thread":{"id":"7357","subject":"[PATCH] Teach git-mergetool about Apple's opendiff/FileMerge","startedAt":"2007-03-22T21:37:28Z","lastAt":"2007-03-29T14:03:12Z","messageCount":7,"participants":["Arjen Laarhoven","Junio C Hamano","Steven Grimm","Theodore Tso","Marco Roeland"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"37764","messageId":"20070322213728.GD3854@regex.yaph.org","threadId":"7357","inReplyTo":null,"subject":"[PATCH] Teach git-mergetool about Apple's opendiff/FileMerge","fromName":"Arjen Laarhoven","fromEmail":"arjen@yaph.org","sentAt":"2007-03-22T21:37:28Z","receivedAt":"2007-03-22T21:37:28Z","isPatch":true,"sender":{"key":"arjen@yaph.org","avatar":"https://gravatar.com/avatar/f776c2c0c5ea62d70827b942eb7d95ce85661a3d70bc3f03cf9773815c599c01?d=mp&s=160"},"body":"\nSigned-off-by: Arjen Laarhoven <arjen@yaph.org>\n---\n git-mergetool.sh |   30 ++++++++++++++++++++++++++++--\n 1 files changed, 28 insertions(+), 2 deletions(-)\n\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex 7942fd0..58ae201 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -248,6 +248,30 @@ merge_file () {\n \t\tmv -- \"$BACKUP\" \"$path.orig\"\n \t    fi\n \t    ;;\n+\topendiff)\n+\t    touch \"$BACKUP\"\n+\t    if base_present; then\n+\t\topendiff $LOCAL $REMOTE -ancestor $BASE -merge $path | cat\n+            else\n+                opendiff $LOCAL $REMOTE -merge $path | cat\n+            fi\n+\t    if test \"$path\" -nt \"$BACKUP\" ; then\n+\t\tstatus=0;\n+\t    else\n+\t\twhile true; do\n+\t\t    echo \"$path seems unchanged.\"\n+\t\t    echo -n \"Was the merge successful? [y/n] \"\n+\t\t    read answer < /dev/tty\n+\t\t    case \"$answer\" in\n+\t\t\ty*|Y*) status=0; break ;;\n+\t\t\tn*|N*) status=1; break ;;\n+\t\t    esac\n+\t\tdone\n+\t    fi\n+\t    if test \"$status\" -eq 0; then\n+\t\tmv -- \"$BACKUP\" \"$path.orig\"\n+\t    fi\n+\t    ;;\n     esac\n     if test \"$status\" -ne 0; then\n \techo \"merge of $path failed\" 1>&2\n@@ -289,7 +313,7 @@ done\n if test -z \"$merge_tool\"; then\n     merge_tool=`git-config merge.tool`\n     case \"$merge_tool\" in\n-\tkdiff3 | tkdiff | xxdiff | meld | emerge | vimdiff)\n+\tkdiff3 | tkdiff | xxdiff | meld | emerge | vimdiff | opendiff)\n \t    ;; # happy\n \t*)\n \t    echo >&2 \"git config option merge.tool set to unknown tool: $merge_tool\"\n@@ -312,6 +336,8 @@ if test -z \"$merge_tool\" ; then\n \tmerge_tool=emerge\n     elif type vimdiff >/dev/null 2>&1; then\n \tmerge_tool=vimdiff\n+    elif type opendiff >/dev/null 2>&1; then\n+\tmerge_tool=opendiff\n     else\n \techo \"No available merge resolution programs available.\"\n \texit 1\n@@ -319,7 +345,7 @@ if test -z \"$merge_tool\" ; then\n fi\n \n case \"$merge_tool\" in\n-    kdiff3|tkdiff|meld|xxdiff|vimdiff)\n+    kdiff3|tkdiff|meld|xxdiff|vimdiff|opendiff)\n \tif ! type \"$merge_tool\" > /dev/null 2>&1; then\n \t    echo \"The merge tool $merge_tool is not available\"\n \t    exit 1\n-- \n1.5.1.rc1.13.g0872\n"},{"id":"37782","messageId":"7vbqiksh4a.fsf@assigned-by-dhcp.cox.net","threadId":"7357","inReplyTo":"20070322213728.GD3854@regex.yaph.org","subject":"Re: [PATCH] Teach git-mergetool about Apple's opendiff/FileMerge","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-23T04:45:09Z","receivedAt":"2007-03-23T04:45:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"arjen@yaph.org (Arjen Laarhoven) writes:\n\n> Signed-off-by: Arjen Laarhoven <arjen@yaph.org>\n\nI cannot comment on the calling interface of opendiff, as I do\nnot have access to an Apple.  Here are my first impressions.\n\n> diff --git a/git-mergetool.sh b/git-mergetool.sh\n> index 7942fd0..58ae201 100755\n> --- a/git-mergetool.sh\n> +++ b/git-mergetool.sh\n> @@ -248,6 +248,30 @@ merge_file () {\n>  \t\tmv -- \"$BACKUP\" \"$path.orig\"\n>  \t    fi\n>  \t    ;;\n> +\topendiff)\n> +\t    touch \"$BACKUP\"\n> +\t    if base_present; then\n> +\t\topendiff $LOCAL $REMOTE -ancestor $BASE -merge $path | cat\n> +            else\n> +                opendiff $LOCAL $REMOTE -merge $path | cat\n> +            fi\n\nI sense inconsistent tabbing here.\n\nMore seriously, all of the above $variable references must be\ndq'ed; see other case arms for good examples.\n\nWhat's the purpose of this cat anyway?  It looks like an\nexpensive no-op to me.\n\n> +\t    if test \"$path\" -nt \"$BACKUP\" ; then\n> +\t\tstatus=0;\n> +\t    else\n> +\t\twhile true; do\n> +\t\t    echo \"$path seems unchanged.\"\n> +\t\t    echo -n \"Was the merge successful? [y/n] \"\n> +\t\t    read answer < /dev/tty\n> +\t\t    case \"$answer\" in\n> +\t\t\ty*|Y*) status=0; break ;;\n> +\t\t\tn*|N*) status=1; break ;;\n> +\t\t    esac\n> +\t\tdone\n> +\t    fi\n> +\t    if test \"$status\" -eq 0; then\n> +\t\tmv -- \"$BACKUP\" \"$path.orig\"\n> +\t    fi\n> +\t    ;;\n>      esac\n\nThis part is duplicated across meld|vimdiff and xxdiff arms; you\nprobably would want to have a patch that makes a shell function\nto factor this out, and then another patch to add this opendiff\nsupport.\n"},{"id":"37783","messageId":"46035D0B.8020009@midwinter.com","threadId":"7357","inReplyTo":"7vbqiksh4a.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Teach git-mergetool about Apple's opendiff/FileMerge","fromName":"Steven Grimm","fromEmail":"koreth@midwinter.com","sentAt":"2007-03-23T04:52:27Z","receivedAt":"2007-03-23T04:52:27Z","isPatch":true,"sender":{"key":"koreth@midwinter.com","avatar":"https://gravatar.com/avatar/71b4d2e8b62f168bdc9e9205341159e3567003b4f9e2127c617c5fa0a1f5bad2?d=mp&s=160"},"body":"Junio C Hamano wrote:\n>> +                opendiff $LOCAL $REMOTE -merge $path | cat\n> What's the purpose of this cat anyway?  It looks like an\n> expensive no-op to me.\n>   \n\nIf stdout is a tty, opendiff appears to background itself automatically. \nI assume he wanted to prevent that without losing any output from the \ncommand.\n\n-Steve\n"},{"id":"37790","messageId":"20070323082501.GF3854@regex.yaph.org","threadId":"7357","inReplyTo":"7vbqiksh4a.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Teach git-mergetool about Apple's opendiff/FileMerge","fromName":"Arjen Laarhoven","fromEmail":"arjen@yaph.org","sentAt":"2007-03-23T08:25:01Z","receivedAt":"2007-03-23T08:25:01Z","isPatch":true,"sender":{"key":"arjen@yaph.org","avatar":"https://gravatar.com/avatar/f776c2c0c5ea62d70827b942eb7d95ce85661a3d70bc3f03cf9773815c599c01?d=mp&s=160"},"body":"Hi,\n\n> I cannot comment on the calling interface of opendiff, as I do\n> not have access to an Apple.  Here are my first impressions.\n> \n> > diff --git a/git-mergetool.sh b/git-mergetool.sh\n> > index 7942fd0..58ae201 100755\n> > --- a/git-mergetool.sh\n> > +++ b/git-mergetool.sh\n> > @@ -248,6 +248,30 @@ merge_file () {\n> >  \t\tmv -- \"$BACKUP\" \"$path.orig\"\n> >  \t    fi\n> >  \t    ;;\n> > +\topendiff)\n> > +\t    touch \"$BACKUP\"\n> > +\t    if base_present; then\n> > +\t\topendiff $LOCAL $REMOTE -ancestor $BASE -merge $path | cat\n> > +            else\n> > +                opendiff $LOCAL $REMOTE -merge $path | cat\n> > +            fi\n> \n> I sense inconsistent tabbing here.\n\nSomehow I missed this.\n\n> More seriously, all of the above $variable references must be\n> dq'ed; see other case arms for good examples.\n\nI don't use shell scripting  much, some reading up on quoting\nenlightened me :-)\n\n> What's the purpose of this cat anyway?  It looks like an\n> expensive no-op to me.\n\nopendiff is a wrapper for the FileMerge.app application.  It launches the\nFileMerge binary with the expanded filenames and returns immediately,\nwhich is confusing, as git-mergetool immediately continues.  When the\noutput of opendiff is piped somewhere, it'll wait until FileMerge is\nexited (and the user has had a chance to save the merged file).\n\nI think there is another solution, I'll look into this.\n\n> > +\t    if test \"$path\" -nt \"$BACKUP\" ; then\n> > +\t\tstatus=0;\n> > +\t    else\n> > +\t\twhile true; do\n> > +\t\t    echo \"$path seems unchanged.\"\n> > +\t\t    echo -n \"Was the merge successful? [y/n] \"\n> > +\t\t    read answer < /dev/tty\n> > +\t\t    case \"$answer\" in\n> > +\t\t\ty*|Y*) status=0; break ;;\n> > +\t\t\tn*|N*) status=1; break ;;\n> > +\t\t    esac\n> > +\t\tdone\n> > +\t    fi\n> > +\t    if test \"$status\" -eq 0; then\n> > +\t\tmv -- \"$BACKUP\" \"$path.orig\"\n> > +\t    fi\n> > +\t    ;;\n> >      esac\n> \n> This part is duplicated across meld|vimdiff and xxdiff arms; you\n> probably would want to have a patch that makes a shell function\n> to factor this out, and then another patch to add this opendiff\n> support.\n\nWill do.\n\nArjen\n"},{"id":"37813","messageId":"20070323141519.GA19649@thunk.org","threadId":"7357","inReplyTo":"20070322213728.GD3854@regex.yaph.org","subject":"Re: [PATCH] Teach git-mergetool about Apple's opendiff/FileMerge","fromName":"Theodore Tso","fromEmail":"tytso@mit.edu","sentAt":"2007-03-23T14:15:19Z","receivedAt":"2007-03-23T14:15:19Z","isPatch":true,"sender":{"key":"tytso@mit.edu","avatar":"https://avatars.githubusercontent.com/u/51416?v=4"},"body":"Thanks for extending mergetool to use opendiff!\n\nI won't have access to my MacOS X machine until I get back home, so I\nwon't be able to try out your patch until early next week.  Is\nopendiff in the standard MacOS release, or do I have to do something\nspecial to get it?\n\nThanks,\n\n\t\t\t\t\t- Ted\n"},{"id":"37832","messageId":"20070323184235.GA11195@fiberbit.xs4all.nl","threadId":"7357","inReplyTo":"20070323141519.GA19649@thunk.org","subject":"Re: [PATCH] Teach git-mergetool about Apple's opendiff/FileMerge","fromName":"Marco Roeland","fromEmail":"marco.roeland@xs4all.nl","sentAt":"2007-03-23T18:42:35Z","receivedAt":"2007-03-23T18:42:35Z","isPatch":true,"sender":{"key":"marco.roeland@xs4all.nl","avatar":null},"body":"On Friday March 23rd 2007 at 10:15 Theodore Tso wrote:\n\n> I won't have access to my MacOS X machine until I get back home, so I\n> won't be able to try out your patch until early next week.  Is\n> opendiff in the standard MacOS release, or do I have to do something\n> special to get it?\n\nThe man page says: \"opendiff and FileMerge are installed as part of the\nMac OS X Developer Tools\" and it looks quite nice!\n-- \nMarco Roeland\n"},{"id":"38312","messageId":"20070329140312.GG2913@thunk.org","threadId":"7357","inReplyTo":"20070322213728.GD3854@regex.yaph.org","subject":"Re: [PATCH] Teach git-mergetool about Apple's opendiff/FileMerge","fromName":"Theodore Tso","fromEmail":"tytso@mit.edu","sentAt":"2007-03-29T14:03:12Z","receivedAt":"2007-03-29T14:03:12Z","isPatch":true,"sender":{"key":"tytso@mit.edu","avatar":"https://avatars.githubusercontent.com/u/51416?v=4"},"body":"Hi Arjen,\n\n\tThe version of your patch which I just checked into my sources\nfixes the issues which Junio raised (whitespace issues, double quotes,\nfactoring out common code).  The other change I made was that I\nchanged the search order so that by default opendiff is preferred over\nemerge (on the assumption that MacOS developers are more likely to\nwant to use the GUI merge tool than emacs's merge tool).  Of course,\npeople are free to set whatever they choose in their .gitconfig file.\n\n\tThanks for the patch!\n\n\t\t\t\t\t\t- Ted\n"}]}