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

Hi,

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

quoted

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.

quoted

+	    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