git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH v4 14/14] difftool/mergetool: refactor commands to use git-mergetool--lib
@ 2009-04-07 23:00 David Aguilar
  2009-04-08  5:33 ` Markus Heidelberg
  0 siblings, 1 reply; 6+ messages in thread
From: David Aguilar @ 2009-04-07 23:00 UTC (permalink / raw)
  To: gitster; +Cc: git, markus.heidelberg, charles, David Aguilar

This consolidates the common functionality from git-mergetool and
git-difftool--helper into a single git-mergetool--lib scriptlet.

Signed-off-by: David Aguilar <davvid@gmail.com>
---

Includes latest suggestions from Markus:

* Modify check_unchanged so that it doesn't check merge_mode

* Modify the way $tools is setup so that we fixup duplicated
spaces later

* Fix the status capturing for the custom tool in run_merge_tool

 .gitignore                           |    1 +
 Documentation/git-mergetool--lib.txt |   56 +++++
 Makefile                             |    1 +
 git-difftool--helper.sh              |  186 +----------------
 git-mergetool--lib.sh                |  381 ++++++++++++++++++++++++++++++++++
 git-mergetool.sh                     |  224 ++-------------------
 6 files changed, 461 insertions(+), 388 deletions(-)
 create mode 100644 Documentation/git-mergetool--lib.txt
 create mode 100644 git-mergetool--lib.sh

diff --git a/.gitignore b/.gitignore
index a36da9d..757c7f0 100644
--- a/.gitignore
+++ b/.gitignore
@@ -80,6 +80,7 @@ git-merge-recursive
 git-merge-resolve
 git-merge-subtree
 git-mergetool
+git-mergetool--lib
 git-mktag
 git-mktree
 git-name-rev
diff --git a/Documentation/git-mergetool--lib.txt b/Documentation/git-mergetool--lib.txt
new file mode 100644
index 0000000..3d57031
--- /dev/null
+++ b/Documentation/git-mergetool--lib.txt
@@ -0,0 +1,56 @@
+git-mergetool--lib(1)
+=====================
+
+NAME
+----
+git-mergetool--lib - Common git merge tool shell scriptlets
+
+SYNOPSIS
+--------
+'. "$(git --exec-path)/git-mergetool--lib"'
+
+DESCRIPTION
+-----------
+
+This is not a command the end user would want to run.  Ever.
+This documentation is meant for people who are studying the
+Porcelain-ish scripts and/or are writing new ones.
+
+The 'git-mergetool--lib' scriptlet is designed to be sourced (using
+`.`) by other shell scripts to set up functions for working
+with git merge tools.
+
+Before sourcing it, your script should set up a few variables;
+`TOOL_MODE` is used to define the operation mode for various
+functions.  'diff' and 'merge' are valid values.
+
+FUNCTIONS
+---------
+get_merge_tool::
+	returns a merge tool
+
+get_merge_tool_cmd::
+	returns the custom command for a merge tool.
+
+get_merge_tool_path::
+	returns the custom path for a merge tool.
+
+run_merge_tool::
+	launches a merge tool given the tool name and a true/false
+	flag to indicate whether a merge base is present.
+	'$merge_tool', '$merge_tool_path', and for custom commands,
+	'$merge_tool_cmd', must be defined prior to calling
+	run_merge_tool.  Additionally, '$MERGED', '$LOCAL', '$REMOTE',
+	and '$BASE' must be defined for use by the merge tool.
+
+Author
+------
+Written by David Aguilar <davvid@gmail.com>
+
+Documentation
+--------------
+Documentation by David Aguilar and the git-list <git@vger.kernel.org>.
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/Makefile b/Makefile
index a80055f..3e56274 100644
--- a/Makefile
+++ b/Makefile
@@ -284,6 +284,7 @@ SCRIPT_SH += git-merge-octopus.sh
 SCRIPT_SH += git-merge-one-file.sh
 SCRIPT_SH += git-merge-resolve.sh
 SCRIPT_SH += git-mergetool.sh
+SCRIPT_SH += git-mergetool--lib.sh
 SCRIPT_SH += git-parse-remote.sh
 SCRIPT_SH += git-pull.sh
 SCRIPT_SH += git-quiltimport.sh
diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh
index f3c27d8..b450036 100755
--- a/git-difftool--helper.sh
+++ b/git-difftool--helper.sh
@@ -5,6 +5,10 @@
 #
 # Copyright (c) 2009 David Aguilar
 
+# Load common functions from git-mergetool--lib
+TOOL_MODE=diff
+. git-mergetool--lib
+
 # difftool.prompt controls the default prompt/no-prompt behavior
 # and is overridden with $GIT_DIFFTOOL*_PROMPT.
 should_prompt () {
@@ -16,8 +20,7 @@ should_prompt () {
 	fi
 }
 
-# This function prepares temporary files and launches the appropriate
-# merge tool.
+# Sets up shell variables and runs a merge tool
 launch_merge_tool () {
 	# Merged is the filename as it appears in the work tree
 	# Local is the contents of a/filename
@@ -37,187 +40,16 @@ launch_merge_tool () {
 	fi
 
 	# Run the appropriate merge tool command
-	case "$merge_tool" in
-	kdiff3)
-		basename=$(basename "$MERGED")
-		"$merge_tool_path" --auto \
-			--L1 "$basename (A)" \
-			--L2 "$basename (B)" \
-			"$LOCAL" "$REMOTE" \
-			> /dev/null 2>&1
-		;;
-
-	kompare)
-		"$merge_tool_path" "$LOCAL" "$REMOTE"
-		;;
-
-	tkdiff)
-		"$merge_tool_path" "$LOCAL" "$REMOTE"
-		;;
-
-	meld)
-		"$merge_tool_path" "$LOCAL" "$REMOTE"
-		;;
-
-	diffuse)
-		"$merge_tool_path" "$LOCAL" "$REMOTE" | cat
-		;;
-
-	vimdiff)
-		"$merge_tool_path" -d -c "wincmd l" "$LOCAL" "$REMOTE"
-		;;
-
-	gvimdiff)
-		"$merge_tool_path" -d -c "wincmd l" -f "$LOCAL" "$REMOTE"
-		;;
-
-	xxdiff)
-		"$merge_tool_path" \
-			-R 'Accel.Search: "Ctrl+F"' \
-			-R 'Accel.SearchForward: "Ctrl-G"' \
-			"$LOCAL" "$REMOTE"
-		;;
-
-	opendiff)
-		"$merge_tool_path" "$LOCAL" "$REMOTE" | cat
-		;;
-
-	ecmerge)
-		"$merge_tool_path" "$LOCAL" "$REMOTE" \
-			--default --mode=merge2 --to="$MERGED"
-		;;
-
-	emerge)
-		"$merge_tool_path" -f emerge-files-command \
-			"$LOCAL" "$REMOTE" "$(basename "$MERGED")"
-		;;
-
-	*)
-		if test -n "$merge_tool_cmd"; then
-			( eval $merge_tool_cmd )
-		fi
-		;;
-	esac
-}
-
-# Verifies that (difftool|mergetool).<tool>.cmd exists
-valid_custom_tool() {
-	merge_tool_cmd="$(git config difftool.$1.cmd)"
-	test -z "$merge_tool_cmd" &&
-	merge_tool_cmd="$(git config mergetool.$1.cmd)"
-	test -n "$merge_tool_cmd"
-}
-
-# Verifies that the chosen merge tool is properly setup.
-# Built-in merge tools are always valid.
-valid_tool() {
-	case "$1" in
-	kdiff3 | kompare | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | gvimdiff | ecmerge)
-		;; # happy
-	*)
-		if ! valid_custom_tool "$1"
-		then
-			return 1
-		fi
-		;;
-	esac
-}
-
-# Sets up the merge_tool_path variable.
-# This handles the difftool.<tool>.path configuration.
-# This also falls back to mergetool defaults.
-init_merge_tool_path() {
-	merge_tool_path=$(git config difftool."$1".path)
-	test -z "$merge_tool_path" &&
-	merge_tool_path=$(git config mergetool."$1".path)
-	if test -z "$merge_tool_path"; then
-		case "$1" in
-		vimdiff)
-			merge_tool_path=vim
-			;;
-		gvimdiff)
-			merge_tool_path=gvim
-			;;
-		emerge)
-			merge_tool_path=emacs
-			;;
-		*)
-			merge_tool_path="$1"
-			;;
-		esac
-	fi
+	run_merge_tool "$merge_tool"
 }
 
 # Allow GIT_DIFF_TOOL and GIT_MERGE_TOOL to provide default values
 test -n "$GIT_MERGE_TOOL" && merge_tool="$GIT_MERGE_TOOL"
 test -n "$GIT_DIFF_TOOL" && merge_tool="$GIT_DIFF_TOOL"
 
-# If merge tool was not specified then use the diff.tool
-# configuration variable.  If that's invalid then reset merge_tool.
-# Fallback to merge.tool.
-if test -z "$merge_tool"; then
-	merge_tool=$(git config diff.tool)
-	test -z "$merge_tool" &&
-	merge_tool=$(git config merge.tool)
-	if test -n "$merge_tool" && ! valid_tool "$merge_tool"; then
-		echo >&2 "git config option diff.tool set to unknown tool: $merge_tool"
-		echo >&2 "Resetting to default..."
-		unset merge_tool
-	fi
-fi
-
-# Try to guess an appropriate merge tool if no tool has been set.
-if test -z "$merge_tool"; then
-	# We have a $DISPLAY so try some common UNIX merge tools
-	if test -n "$DISPLAY"; then
-		# If gnome then prefer meld, otherwise, prefer kdiff3 or kompare
-		if test -n "$GNOME_DESKTOP_SESSION_ID" ; then
-			merge_tool_candidates="meld kdiff3 kompare tkdiff xxdiff gvimdiff diffuse"
-		else
-			merge_tool_candidates="kdiff3 kompare tkdiff xxdiff meld gvimdiff diffuse"
-		fi
-	fi
-	if echo "${VISUAL:-$EDITOR}" | grep 'emacs' > /dev/null 2>&1; then
-		# $EDITOR is emacs so add emerge as a candidate
-		merge_tool_candidates="$merge_tool_candidates emerge opendiff vimdiff"
-	elif echo "${VISUAL:-$EDITOR}" | grep 'vim' > /dev/null 2>&1; then
-		# $EDITOR is vim so add vimdiff as a candidate
-		merge_tool_candidates="$merge_tool_candidates vimdiff opendiff emerge"
-	else
-		merge_tool_candidates="$merge_tool_candidates opendiff emerge vimdiff"
-	fi
-	echo "merge tool candidates: $merge_tool_candidates"
-
-	# Loop over each candidate and stop when a valid merge tool is found.
-	for i in $merge_tool_candidates
-	do
-		init_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 "No known merge resolution program available."
-		exit 1
-	fi
-
-else
-	# A merge tool has been set, so verify that it's valid.
-	if ! valid_tool "$merge_tool"; then
-		echo >&2 "Unknown merge tool $merge_tool"
-		exit 1
-	fi
-
-	init_merge_tool_path "$merge_tool"
-
-	if test -z "$merge_tool_cmd" && ! type "$merge_tool_path" > /dev/null 2>&1; then
-		echo "The merge tool $merge_tool is not available as '$merge_tool_path'"
-		exit 1
-	fi
-fi
-
+merge_tool=$(get_merge_tool "$merge_tool") || exit
+merge_tool_cmd="$(get_merge_tool_cmd "$merge_tool")"
+merge_tool_path="$(get_merge_tool_path "$merge_tool")" || exit
 
 # Launch the merge tool on each path provided by 'git diff'
 while test $# -gt 6
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
new file mode 100644
index 0000000..95cc355
--- /dev/null
+++ b/git-mergetool--lib.sh
@@ -0,0 +1,381 @@
+# git-mergetool--lib is a library for common merge tool functions
+diff_mode() {
+	test "$TOOL_MODE" = diff
+}
+
+merge_mode() {
+	test "$TOOL_MODE" = merge
+}
+
+translate_merge_tool_path () {
+	if test -n "$2"; then
+		echo "$2"
+	else
+		case "$1" in
+		vimdiff)
+			path=vim
+			;;
+		gvimdiff)
+			path=gvim
+			;;
+		emerge)
+			path=emacs
+			;;
+		*)
+			path="$1"
+			;;
+		esac
+		echo "$path"
+	fi
+}
+
+check_unchanged () {
+	if test "$MERGED" -nt "$BACKUP"; then
+		status=0
+	else
+		while true; do
+			echo "$MERGED seems unchanged."
+			printf "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
+}
+
+valid_tool () {
+	case "$1" in
+	kdiff3 | tkdiff | xxdiff | meld | opendiff | \
+	emerge | vimdiff | gvimdiff | ecmerge | diffuse)
+		;; # happy
+	tortoisemerge)
+		if ! merge_mode; then
+			return 1
+		fi
+		;;
+	kompare)
+		if ! diff_mode; then
+			return 1
+		fi
+		;;
+	*)
+		if test -z "$(get_merge_tool_cmd "$1")"; then
+			return 1
+		fi
+		;;
+	esac
+}
+
+get_merge_tool_cmd () {
+	diff_mode &&
+	custom_cmd="$(git config difftool.$1.cmd)"
+	test -z "$custom_cmd" &&
+	custom_cmd="$(git config mergetool.$1.cmd)"
+	test -n "$custom_cmd" &&
+	echo "$custom_cmd"
+}
+
+run_merge_tool () {
+	base_present="$2"
+	status=0
+
+	case "$1" in
+	kdiff3)
+		if merge_mode; then
+			if $base_present; then
+				("$merge_tool_path" --auto \
+					--L1 "$MERGED (Base)" \
+					--L2 "$MERGED (Local)" \
+					--L3 "$MERGED (Remote)" \
+					-o "$MERGED" \
+					"$BASE" "$LOCAL" "$REMOTE" \
+				> /dev/null 2>&1)
+			else
+				("$merge_tool_path" --auto \
+					--L1 "$MERGED (Local)" \
+					--L2 "$MERGED (Remote)" \
+					-o "$MERGED" \
+					"$LOCAL" "$REMOTE" \
+				> /dev/null 2>&1)
+			fi
+			status=$?
+		else
+			("$merge_tool_path" --auto \
+			 --L1 "$MERGED (A)" \
+			 --L2 "$MERGED (B)" "$LOCAL" "$REMOTE" \
+			 > /dev/null 2>&1)
+		fi
+		;;
+	kompare)
+		"$merge_tool_path" "$LOCAL" "$REMOTE"
+		;;
+	tkdiff)
+		if merge_mode; then
+			if $base_present; then
+				"$merge_tool_path" -a "$BASE" \
+					-o "$MERGED" "$LOCAL" "$REMOTE"
+			else
+				"$merge_tool_path" \
+					-o "$MERGED" "$LOCAL" "$REMOTE"
+			fi
+			status=$?
+		else
+			"$merge_tool_path" "$LOCAL" "$REMOTE"
+		fi
+		;;
+	meld)
+		if merge_mode; then
+			touch "$BACKUP"
+			"$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"
+			check_unchanged
+		else
+			"$merge_tool_path" "$LOCAL" "$REMOTE"
+		fi
+		;;
+	diffuse)
+		if merge_mode; then
+			touch "$BACKUP"
+			if $base_present; then
+				"$merge_tool_path" \
+					"$LOCAL" "$MERGED" "$REMOTE" \
+					"$BASE" | cat
+			else
+				"$merge_tool_path" \
+					"$LOCAL" "$MERGED" "$REMOTE" | cat
+			fi
+			check_unchanged
+		else
+			"$merge_tool_path" "$LOCAL" "$REMOTE" | cat
+		fi
+		;;
+	vimdiff)
+		if merge_mode; then
+			touch "$BACKUP"
+			"$merge_tool_path" -d -c "wincmd l" \
+				"$LOCAL" "$MERGED" "$REMOTE"
+			check_unchanged
+		else
+			"$merge_tool_path" -d -c "wincmd l" \
+				"$LOCAL" "$REMOTE"
+		fi
+		;;
+	gvimdiff)
+		if merge_mode; then
+			touch "$BACKUP"
+			"$merge_tool_path" -d -c "wincmd l" -f \
+				"$LOCAL" "$MERGED" "$REMOTE"
+			check_unchanged
+		else
+			"$merge_tool_path" -d -c "wincmd l" -f \
+				"$LOCAL" "$REMOTE"
+		fi
+		;;
+	xxdiff)
+		if merge_mode; then
+			touch "$BACKUP"
+			if $base_present; then
+				"$merge_tool_path" -X --show-merged-pane \
+					-R 'Accel.SaveAsMerged: "Ctrl-S"' \
+					-R 'Accel.Search: "Ctrl+F"' \
+					-R 'Accel.SearchForward: "Ctrl-G"' \
+					--merged-file "$MERGED" \
+					"$LOCAL" "$BASE" "$REMOTE"
+			else
+				"$merge_tool_path" -X $extra \
+					-R 'Accel.SaveAsMerged: "Ctrl-S"' \
+					-R 'Accel.Search: "Ctrl+F"' \
+					-R 'Accel.SearchForward: "Ctrl-G"' \
+					--merged-file "$MERGED" \
+					"$LOCAL" "$REMOTE"
+			fi
+			check_unchanged
+		else
+			"$merge_tool_path" \
+				-R 'Accel.Search: "Ctrl+F"' \
+				-R 'Accel.SearchForward: "Ctrl-G"' \
+				"$LOCAL" "$REMOTE"
+		fi
+		;;
+	opendiff)
+		if merge_mode; then
+			touch "$BACKUP"
+			if $base_present; then
+				"$merge_tool_path" "$LOCAL" "$REMOTE" \
+					-ancestor "$BASE" \
+					-merge "$MERGED" | cat
+			else
+				"$merge_tool_path" "$LOCAL" "$REMOTE" \
+					-merge "$MERGED" | cat
+			fi
+			check_unchanged
+		else
+			"$merge_tool_path" "$LOCAL" "$REMOTE" | cat
+		fi
+		;;
+	ecmerge)
+		if merge_mode; then
+			touch "$BACKUP"
+			if $base_present; then
+				"$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" \
+					--default --mode=merge3 --to="$MERGED"
+			else
+				"$merge_tool_path" "$LOCAL" "$REMOTE" \
+					--default --mode=merge2 --to="$MERGED"
+			fi
+			check_unchanged
+		else
+			"$merge_tool_path" "$LOCAL" "$REMOTE" \
+				--default --mode=merge2 --to="$MERGED"
+		fi
+		;;
+	emerge)
+		if merge_mode; then
+			if $base_present; then
+				"$merge_tool_path" \
+					-f emerge-files-with-ancestor-command \
+					"$LOCAL" "$REMOTE" "$BASE" \
+					"$(basename "$MERGED")"
+			else
+				"$merge_tool_path" \
+					-f emerge-files-command \
+					"$LOCAL" "$REMOTE" \
+					"$(basename "$MERGED")"
+			fi
+			status=$?
+		else
+			"$merge_tool_path" -f emerge-files-command \
+				"$LOCAL" "$REMOTE" "$(basename "$MERGED")"
+		fi
+		;;
+	tortoisemerge)
+		if $base_present; then
+			touch "$BACKUP"
+			"$merge_tool_path" \
+				-base:"$BASE" -mine:"$LOCAL" \
+				-theirs:"$REMOTE" -merged:"$MERGED"
+			check_unchanged
+		else
+			echo "TortoiseMerge cannot be used without a base" 1>&2
+			status=1
+		fi
+		;;
+	*)
+		if test -z "$merge_tool_cmd"; then
+			if merge_mode; then
+				status=1
+			fi
+			break
+		fi
+		if merge_mode; then
+			if test "$merge_tool_trust_exit_code" = "false"; then
+				touch "$BACKUP"
+				( eval $merge_tool_cmd )
+				check_unchanged
+			else
+				( eval $merge_tool_cmd )
+				status=$?
+			fi
+		else
+			( eval $merge_tool_cmd )
+		fi
+		;;
+	esac
+	return $status
+}
+
+guess_merge_tool () {
+	tools="ecmerge"
+	if merge_mode; then
+		tools="$tools tortoisemerge"
+	else
+		kompare="kompare"
+	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
+	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')"
+	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"
+}
+
+get_configured_merge_tool () {
+	# Diff mode first tries diff.tool and falls back to merge.tool.
+	# Merge mode only checks merge.tool
+	if diff_mode; then
+		tool=$(git config diff.tool)
+	fi
+	if test -z "$tool"; then
+		tool=$(git config merge.tool)
+	fi
+	if test -n "$merge_tool" && ! valid_tool "$merge_tool"; then
+		echo >&2 "git config option $TOOL_MODE.tool set to unknown tool: $merge_tool"
+		echo >&2 "Resetting to default..."
+		return 1
+	fi
+	echo "$tool"
+}
+
+get_merge_tool_path () {
+	# A merge tool has been set, so verify that it's valid.
+	if ! valid_tool "$merge_tool"; then
+		echo >&2 "Unknown merge tool $merge_tool"
+		exit 1
+	fi
+	if diff_mode; then
+		merge_tool_path=$(git config difftool."$merge_tool".path)
+	fi
+	if test -z "$merge_tool_path"; then
+		merge_tool_path=$(git config mergetool."$merge_tool".path)
+	fi
+	merge_tool_path="$(translate_merge_tool_path "$merge_tool" "$merge_tool_path")"
+	if test -z "$merge_tool_cmd" && ! type "$merge_tool_path" > /dev/null 2>&1; then
+		echo >&2 "The $TOOL_MODE tool $merge_tool is not available as '$merge_tool_path'"
+		exit 1
+	fi
+	echo "$merge_tool_path"
+}
+
+get_merge_tool () {
+	merge_tool="$1"
+	# Check if a merge tool has been configured
+	if test -z "$merge_tool"; then
+		merge_tool=$(get_configured_merge_tool)
+	fi
+	# Try to guess an appropriate merge tool if no tool has been set.
+	if test -z "$merge_tool"; then
+		merge_tool=$(guess_merge_tool) || exit
+	fi
+	echo "$merge_tool"
+}
diff --git a/git-mergetool.sh b/git-mergetool.sh
index cceebb7..efa31a2 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -11,7 +11,9 @@
 USAGE='[--tool=tool] [-y|--no-prompt|--prompt] [file to merge] ...'
 SUBDIRECTORY_OK=Yes
 OPTIONS_SPEC=
+TOOL_MODE=merge
 . git-sh-setup
+. git-mergetool--lib
 require_work_tree
 
 # Returns true if the mode reflects a symlink
@@ -110,22 +112,6 @@ resolve_deleted_merge () {
 	done
 }
 
-check_unchanged () {
-    if test "$MERGED" -nt "$BACKUP" ; then
-	status=0;
-    else
-	while true; do
-	    echo "$MERGED seems unchanged."
-	    printf "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
-}
-
 checkout_staged_file () {
     tmpfile=$(expr "$(git checkout-index --temp --stage="$1" "$2")" : '\([^	]*\)	')
 
@@ -188,107 +174,11 @@ merge_file () {
 	read ans
     fi
 
-    case "$merge_tool" in
-	kdiff3)
-	    if base_present ; then
-		("$merge_tool_path" --auto --L1 "$MERGED (Base)" --L2 "$MERGED (Local)" --L3 "$MERGED (Remote)" \
-		    -o "$MERGED" "$BASE" "$LOCAL" "$REMOTE" > /dev/null 2>&1)
-	    else
-		("$merge_tool_path" --auto --L1 "$MERGED (Local)" --L2 "$MERGED (Remote)" \
-		    -o "$MERGED" "$LOCAL" "$REMOTE" > /dev/null 2>&1)
-	    fi
-	    status=$?
-	    ;;
-	tkdiff)
-	    if base_present ; then
-		"$merge_tool_path" -a "$BASE" -o "$MERGED" "$LOCAL" "$REMOTE"
-	    else
-		"$merge_tool_path" -o "$MERGED" "$LOCAL" "$REMOTE"
-	    fi
-	    status=$?
-	    ;;
-	meld)
-	    touch "$BACKUP"
-	    "$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"
-	    check_unchanged
-	    ;;
-	vimdiff)
-	    touch "$BACKUP"
-	    "$merge_tool_path" -d -c "wincmd l" "$LOCAL" "$MERGED" "$REMOTE"
-	    check_unchanged
-	    ;;
-	gvimdiff)
-	    touch "$BACKUP"
-	    "$merge_tool_path" -d -c "wincmd l" -f "$LOCAL" "$MERGED" "$REMOTE"
-	    check_unchanged
-	    ;;
-	xxdiff)
-	    touch "$BACKUP"
-	    if base_present ; then
-		"$merge_tool_path" -X --show-merged-pane \
-		    -R 'Accel.SaveAsMerged: "Ctrl-S"' \
-		    -R 'Accel.Search: "Ctrl+F"' \
-		    -R 'Accel.SearchForward: "Ctrl-G"' \
-		    --merged-file "$MERGED" "$LOCAL" "$BASE" "$REMOTE"
-	    else
-		"$merge_tool_path" -X --show-merged-pane \
-		    -R 'Accel.SaveAsMerged: "Ctrl-S"' \
-		    -R 'Accel.Search: "Ctrl+F"' \
-		    -R 'Accel.SearchForward: "Ctrl-G"' \
-		    --merged-file "$MERGED" "$LOCAL" "$REMOTE"
-	    fi
-	    check_unchanged
-	    ;;
-	opendiff)
-	    touch "$BACKUP"
-	    if base_present; then
-		"$merge_tool_path" "$LOCAL" "$REMOTE" -ancestor "$BASE" -merge "$MERGED" | cat
-	    else
-		"$merge_tool_path" "$LOCAL" "$REMOTE" -merge "$MERGED" | cat
-	    fi
-	    check_unchanged
-	    ;;
-	ecmerge)
-	    touch "$BACKUP"
-	    if base_present; then
-		"$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" --default --mode=merge3 --to="$MERGED"
-	    else
-		"$merge_tool_path" "$LOCAL" "$REMOTE" --default --mode=merge2 --to="$MERGED"
-	    fi
-	    check_unchanged
-	    ;;
-	emerge)
-	    if base_present ; then
-		"$merge_tool_path" -f emerge-files-with-ancestor-command "$LOCAL" "$REMOTE" "$BASE" "$(basename "$MERGED")"
-	    else
-		"$merge_tool_path" -f emerge-files-command "$LOCAL" "$REMOTE" "$(basename "$MERGED")"
-	    fi
-	    status=$?
-	    ;;
-	tortoisemerge)
-	    if base_present ; then
-		touch "$BACKUP"
-		"$merge_tool_path" -base:"$BASE" -mine:"$LOCAL" -theirs:"$REMOTE" -merged:"$MERGED"
-		check_unchanged
-	    else
-		echo "TortoiseMerge cannot be used without a base" 1>&2
-		status=1
-	    fi
-	    ;;
-	*)
-	    if test -n "$merge_tool_cmd"; then
-		if test "$merge_tool_trust_exit_code" = "false"; then
-		    touch "$BACKUP"
-		    ( eval $merge_tool_cmd )
-		    check_unchanged
-		else
-		    ( eval $merge_tool_cmd )
-		    status=$?
-		fi
-	    fi
-	    ;;
-    esac
-    if test "$status" -ne 0; then
+    present=false
+    base_present &&
+    present=true
+
+    if ! run_merge_tool "$merge_tool" "$present"; then
 	echo "merge of $MERGED failed" 1>&2
 	mv -- "$BACKUP" "$MERGED"
 
@@ -347,44 +237,6 @@ do
     shift
 done
 
-valid_custom_tool()
-{
-    merge_tool_cmd="$(git config mergetool.$1.cmd)"
-    test -n "$merge_tool_cmd"
-}
-
-valid_tool() {
-	case "$1" in
-		kdiff3 | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | gvimdiff | ecmerge | tortoisemerge)
-			;; # happy
-		*)
-			if ! valid_custom_tool "$1"; then
-				return 1
-			fi
-			;;
-	esac
-}
-
-init_merge_tool_path() {
-	merge_tool_path=$(git config mergetool.$1.path)
-	if test -z "$merge_tool_path" ; then
-		case "$1" in
-			vimdiff)
-				merge_tool_path=vim
-				;;
-			gvimdiff)
-				merge_tool_path=gvim
-				;;
-			emerge)
-				merge_tool_path=emacs
-				;;
-			*)
-				merge_tool_path=$1
-				;;
-		esac
-	fi
-}
-
 prompt_after_failed_merge() {
     while true; do
 	printf "Continue merging other unresolved paths (y/n) ? "
@@ -402,62 +254,12 @@ prompt_after_failed_merge() {
     done
 }
 
-if test -z "$merge_tool"; then
-    merge_tool=$(git config merge.tool)
-    if test -n "$merge_tool" && ! valid_tool "$merge_tool"; then
-	    echo >&2 "git config option merge.tool set to unknown tool: $merge_tool"
-	    echo >&2 "Resetting to default..."
-	    unset merge_tool
-    fi
-fi
-
-if test -z "$merge_tool" ; then
-    if test -n "$DISPLAY"; then
-        if test -n "$GNOME_DESKTOP_SESSION_ID" ; then
-            merge_tool_candidates="meld kdiff3 tkdiff xxdiff tortoisemerge gvimdiff diffuse"
-        else
-            merge_tool_candidates="kdiff3 tkdiff xxdiff meld tortoisemerge gvimdiff diffuse"
-        fi
-    fi
-    if echo "${VISUAL:-$EDITOR}" | grep 'emacs' > /dev/null 2>&1; then
-        merge_tool_candidates="$merge_tool_candidates emerge opendiff vimdiff"
-    elif echo "${VISUAL:-$EDITOR}" | grep 'vim' > /dev/null 2>&1; then
-        merge_tool_candidates="$merge_tool_candidates vimdiff opendiff emerge"
-    else
-        merge_tool_candidates="$merge_tool_candidates opendiff emerge vimdiff"
-    fi
-    echo "merge tool candidates: $merge_tool_candidates"
-    for i in $merge_tool_candidates; do
-        init_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 "No known merge resolution program available."
-	exit 1
-    fi
-else
-    if ! valid_tool "$merge_tool"; then
-        echo >&2 "Unknown merge_tool $merge_tool"
-        exit 1
-    fi
-
-    init_merge_tool_path "$merge_tool"
-
-    merge_keep_backup="$(git config --bool merge.keepBackup || echo true)"
-    merge_keep_temporaries="$(git config --bool mergetool.keepTemporaries || echo false)"
-
-    if test -z "$merge_tool_cmd" && ! type "$merge_tool_path" > /dev/null 2>&1; then
-        echo "The merge tool $merge_tool is not available as '$merge_tool_path'"
-        exit 1
-    fi
-
-    if ! test -z "$merge_tool_cmd"; then
-        merge_tool_trust_exit_code="$(git config --bool mergetool.$merge_tool.trustExitCode || echo false)"
-    fi
-fi
+merge_tool=$(get_merge_tool "$merge_tool") || exit
+merge_tool_cmd="$(get_merge_tool_cmd "$merge_tool")"
+merge_tool_path="$(get_merge_tool_path "$merge_tool")" || exit
+merge_keep_backup="$(git config --bool merge.keepBackup || echo true)"
+merge_keep_temporaries="$(git config --bool mergetool.keepTemporaries || echo false)"
+merge_tool_trust_exit_code="$(git config --bool mergetool."$merge_tool".trustExitCode || echo false)"
 
 last_status=0
 rollup_status=0
-- 
1.6.1.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v4 14/14] difftool/mergetool: refactor commands to use git-mergetool--lib
  2009-04-07 23:00 [PATCH v4 14/14] difftool/mergetool: refactor commands to use git-mergetool--lib David Aguilar
@ 2009-04-08  5:33 ` Markus Heidelberg
  2009-04-08  6:09   ` Junio C Hamano
  0 siblings, 1 reply; 6+ messages in thread
From: Markus Heidelberg @ 2009-04-08  5:33 UTC (permalink / raw)
  To: David Aguilar; +Cc: gitster, git, charles

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"

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

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

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4 14/14] difftool/mergetool: refactor commands to use git-mergetool--lib
  2009-04-08  5:33 ` Markus Heidelberg
@ 2009-04-08  6:09   ` Junio C Hamano
  2009-04-08  6:35     ` David Aguilar
  2009-04-08  6:40     ` Charles Bailey
  0 siblings, 2 replies; 6+ messages in thread
From: Junio C Hamano @ 2009-04-08  6:09 UTC (permalink / raw)
  To: markus.heidelberg; +Cc: David Aguilar, gitster, git, charles

Markus Heidelberg <markus.heidelberg@web.de> writes:

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

Pleae don't.  "s/  */ /g' should be the most portable (the point being "do
not use one-or-more +").

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

Thanks.  I've replaced the series with the following applied on top of the
'master', but I won't be merging them to 'next' for tonight, I guess.

[PATCH 01/14] doc/merge-config: list ecmerge as a built-in merge
[PATCH 02/14] git-mergetool/difftool: make (g)vimdiff workable under
[PATCH 03/14] git-mergetool: add new merge tool TortoiseMerge
[PATCH 04/14] difftool: remove merge options for opendiff, tkdiff,
[PATCH 05/14] difftool: remove the backup file feature
[PATCH 06/14] difftool: use perl built-ins when testing for msys
[PATCH v2 07/14] difftool: add a -y shortcut for --no-prompt
[PATCH 08/14] difftool/mergetool: add diffuse as merge and diff tool
[PATCH v2 09/14] difftool: move 'git-difftool' out of contrib
[PATCH v3 10/14] difftool: add various git-difftool tests
[PATCH v2 11/14] difftool: add support for a difftool.prompt config variable
[PATCH 12/14] bash completion: add git-difftool
[PATCH 13/14] mergetool: use $( ... ) instead of `backticks`
[PATCH v4 14/14] difftool/mergetool: refactor commands to use git-mergetool--lib

It appears that, assuming that up to 13/14 above is what you two expected
me to pick up, we would perhaps need to only replace 14/14?

As the final sanity check, please eyeball the attached interdiff, created
this way:

 $ git checkout master^0
 $ git am -s ./+da-fourteen-patches
 $ A=$(git rev-parse HEAD)
 $ git reset --hard master
 $ git merge da/difftool ;# old series
 $ git diff --stat -p HEAD $A ;# what's new in the new series?

 Documentation/config.txt               |    3 +-
 Documentation/git-mergetool.txt        |    2 +-
 Documentation/merge-config.txt         |    6 +-
 command-list.txt                       |    1 -
 contrib/completion/git-completion.bash |    2 +-
 git-mergetool--lib.sh                  |  237 ++++++++++++++++++--------------
 t/t7800-difftool.sh                    |    4 +-
 7 files changed, 144 insertions(+), 111 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 6e29623..d427daf 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -670,7 +670,8 @@ diff.suppressBlankEmpty::
 diff.tool::
 	Controls which diff tool is used.  `diff.tool` overrides
 	`merge.tool` when used by linkgit:git-difftool[1] and has
-	the same valid values as `merge.tool`.
+	the same valid values as `merge.tool` minus "tortoisemerge"
+	and plus "kompare".
 
 difftool.<tool>.path::
 	Override the path for the given tool.  This is useful in case
diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index c3a8092..ff9700d 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -27,7 +27,7 @@ OPTIONS
 	Use the merge resolution program specified by <tool>.
 	Valid merge tools are:
 	kdiff3, tkdiff, meld, xxdiff, emerge, vimdiff, gvimdiff, ecmerge,
-	diffuse and opendiff
+	diffuse, tortoisemerge and opendiff
 +
 If a merge resolution program is not specified, 'git-mergetool'
 will use the configuration variable `merge.tool`.  If the
diff --git a/Documentation/merge-config.txt b/Documentation/merge-config.txt
index c4fc3eb..4832bc7 100644
--- a/Documentation/merge-config.txt
+++ b/Documentation/merge-config.txt
@@ -23,9 +23,9 @@ merge.tool::
 	Controls which merge resolution program is used by
 	linkgit:git-mergetool[1].  Valid built-in values are: "kdiff3",
 	"tkdiff", "meld", "xxdiff", "emerge", "vimdiff", "gvimdiff",
-	"diffuse" and "opendiff".  Any other value is treated as a custom
-	merge tool and there must be a corresponding mergetool.<tool>.cmd
-	option.
+	"diffuse", "ecmerge", "tortoisemerge", and
+	"opendiff".  Any other value is treated is custom merge tool
+	and there must be a corresponding mergetool.<tool>.cmd option.
 
 merge.verbosity::
 	Controls the amount of output shown by the recursive merge
diff --git a/command-list.txt b/command-list.txt
index fd66395..fb03a2e 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -69,7 +69,6 @@ git-merge-file                          plumbingmanipulators
 git-merge-index                         plumbingmanipulators
 git-merge-one-file                      purehelpers
 git-mergetool                           ancillarymanipulators
-git-mergetool--lib                      purehelpers
 git-merge-tree                          ancillaryinterrogators
 git-mktag                               plumbingmanipulators
 git-mktree                              plumbingmanipulators
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 6f96d75..069e19e 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1192,7 +1192,7 @@ _git_mergetool ()
 	local cur="${COMP_WORDS[COMP_CWORD]}"
 	case "$cur" in
 	--tool=*)
-		__gitcomp "$__git_mergetools_common" "" "${cur##--tool=}"
+		__gitcomp "$__git_mergetools_common tortoisemerge" "" "${cur##--tool=}"
 		return
 		;;
 	--*)
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 3d3edda..95cc355 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -1,10 +1,10 @@
 # git-mergetool--lib is a library for common merge tool functions
 diff_mode() {
-	test $TOOL_MODE = "diff"
+	test "$TOOL_MODE" = diff
 }
 
 merge_mode() {
-	test $TOOL_MODE = "merge"
+	test "$TOOL_MODE" = merge
 }
 
 translate_merge_tool_path () {
@@ -30,36 +30,36 @@ translate_merge_tool_path () {
 }
 
 check_unchanged () {
-	if merge_mode; then
-		if test "$MERGED" -nt "$BACKUP"; then
-			status=0
-		else
-			while true; do
-				echo "$MERGED seems unchanged."
-				printf "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
-	else
+	if test "$MERGED" -nt "$BACKUP"; then
 		status=0
+	else
+		while true; do
+			echo "$MERGED seems unchanged."
+			printf "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
-	return $status
 }
 
 valid_tool () {
 	case "$1" in
-	kdiff3 | kompare | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | gvimdiff | ecmerge | diffuse | tortoisemerge)
-		if test "$1" = "kompare" && ! diff_mode; then
+	kdiff3 | tkdiff | xxdiff | meld | opendiff | \
+	emerge | vimdiff | gvimdiff | ecmerge | diffuse)
+		;; # happy
+	tortoisemerge)
+		if ! merge_mode; then
 			return 1
 		fi
-		if test "$1" = "tortoisemerge" && ! merge_mode; then
+		;;
+	kompare)
+		if ! diff_mode; then
 			return 1
 		fi
-		;; # happy
+		;;
 	*)
 		if test -z "$(get_merge_tool_cmd "$1")"; then
 			return 1
@@ -79,160 +79,182 @@ get_merge_tool_cmd () {
 
 run_merge_tool () {
 	base_present="$2"
-	if diff_mode; then
-		base_present="false"
-	fi
-	if test -z "$base_present"; then
-		base_present="true"
-	fi
+	status=0
 
 	case "$1" in
 	kdiff3)
-		if $base_present; then
-			("$merge_tool_path" --auto \
-			 --L1 "$MERGED (Base)" --L2 "$MERGED (Local)" --L3 "$MERGED (Remote)" \
-			 -o "$MERGED" "$BASE" "$LOCAL" "$REMOTE" > /dev/null 2>&1)
-		else
-			if merge_mode; then
+		if merge_mode; then
+			if $base_present; then
 				("$merge_tool_path" --auto \
-				 --L1 "$MERGED (Local)" \
-				 --L2 "$MERGED (Remote)" \
-				 -o "$MERGED" "$LOCAL" "$REMOTE" \
+					--L1 "$MERGED (Base)" \
+					--L2 "$MERGED (Local)" \
+					--L3 "$MERGED (Remote)" \
+					-o "$MERGED" \
+					"$BASE" "$LOCAL" "$REMOTE" \
 				> /dev/null 2>&1)
 			else
 				("$merge_tool_path" --auto \
-				 --L1 "$MERGED (A)" \
-				 --L2 "$MERGED (B)" \
-				 "$LOCAL" "$REMOTE" \
+					--L1 "$MERGED (Local)" \
+					--L2 "$MERGED (Remote)" \
+					-o "$MERGED" \
+					"$LOCAL" "$REMOTE" \
 				> /dev/null 2>&1)
 			fi
+			status=$?
+		else
+			("$merge_tool_path" --auto \
+			 --L1 "$MERGED (A)" \
+			 --L2 "$MERGED (B)" "$LOCAL" "$REMOTE" \
+			 > /dev/null 2>&1)
 		fi
-		status=$?
 		;;
 	kompare)
 		"$merge_tool_path" "$LOCAL" "$REMOTE"
-		status=$?
 		;;
 	tkdiff)
-		if $base_present; then
-			"$merge_tool_path" -a "$BASE" -o "$MERGED" "$LOCAL" "$REMOTE"
-		else
-			if merge_mode; then
-				"$merge_tool_path" -o "$MERGED" "$LOCAL" "$REMOTE"
+		if merge_mode; then
+			if $base_present; then
+				"$merge_tool_path" -a "$BASE" \
+					-o "$MERGED" "$LOCAL" "$REMOTE"
 			else
-				"$merge_tool_path" "$LOCAL" "$REMOTE"
+				"$merge_tool_path" \
+					-o "$MERGED" "$LOCAL" "$REMOTE"
 			fi
+			status=$?
+		else
+			"$merge_tool_path" "$LOCAL" "$REMOTE"
 		fi
-		status=$?
 		;;
 	meld)
 		if merge_mode; then
 			touch "$BACKUP"
 			"$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"
+			check_unchanged
 		else
 			"$merge_tool_path" "$LOCAL" "$REMOTE"
 		fi
-		check_unchanged
 		;;
 	diffuse)
 		if merge_mode; then
 			touch "$BACKUP"
-		fi
-		if $base_present; then
-			"$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE" "$BASE" | cat
-		else
-			if merge_mode; then
-				"$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE" | cat
+			if $base_present; then
+				"$merge_tool_path" \
+					"$LOCAL" "$MERGED" "$REMOTE" \
+					"$BASE" | cat
 			else
-				"$merge_tool_path" "$LOCAL" "$REMOTE" | cat
+				"$merge_tool_path" \
+					"$LOCAL" "$MERGED" "$REMOTE" | cat
 			fi
+			check_unchanged
+		else
+			"$merge_tool_path" "$LOCAL" "$REMOTE" | cat
 		fi
-		check_unchanged
 		;;
 	vimdiff)
 		if merge_mode; then
 			touch "$BACKUP"
-			"$merge_tool_path" -d -c "wincmd l" "$LOCAL" "$MERGED" "$REMOTE"
+			"$merge_tool_path" -d -c "wincmd l" \
+				"$LOCAL" "$MERGED" "$REMOTE"
 			check_unchanged
 		else
-			"$merge_tool_path" -d -c "wincmd l" "$LOCAL" "$REMOTE"
+			"$merge_tool_path" -d -c "wincmd l" \
+				"$LOCAL" "$REMOTE"
 		fi
 		;;
 	gvimdiff)
 		if merge_mode; then
 			touch "$BACKUP"
-			"$merge_tool_path" -d -c "wincmd l" -f "$LOCAL" "$MERGED" "$REMOTE"
+			"$merge_tool_path" -d -c "wincmd l" -f \
+				"$LOCAL" "$MERGED" "$REMOTE"
 			check_unchanged
 		else
-			"$merge_tool_path" -d -c "wincmd l" -f "$LOCAL" "$REMOTE"
+			"$merge_tool_path" -d -c "wincmd l" -f \
+				"$LOCAL" "$REMOTE"
 		fi
 		;;
 	xxdiff)
 		if merge_mode; then
 			touch "$BACKUP"
-		fi
-		if $base_present; then
-			"$merge_tool_path" -X --show-merged-pane \
-			    -R 'Accel.SaveAsMerged: "Ctrl-S"' \
-			    -R 'Accel.Search: "Ctrl+F"' \
-			    -R 'Accel.SearchForward: "Ctrl-G"' \
-			    --merged-file "$MERGED" "$LOCAL" "$BASE" "$REMOTE"
-		else
-			if merge_mode; then
-				"$merge_tool_path" -X $extra \
+			if $base_present; then
+				"$merge_tool_path" -X --show-merged-pane \
 					-R 'Accel.SaveAsMerged: "Ctrl-S"' \
 					-R 'Accel.Search: "Ctrl+F"' \
 					-R 'Accel.SearchForward: "Ctrl-G"' \
-					--merged-file "$MERGED" "$LOCAL" "$REMOTE"
+					--merged-file "$MERGED" \
+					"$LOCAL" "$BASE" "$REMOTE"
 			else
-				"$merge_tool_path" \
+				"$merge_tool_path" -X $extra \
+					-R 'Accel.SaveAsMerged: "Ctrl-S"' \
 					-R 'Accel.Search: "Ctrl+F"' \
 					-R 'Accel.SearchForward: "Ctrl-G"' \
+					--merged-file "$MERGED" \
 					"$LOCAL" "$REMOTE"
 			fi
+			check_unchanged
+		else
+			"$merge_tool_path" \
+				-R 'Accel.Search: "Ctrl+F"' \
+				-R 'Accel.SearchForward: "Ctrl-G"' \
+				"$LOCAL" "$REMOTE"
 		fi
-		check_unchanged
 		;;
 	opendiff)
-		merge_mode && touch "$BACKUP"
-		if $base_present; then
-			"$merge_tool_path" "$LOCAL" "$REMOTE" \
-				-ancestor "$BASE" -merge "$MERGED" | cat
-		else
-			if merge_mode; then
+		if merge_mode; then
+			touch "$BACKUP"
+			if $base_present; then
 				"$merge_tool_path" "$LOCAL" "$REMOTE" \
+					-ancestor "$BASE" \
 					-merge "$MERGED" | cat
 			else
-				"$merge_tool_path" "$LOCAL" "$REMOTE" | cat
+				"$merge_tool_path" "$LOCAL" "$REMOTE" \
+					-merge "$MERGED" | cat
 			fi
+			check_unchanged
+		else
+			"$merge_tool_path" "$LOCAL" "$REMOTE" | cat
 		fi
-		check_unchanged
 		;;
 	ecmerge)
-		merge_mode && touch "$BACKUP"
-		if $base_present; then
-			"$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" \
-				--default --mode=merge3 --to="$MERGED"
+		if merge_mode; then
+			touch "$BACKUP"
+			if $base_present; then
+				"$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" \
+					--default --mode=merge3 --to="$MERGED"
+			else
+				"$merge_tool_path" "$LOCAL" "$REMOTE" \
+					--default --mode=merge2 --to="$MERGED"
+			fi
+			check_unchanged
 		else
 			"$merge_tool_path" "$LOCAL" "$REMOTE" \
 				--default --mode=merge2 --to="$MERGED"
 		fi
-		check_unchanged
 		;;
 	emerge)
-		if $base_present; then
-			"$merge_tool_path" -f emerge-files-with-ancestor-command \
-				"$LOCAL" "$REMOTE" "$BASE" "$(basename "$MERGED")"
+		if merge_mode; then
+			if $base_present; then
+				"$merge_tool_path" \
+					-f emerge-files-with-ancestor-command \
+					"$LOCAL" "$REMOTE" "$BASE" \
+					"$(basename "$MERGED")"
+			else
+				"$merge_tool_path" \
+					-f emerge-files-command \
+					"$LOCAL" "$REMOTE" \
+					"$(basename "$MERGED")"
+			fi
+			status=$?
 		else
 			"$merge_tool_path" -f emerge-files-command \
 				"$LOCAL" "$REMOTE" "$(basename "$MERGED")"
 		fi
-		status=$?
 		;;
 	tortoisemerge)
 		if $base_present; then
 			touch "$BACKUP"
-			"$merge_tool_path" -base:"$BASE" -mine:"$LOCAL" -theirs:"$REMOTE" -merged:"$MERGED"
+			"$merge_tool_path" \
+				-base:"$BASE" -mine:"$LOCAL" \
+				-theirs:"$REMOTE" -merged:"$MERGED"
 			check_unchanged
 		else
 			echo "TortoiseMerge cannot be used without a base" 1>&2
@@ -240,9 +262,14 @@ run_merge_tool () {
 		fi
 		;;
 	*)
-		if test -n "$merge_tool_cmd"; then
-			if merge_mode &&
-			test "$merge_tool_trust_exit_code" = "false"; then
+		if test -z "$merge_tool_cmd"; then
+			if merge_mode; then
+				status=1
+			fi
+			break
+		fi
+		if merge_mode; then
+			if test "$merge_tool_trust_exit_code" = "false"; then
 				touch "$BACKUP"
 				( eval $merge_tool_cmd )
 				check_unchanged
@@ -250,6 +277,8 @@ run_merge_tool () {
 				( eval $merge_tool_cmd )
 				status=$?
 			fi
+		else
+			( eval $merge_tool_cmd )
 		fi
 		;;
 	esac
@@ -257,27 +286,31 @@ run_merge_tool () {
 }
 
 guess_merge_tool () {
-	if diff_mode; then
+	tools="ecmerge"
+	if merge_mode; then
+		tools="$tools tortoisemerge"
+	else
 		kompare="kompare"
 	fi
 	if test -n "$DISPLAY"; then
 		if test -n "$GNOME_DESKTOP_SESSION_ID" ; then
-			tools="meld kdiff3 $kompare tkdiff"
+			tools="meld opendiff kdiff3 $kompare tkdiff $tools"
 			tools="$tools xxdiff gvimdiff diffuse"
 		else
-			tools="kdiff3 $kompare tkdiff xxdiff"
+			tools="opendiff kdiff3 $kompare tkdiff xxdiff $tools"
 			tools="$tools meld gvimdiff diffuse"
 		fi
 	fi
 	if echo "${VISUAL:-$EDITOR}" | grep emacs > /dev/null 2>&1; then
 		# $EDITOR is emacs so add emerge as a candidate
-		tools="$tools emerge opendiff vimdiff"
+		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 opendiff emerge"
+		tools="$tools vimdiff emerge"
 	else
-		tools="$tools opendiff emerge vimdiff"
+		tools="$tools emerge vimdiff"
 	fi
+	tools="$(echo "$tools" | sed -e 's/ +/ /g')"
 	echo >&2 "merge tool candidates: $tools"
 
 	# Loop over each candidate and stop when a valid merge tool is found.
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index cbfbe87..2586f86 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -202,10 +202,10 @@ test_expect_success 'difftool + mergetool config variables' '
 
 test_expect_success 'difftool.<tool>.path' '
 	git config difftool.tkdiff.path echo &&
-	diff=$(git difftool -y -t tkdiff branch) &&
+	diff=$(git difftool --tool=tkdiff --no-prompt branch) &&
 	git config --unset difftool.tkdiff.path &&
 	lines=$(echo "$diff" | grep file | wc -l) &&
-	test "$lines" = 1
+	test "$lines" -eq 1
 '
 
 test_done

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v4 14/14] difftool/mergetool: refactor commands to use git-mergetool--lib
  2009-04-08  6:09   ` Junio C Hamano
@ 2009-04-08  6:35     ` David Aguilar
  2009-04-08  6:40     ` Charles Bailey
  1 sibling, 0 replies; 6+ messages in thread
From: David Aguilar @ 2009-04-08  6:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: markus.heidelberg, git, charles

On  0, Junio C Hamano <gitster@pobox.com> wrote:
> Markus Heidelberg <markus.heidelberg@web.de> writes:
> 
> >> +	tools="$(echo "$tools" | sed -e 's/ +/ /g')"
> >
> > Doesn't work for me. For me 's/ \+/ /g' works.
> > ...like this: 's/[ 	]\+/ /g' (space and tab)
> 
> Pleae don't.  "s/  */ /g' should be the most portable (the point being "do
> not use one-or-more +").

v5 14/14.  good stuff.


> > Looks good to me, after these last 2 issues are adjusted.
> > Maybe resend the whole series then, so that Junio can apply them easily?
> 
> Thanks.  I've replaced the series with the following applied on top of the
> 'master', but I won't be merging them to 'next' for tonight, I guess.
> 
> ...
> [PATCH v2 07/14] difftool: add a -y shortcut for --no-prompt
> [PATCH v2 09/14] difftool: move 'git-difftool' out of contrib
> [PATCH v3 10/14] difftool: add various git-difftool tests
> [PATCH v2 11/14] difftool: add support for a difftool.prompt config variable
> [PATCH v4 14/14] difftool/mergetool: refactor commands to use git-mergetool--lib
> 
> It appears that, assuming that up to 13/14 above is what you two expected
> me to pick up, we would perhaps need to only replace 14/14?

Yup

> 
> As the final sanity check, please eyeball the attached interdiff, created
> this way:

That looks right to me.

> 
>  $ git checkout master^0
>  $ git am -s ./+da-fourteen-patches
>  $ A=$(git rev-parse HEAD)
>  $ git reset --hard master
>  $ git merge da/difftool ;# old series
>  $ git diff --stat -p HEAD $A ;# what's new in the new series?
> 
>  Documentation/config.txt               |    3 +-
>  Documentation/git-mergetool.txt        |    2 +-
>  Documentation/merge-config.txt         |    6 +-
>  command-list.txt                       |    1 -
>  contrib/completion/git-completion.bash |    2 +-
>  git-mergetool--lib.sh                  |  237 ++++++++++++++++++--------------
>  t/t7800-difftool.sh                    |    4 +-
>  7 files changed, 144 insertions(+), 111 deletions(-)
> 
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> ...

-- 

	David

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4 14/14] difftool/mergetool: refactor commands to use git-mergetool--lib
  2009-04-08  6:09   ` Junio C Hamano
  2009-04-08  6:35     ` David Aguilar
@ 2009-04-08  6:40     ` Charles Bailey
  2009-04-08  6:56       ` Junio C Hamano
  1 sibling, 1 reply; 6+ messages in thread
From: Charles Bailey @ 2009-04-08  6:40 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: markus.heidelberg, David Aguilar, git

On Tue, Apr 07, 2009 at 11:09:36PM -0700, Junio C Hamano wrote:
> Markus Heidelberg <markus.heidelberg@web.de> writes:
> > Looks good to me, after these last 2 issues are adjusted.
> > Maybe resend the whole series then, so that Junio can apply them easily?
> 
> Thanks.  I've replaced the series with the following applied on top of the
> 'master', but I won't be merging them to 'next' for tonight, I guess.

I feel I should be re-reviewing the mergetool patches, but while I
have sufficient time, it is usually at the weekend so I can't always
respond very rapidly.

I have been skimming this series, but most of the times that I thought
I could apply the series and have a thorough review, another comment
and re-roll has come through and I've decided to wait until it's
stable.

My slight concern is that this series has been changing very rapidly,
is everyone else happy that it is stable enough for merge into next?
Would you like my further input?

Charles.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4 14/14] difftool/mergetool: refactor commands to use git-mergetool--lib
  2009-04-08  6:40     ` Charles Bailey
@ 2009-04-08  6:56       ` Junio C Hamano
  0 siblings, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2009-04-08  6:56 UTC (permalink / raw)
  To: Charles Bailey; +Cc: markus.heidelberg, David Aguilar, git

Charles Bailey <charles@hashpling.org> writes:

> I have been skimming this series, but most of the times that I thought
> I could apply the series and have a thorough review, another comment
> and re-roll has come through and I've decided to wait until it's
> stable.
>
> My slight concern is that this series has been changing very rapidly,
> is everyone else happy that it is stable enough for merge into next?
> Would you like my further input?

I'd feel safer to pick up v5 of 14/14 from David and park the result in
'pu' for a few days.  I expect myself to be overloaded with the day job
for the rest of the week and shouldn't be advancing topics without having
enough concentration myself anyway.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2009-04-08  6:59 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-04-07 23:00 [PATCH v4 14/14] difftool/mergetool: refactor commands to use git-mergetool--lib David Aguilar
2009-04-08  5:33 ` Markus Heidelberg
2009-04-08  6:09   ` Junio C Hamano
2009-04-08  6:35     ` David Aguilar
2009-04-08  6:40     ` Charles Bailey
2009-04-08  6:56       ` Junio C Hamano

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).