[PATCH] Teach git-mergetool about Apple's opendiff/FileMerge

[PATCH] Teach git-mergetool about Apple's opendiff/FileMerge

[Arjen Laarhoven]

Signed-off-by: Arjen Laarhoven <arjen@yaph.org>
---
 git-mergetool.sh |   30 ++++++++++++++++++++++++++++--
 1 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/git-mergetool.sh b/git-mergetool.sh
index 7942fd0..58ae201 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -248,6 +248,30 @@ merge_file () {
 		mv -- "$BACKUP" "$path.orig"
 	    fi
 	    ;;
+	opendiff)
+	    touch "$BACKUP"
+	    if base_present; then
+		opendiff $LOCAL $REMOTE -ancestor $BASE -merge $path | cat
+            else
+                opendiff $LOCAL $REMOTE -merge $path | cat
+            fi
+	    if test "$path" -nt "$BACKUP" ; then
+		status=0;
+	    else
+		while true; do
+		    echo "$path seems unchanged."
+		    echo -n "Was the merge successful? [y/n] "
+		    read answer < /dev/tty
+		    case "$answer" in
+			y*|Y*) status=0; break ;;
+			n*|N*) status=1; break ;;
+		    esac
+		done
+	    fi
+	    if test "$status" -eq 0; then
+		mv -- "$BACKUP" "$path.orig"
+	    fi
+	    ;;
     esac
     if test "$status" -ne 0; then
 	echo "merge of $path failed" 1>&2
@@ -289,7 +313,7 @@ done
 if test -z "$merge_tool"; then
     merge_tool=`git-config merge.tool`
     case "$merge_tool" in
-	kdiff3 | tkdiff | xxdiff | meld | emerge | vimdiff)
+	kdiff3 | tkdiff | xxdiff | meld | emerge | vimdiff | opendiff)
 	    ;; # happy
 	*)
 	    echo >&2 "git config option merge.tool set to unknown tool: $merge_tool"
@@ -312,6 +336,8 @@ if test -z "$merge_tool" ; then
 	merge_tool=emerge
     elif type vimdiff >/dev/null 2>&1; then
 	merge_tool=vimdiff
+    elif type opendiff >/dev/null 2>&1; then
+	merge_tool=opendiff
     else
 	echo "No available merge resolution programs available."
 	exit 1
@@ -319,7 +345,7 @@ if test -z "$merge_tool" ; then
 fi
 
 case "$merge_tool" in
-    kdiff3|tkdiff|meld|xxdiff|vimdiff)
+    kdiff3|tkdiff|meld|xxdiff|vimdiff|opendiff)
 	if ! type "$merge_tool" > /dev/null 2>&1; then
 	    echo "The merge tool $merge_tool is not available"
 	    exit 1
-- 
1.5.1.rc1.13.g0872

Re: [PATCH] Teach git-mergetool about Apple's opendiff/FileMerge

[Junio C Hamano]

arjen@yaph.org (Arjen Laarhoven) writes:

> Signed-off-by: Arjen Laarhoven <arjen@yaph.org>

I cannot comment on the calling interface of opendiff, as I do
not have access to an Apple.  Here are my first impressions.

> diff --git a/git-mergetool.sh b/git-mergetool.sh
> index 7942fd0..58ae201 100755
> --- a/git-mergetool.sh
> +++ b/git-mergetool.sh
> @@ -248,6 +248,30 @@ merge_file () {
>  		mv -- "$BACKUP" "$path.orig"
>  	    fi
>  	    ;;
> +	opendiff)
> +	    touch "$BACKUP"
> +	    if base_present; then
> +		opendiff $LOCAL $REMOTE -ancestor $BASE -merge $path | cat
> +            else
> +                opendiff $LOCAL $REMOTE -merge $path | cat
> +            fi

I sense inconsistent tabbing here.

More seriously, all of the above $variable references must be
dq'ed; see other case arms for good examples.

What's the purpose of this cat anyway?  It looks like an
expensive no-op to me.

> +	    if test "$path" -nt "$BACKUP" ; then
> +		status=0;
> +	    else
> +		while true; do
> +		    echo "$path seems unchanged."
> +		    echo -n "Was the merge successful? [y/n] "
> +		    read answer < /dev/tty
> +		    case "$answer" in
> +			y*|Y*) status=0; break ;;
> +			n*|N*) status=1; break ;;
> +		    esac
> +		done
> +	    fi
> +	    if test "$status" -eq 0; then
> +		mv -- "$BACKUP" "$path.orig"
> +	    fi
> +	    ;;
>      esac

This part is duplicated across meld|vimdiff and xxdiff arms; you
probably would want to have a patch that makes a shell function
to factor this out, and then another patch to add this opendiff
support.

Re: [PATCH] Teach git-mergetool about Apple's opendiff/FileMerge

[Steven Grimm]

Junio C Hamano wrote:
>> +                opendiff $LOCAL $REMOTE -merge $path | cat
> What's the purpose of this cat anyway?  It looks like an
> expensive no-op to me.
>   

If stdout is a tty, opendiff appears to background itself automatically. 
I assume he wanted to prevent that without losing any output from the 
command.

-Steve

Re: [PATCH] Teach git-mergetool about Apple's opendiff/FileMerge

[Arjen Laarhoven]

Hi,

> I cannot comment on the calling interface of opendiff, as I do
> not have access to an Apple.  Here are my first impressions.
> 
> > diff --git a/git-mergetool.sh b/git-mergetool.sh
> > index 7942fd0..58ae201 100755
> > --- a/git-mergetool.sh
> > +++ b/git-mergetool.sh
> > @@ -248,6 +248,30 @@ merge_file () {
> >  		mv -- "$BACKUP" "$path.orig"
> >  	    fi
> >  	    ;;
> > +	opendiff)
> > +	    touch "$BACKUP"
> > +	    if base_present; then
> > +		opendiff $LOCAL $REMOTE -ancestor $BASE -merge $path | cat
> > +            else
> > +                opendiff $LOCAL $REMOTE -merge $path | cat
> > +            fi
> 
> I sense inconsistent tabbing here.

Somehow I missed this.

> More seriously, all of the above $variable references must be
> dq'ed; see other case arms for good examples.

I don't use shell scripting  much, some reading up on quoting
enlightened me :-)

> What's the purpose of this cat anyway?  It looks like an
> expensive no-op to me.

opendiff is a wrapper for the FileMerge.app application.  It launches the
FileMerge binary with the expanded filenames and returns immediately,
which is confusing, as git-mergetool immediately continues.  When the
output of opendiff is piped somewhere, it'll wait until FileMerge is
exited (and the user has had a chance to save the merged file).

I think there is another solution, I'll look into this.

> > +	    if test "$path" -nt "$BACKUP" ; then
> > +		status=0;
> > +	    else
> > +		while true; do
> > +		    echo "$path seems unchanged."
> > +		    echo -n "Was the merge successful? [y/n] "
> > +		    read answer < /dev/tty
> > +		    case "$answer" in
> > +			y*|Y*) status=0; break ;;
> > +			n*|N*) status=1; break ;;
> > +		    esac
> > +		done
> > +	    fi
> > +	    if test "$status" -eq 0; then
> > +		mv -- "$BACKUP" "$path.orig"
> > +	    fi
> > +	    ;;
> >      esac
> 
> This part is duplicated across meld|vimdiff and xxdiff arms; you
> probably would want to have a patch that makes a shell function
> to factor this out, and then another patch to add this opendiff
> support.

Will do.

Arjen

Re: [PATCH] Teach git-mergetool about Apple's opendiff/FileMerge

[Theodore Tso]

Thanks for extending mergetool to use opendiff!

I won't have access to my MacOS X machine until I get back home, so I
won't be able to try out your patch until early next week.  Is
opendiff in the standard MacOS release, or do I have to do something
special to get it?

Thanks,

					- Ted

Re: [PATCH] Teach git-mergetool about Apple's opendiff/FileMerge

[Marco Roeland]

On Friday March 23rd 2007 at 10:15 Theodore Tso wrote:

> I won't have access to my MacOS X machine until I get back home, so I
> won't be able to try out your patch until early next week.  Is
> opendiff in the standard MacOS release, or do I have to do something
> special to get it?

The man page says: "opendiff and FileMerge are installed as part of the
Mac OS X Developer Tools" and it looks quite nice!
-- 
Marco Roeland

Re: [PATCH] Teach git-mergetool about Apple's opendiff/FileMerge

[Theodore Tso]

Hi Arjen,

	The version of your patch which I just checked into my sources
fixes the issues which Junio raised (whitespace issues, double quotes,
factoring out common code).  The other change I made was that I
changed the search order so that by default opendiff is preferred over
emerge (on the assumption that MacOS developers are more likely to
want to use the GUI merge tool than emacs's merge tool).  Of course,
people are free to set whatever they choose in their .gitconfig file.

	Thanks for the patch!

						- Ted