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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



"Theodore Ts'o" <tytso@xxxxxxx> writes:

> +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? ;-)

> 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 ;-).

> +#
> +
> +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.

> +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"

> +	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?

> +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.

-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]