Re: [PATCH] Add git-mergetool to run an appropriate merge conflict resolution program

From: Junio C Hamano <junkio@cox.net>
Date: Mar 6, 2007, 05:43 UTC
Message-ID: <7vr6s3sz8r.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <E1HORtY-0000zK-8B@candygram.thunk.org>

"Theodore Ts'o" <tytso@mit.edu> writes:

Show 7 quoted lines

> +git-mergetool(1)
> +================
> +
> +NAME
> +----
> +git-mergetool - Forward-port local commits to the updated upstream head
> +

Hmph. We already have a tool to achieve such a goal, and that is called git-rebase. Why would we want your program? ;-)

Show 14 quoted lines

> diff --git a/git-mergetool.sh b/git-mergetool.sh
> new file mode 100755
> index 0000000..b961719
> --- /dev/null
> +++ b/git-mergetool.sh
> @@ -0,0 +1,208 @@
> +#!/bin/sh
> +#
> +# This program resolves merge conflicts in git
> +#
> +# Copyright (c) 2006 Theodore Y. Ts'o
> +#
> +# This file is licensed under the GPL v2, or a later version
> +# at the discretion of Linus Torvalds.

Heh ;-).

Show 6 quoted lines

> +#
> +
> +usage () {
> +    echo "Usage: git mergetool [--tool=tool] [file to merge] ..."
> +    exit 1
> +}

Do we want to do this by hand ourselves, or dot-source sh-setup like others? You would also get die() for free.

Show 13 quoted lines

> +merge_file () {
> ...
> +
> +	if test ! -f "$path" ; then
> +		echo "$path: file not found"
> +		exit 1
> +	fi
> +
> +	f=`git-ls-files -u "$path"`
> +	if test -z "$f" ; then
> +		echo "$path: file does not need merging"
> +		exit 1
> +	fi

You should be able to set IFS to exclude SP and then you only have to say you do not support LF and HT, both of which are much less likely than SP to be in the pathname.

> +	mv "$path" "$BACKUP"
> +	cp "$BACKUP" "$path"

What if $path is a symlink blob?  ;-)

> +	git cat-file blob ":1:$path" > "$BASE"
> +	git cat-file blob ":2:$path" > "$LOCAL"
> +	git cat-file blob ":3:$path" > "$REMOTE"

Show 9 quoted lines

> +	case "$merge_tool" in
> +	    kdiff3)
> ...
> +	    tkdiff)
> ...
> +	    meld)
> ...
> +	    xxdiff)
> ...

It is depressing to see that the differences between the command lines of these have to be much larger than just the command name and order of three (or four if we count the result) paths parameters. I was hoping that we could do something like:

	mergetool -t='newmerge $BASE $LOCAL $REMOTE'

> +		xxdiff -X --show-merged-pane \
> +		    -R 'Accel.SaveAsMerged: "Ctrl-S"' \
> +		    -R 'Accel.Search: "Ctrl+F"' \
> +		    -R 'Accel.SearchForward: "Ctrl-G"' \

Do these configuration belong to individual scripts like this?

Show 13 quoted lines

> +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 emacs >/dev/null 2>&1; then
> +        merge_tool=emerge
> +    else
> +	echo "No available merge tools available."

Curious choice of words...

> +if test $# -eq 0 ; then
> +	files=`git ls-files -u --abbrev=8 | colrm 1 24 | sort -u`

Careful. I think --abbrev=8 just means use at least 8 but more as needed to make them unique. sed -e 's/^[^ ]* //' (whitespace are HTs) would be safer and simpler, as you are not dealing with a pathname that has LF in it anyway.