threads / discuss / 8541

Please pull mergetool.git

Subject: Please pull mergetool.git

## tl;dr

7 messages between Jun 10, 2007 and Jun 11, 2007.

replies: 6people: 4as markdown or json

Theodore Ts'o· Jun 10, 2007, 16:03 UTC · lore
Hi Junio,
Please pull from the "mergetool" branch at:
	git://repo.or.cz/git/mergetool.git mergetool

It adds support for vimdiff/gvimdiff as a mergetool program, as well Josh's suggestion of making the default merge-tool selection more intelligent, although I've rewritten it somewhat take into account the comments made by you and others on the git mailing list.

(Note that as of this writing, meld is a pretty sad/dificient tool, and even GNOME users may very well prefer kdiff3 over meld --- which is my default. Still, it seems reasonable to default to using KDE tools in an KDE login session, and GNOME tools in a GNOME login session, and people who care differently should set their own preferences in ~/.gitconfig.)

						- Ted
Dan McGee (1):
      git-mergetool: Allow gvimdiff to be used as a mergetool
Theodore Ts'o (2):
      git-mergetool: Make default selection of merge-tool more intelligent
      Add git-applymbox, git-applypatch, and *~ to .gitignore
 b/.gitignore                      |    3 ++
 b/Documentation/config.txt        |    2 -
 b/Documentation/git-mergetool.txt |    2 -
 b/git-mergetool.sh                |   46 +++++++++++++++++++++++---------------
 git-mergetool.sh                  |   12 ++++++++-
 5 files changed, 44 insertions(+), 21 deletions(-)
diff --git a/.gitignore b/.gitignore
index 27e5aeb..70a4a68 100644
--- a/.gitignore
+++ b/.gitignore
@@ -7,6 +7,8 @@ git-add--interactive
 git-am
 git-annotate
 git-apply
+git-applymbox
+git-applypatch
 git-archimport
 git-archive
 git-bisect
@@ -161,6 +163,7 @@ git-core.spec
 *.exe
 *.[ao]
 *.py[co]
+*~
 config.mak
 autom4te.cache
 config.cache
diff --git a/Documentation/config.txt b/Documentation/config.txt
index de408b6..a2057d9 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -531,7 +531,7 @@ merge.summary::
 merge.tool::
 	Controls which merge resolution program is used by
 	gitlink:git-mergetool[l].  Valid values are: "kdiff3", "tkdiff",
-	"meld", "xxdiff", "emerge", "vimdiff", and "opendiff"
+	"meld", "xxdiff", "emerge", "vimdiff", "gvimdiff", and "opendiff".
 
 merge.verbosity::
 	Controls the amount of output shown by the recursive merge
diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index b89c51c..6c32c6d 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -25,7 +25,7 @@ OPTIONS
 -t or --tool=<tool>::
 	Use the merge resolution program specified by <tool>.
 	Valid merge tools are:
-	kdiff3, tkdiff, meld, xxdiff, emerge, vimdiff, and opendiff
+	kdiff3, tkdiff, meld, xxdiff, emerge, vimdiff, gvimdiff, and opendiff
 +
 If a merge resolution program is not specified, 'git mergetool'
 will use the configuration variable merge.tool.  If the
diff --git a/git-mergetool.sh b/git-mergetool.sh
index bb21b03..7b66309 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -215,6 +215,12 @@ merge_file () {
 	    check_unchanged
 	    save_backup
 	    ;;
+	gvimdiff)
+		touch "$BACKUP"
+		gvimdiff -f -- "$LOCAL" "$path" "$REMOTE"
+		check_unchanged
+		save_backup
+		;;
 	xxdiff)
 	    touch "$BACKUP"
 	    if base_present ; then
@@ -293,7 +299,7 @@ done
 if test -z "$merge_tool"; then
     merge_tool=`git-config merge.tool`
     case "$merge_tool" in
-	kdiff3 | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | "")
+	kdiff3 | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | gvimdiff | "")
 	    ;; # happy
 	*)
 	    echo >&2 "git config option merge.tool set to unknown tool: $merge_tool"
@@ -304,28 +310,42 @@ if test -z "$merge_tool"; then
 fi
 
 if test -z "$merge_tool" ; then
-    if type kdiff3 >/dev/null 2>&1 && test -n "$DISPLAY"; then
-	merge_tool="kdiff3";
-    elif type tkdiff >/dev/null 2>&1 && test -n "$DISPLAY"; then
-	merge_tool=tkdiff
-    elif type xxdiff >/dev/null 2>&1 && test -n "$DISPLAY"; then
-	merge_tool=xxdiff
-    elif type meld >/dev/null 2>&1 && test -n "$DISPLAY"; then
-	merge_tool=meld
-    elif type opendiff >/dev/null 2>&1; then
-	merge_tool=opendiff
-    elif type emacs >/dev/null 2>&1; then
-	merge_tool=emerge
-    elif type vimdiff >/dev/null 2>&1; then
-	merge_tool=vimdiff
-    else
+    if test -n "$DISPLAY"; then
+        merge_tool_candidates="kdiff3 tkdiff xxdiff meld gvimdiff"
+        if test -n "$GNOME_DESKTOP_SESSION_ID" ; then
+            merge_tool_candidates="meld $merge_tool_candidates"
+        fi
+        if test "$KDE_FULL_SESSION" = "true"; then
+            merge_tool_candidates="kdiff3 $merge_tool_candidates"
+        fi
+    fi
+    if echo "${VISUAL:-$EDITOR}" | grep 'emacs' > /dev/null 2>&1; then
+        merge_tool_candidates="$merge_tool_candidates emerge"
+    fi
+    if echo "${VISUAL:-$EDITOR}" | grep 'vim' > /dev/null 2>&1; then
+        merge_tool_candidates="$merge_tool_candidates vimdiff"
+    fi
+    merge_tool_candidates="$merge_tool_candidates opendiff emerge vimdiff"
+    echo "merge tool candidates: $merge_tool_candidates"
+    for i in $merge_tool_candidates; do
+        if test $i = emerge ; then
+            cmd=emacs
+        else
+            cmd=$i
+        fi
+        if type $cmd > /dev/null 2>&1; then
+            merge_tool=$i
+            break
+        fi
+    done
+    if test -z "$merge_tool" ; then
 	echo "No available merge resolution programs available."
 	exit 1
     fi
 fi
 
 case "$merge_tool" in
-    kdiff3|tkdiff|meld|xxdiff|vimdiff|opendiff)
+    kdiff3|tkdiff|meld|xxdiff|vimdiff|gvimdiff|opendiff)
 	if ! type "$merge_tool" > /dev/null 2>&1; then
 	    echo "The merge tool $merge_tool is not available"
 	    exit 1
Junio C Hamano· Jun 10, 2007, 19:55 UTC · re: Theodore Ts'o · lore

Re: Please pull mergetool.git

"Theodore Ts'o" <tytso@mit.edu> writes:
Show 13 quoted lines
> Please pull from the "mergetool" branch at:
> 	git://repo.or.cz/git/mergetool.git mergetool
>
> It adds support for vimdiff/gvimdiff as a mergetool program, as well
> Josh's suggestion of making the default merge-tool selection more
> intelligent, although I've rewritten it somewhat take into account the
> comments made by you and others on the git mailing list.
>
> (Note that as of this writing, meld is a pretty sad/dificient tool, and
> even GNOME users may very well prefer kdiff3 over meld --- which is my
> default.  Still, it seems reasonable to default to using KDE tools in an
> KDE login session, and GNOME tools in a GNOME login session, and people
> who care differently should set their own preferences in ~/.gitconfig.)
Thanks for keeping track of the mergetool.

I do not see problems in the mergetool part, other than that I mildly suspect that opendiff -- actually FileMerge -- might want to be in the test -n "$DISPLAY" section, but that is inherited from the previous iteration so in that sense leaving outside is a sane thing to do.

Show 6 quoted lines
> Dan McGee (1):
>       git-mergetool: Allow gvimdiff to be used as a mergetool
>
> Theodore Ts'o (2):
>       git-mergetool: Make default selection of merge-tool more intelligent
>       Add git-applymbox, git-applypatch, and *~ to .gitignore
But I hope you would not be offended if I said I do not want to.

This is not such a strong objection, but I really wish that you did not mix in the .gitignore change; it does not belong to this "series".

If it were an obvious and universally nondisagreeable fix, I would not mind you mixing it in this mergetool updates, but I am of two minds about the .gitignore change, and actually slightly in favor of not adding git-applymbox and git-applypatch back.

About git-applymbox and git-applypatch, it helps people when they switch branches and/or bisect to keep potential build products from older/different revisions listed in .gitignore. That would however imply we would end up carrying old entries forever in it. We do not keep clean rule in Makefile to remove build products from older/different revisions when remove build targets, so why should we keep them in .gitignore?

Also I deliberately have kept *~ out of .gitignore for a reason. I do have that entry in .git/info/exclude, but the choice of Emacs over vi is personal to me.

Theodore Tso· Jun 10, 2007, 21:31 UTC · re: Junio C Hamano · lore

Re: Please pull mergetool.git

On Sun, Jun 10, 2007 at 12:55:13PM -0700, Junio C Hamano wrote:
Show 5 quoted lines
> But I hope you would not be offended if I said I do not want to.
> 
> This is not such a strong objection, but I really wish that you
> did not mix in the .gitignore change; it does not belong to this
> "series".

OK, no problem. It was something I noticed while I was preparing the series, and I thought it was non-controverisal enough to just throw it in. It's at the tail of the series, so it's easy enough for me to do a

	git reset --hard HEAD^
on the mergetool branch....   OK, done.  If you pull from 
        git://repo.or.cz/git/mergetool.git mergetool

You'll only get the first two changes to git-mergetool, and the .gitignore change has been dropped.

Regards,
					- Ted
Brian Gernhardt· Jun 10, 2007, 23:29 UTC · re: Junio C Hamano · lore

Re: Please pull mergetool.git

On Jun 10, 2007, at 3:55 PM, Junio C Hamano wrote:
Show 5 quoted lines
> I do not see problems in the mergetool part, other than that I
> mildly suspect that opendiff -- actually FileMerge -- might want
> to be in the test -n "$DISPLAY" section, but that is inherited
> from the previous iteration so in that sense leaving outside is
> a sane thing to do.

Actually, opendiff (as FileMerge.app) is available under OS X, where $DISPLAY is not set.

~~ Brian
Junio C Hamano· Jun 11, 2007, 01:02 UTC · re: Brian Gernhardt · lore

Re: Please pull mergetool.git

Brian Gernhardt <benji@silverinsanity.com> writes:
Show 10 quoted lines
> On Jun 10, 2007, at 3:55 PM, Junio C Hamano wrote:
>
>> I do not see problems in the mergetool part, other than that I
>> mildly suspect that opendiff -- actually FileMerge -- might want
>> to be in the test -n "$DISPLAY" section, but that is inherited
>> from the previous iteration so in that sense leaving outside is
>> a sane thing to do.
>
> Actually, opendiff (as FileMerge.app) is available under OS X, where
> $DISPLAY is not set.
Heh, a graphical environment that does not use DISPLAY...
Thanks.
Martin Langhoff· Jun 11, 2007, 01:55 UTC · re: Junio C Hamano · lore

Re: Please pull mergetool.git

On 6/11/07, Junio C Hamano <gitster@pobox.com> wrote:
Show 5 quoted lines
> Brian Gernhardt <benji@silverinsanity.com> writes:
> > Actually, opendiff (as FileMerge.app) is available under OS X, where
> > $DISPLAY is not set.
>
> Heh, a graphical environment that does not use DISPLAY...

FWIW, it depends on whether you are using xterm (DISPLAY is set) or Terminal.app (DISPLAY may not be set). In any case FileMerge.app will ignore DISPLAY.

cheers,
m
Theodore Tso· Jun 11, 2007, 13:31 UTC · re: Martin Langhoff · lore

Re: Please pull mergetool.git

On Mon, Jun 11, 2007 at 01:55:25PM +1200, Martin Langhoff wrote:
> FWIW, it depends on whether you are using xterm (DISPLAY is set) or
> Terminal.app (DISPLAY may not be set). In any case FileMerge.app will
> ignore DISPLAY.

Is there a reliable way on MacOS to determine whether the user is sitting in front of the terminal, as opposed to logged into the MacOS box remotely? Ideally we would only try opendiff/FileMerge.app if we new that the user is indeed sitting in front of the graphical display.

						- Ted

← back to recent threads