Re: [PATCH v4 14/14] difftool/mergetool: refactor commands to use git-mergetool--lib

From: Markus Heidelberg <markus.heidelberg@web.de>
Date: Apr 8, 2009, 05:33 UTC
Message-ID: <200904080733.01030.markus.heidelberg@web.de>
In-Reply-To: <1239145213-76701-1-git-send-email-davvid@gmail.com>

David Aguilar, 08.04.2009:

> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh

> +guess_merge_tool () {
> +	tools="ecmerge"
> +	if merge_mode; then
> +		tools="$tools tortoisemerge"

ecmerge can now go to the block after "test -n $DISPLAY", so that in this if-then-else really only merge-only and diff-only tools are handled. Then here it is + tools="tortoisemerge"

> +	else
> +		kompare="kompare"

and here + tools="kompare"

Show 9 quoted lines

> +	fi
> +	if test -n "$DISPLAY"; then
> +		if test -n "$GNOME_DESKTOP_SESSION_ID" ; then
> +			tools="meld opendiff kdiff3 $kompare tkdiff $tools"
> +			tools="$tools xxdiff gvimdiff diffuse"
> +		else
> +			tools="opendiff kdiff3 $kompare tkdiff xxdiff $tools"
> +			tools="$tools meld gvimdiff diffuse"
> +		fi

And above ecmerge And now that we remove the duplicated spaces afterwards anyway, we can get rid of the double tools= and continue the line with \ if we adjust the sed command below...

Show 11 quoted lines

> +	fi
> +	if echo "${VISUAL:-$EDITOR}" | grep emacs > /dev/null 2>&1; then
> +		# $EDITOR is emacs so add emerge as a candidate
> +		tools="$tools emerge vimdiff"
> +	elif echo "${VISUAL:-$EDITOR}" | grep vim > /dev/null 2>&1; then
> +		# $EDITOR is vim so add vimdiff as a candidate
> +		tools="$tools vimdiff emerge"
> +	else
> +		tools="$tools emerge vimdiff"
> +	fi
> +	tools="$(echo "$tools" | sed -e 's/ +/ /g')"

Doesn't work for me. For me 's/ \+/ /g' works.

...like this: 's/[ 	]\+/ /g' (space and tab)

OK, for clarification now:
	if merge_mode; then
		tools="tortoisemerge"
	else
		tools="kompare"
	fi
	if test -n "$DISPLAY"; then
		if test -n "$GNOME_DESKTOP_SESSION_ID" ; then
			tools="meld opendiff kdiff3 tkdiff xxdiff $tools \
				gvimdiff diffuse ecmerge"
		else
			tools="opendiff kdiff3 tkdiff xxdiff meld $tools \
				gvimdiff diffuse ecmerge"
		fi
	fi
	[...]
	tools="$(echo "$tools" | sed -e 's/[ 	]\+/ /g')"

Show 18 quoted lines

> +	echo >&2 "merge tool candidates: $tools"
> +
> +	# Loop over each candidate and stop when a valid merge tool is found.
> +	for i in $tools
> +	do
> +		merge_tool_path="$(translate_merge_tool_path "$i")"
> +		if type "$merge_tool_path" > /dev/null 2>&1; then
> +			merge_tool="$i"
> +			break
> +		fi
> +	done
> +
> +	if test -z "$merge_tool" ; then
> +		echo >&2 "No known merge resolution program available."
> +		return 1
> +	fi
> +	echo "$merge_tool"
> +}

Looks good to me, after these last 2 issues are adjusted. Maybe resend the whole series then, so that Junio can apply them easily?

Markus