[PATCH 2/2] mergetool--lib: simplify API usage by removing more global variables

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

 



The mergetool--lib scriplet was tricky to use because it relied upon
the existance of several global shell variables.  This removes more
global variables so that things are simpler for callers.

A side effect is that some variables are recomputed each time
run_merge_tool() is called, but the overhead for recomputing
them is justified by the simpler implementation.

Signed-off-by: David Aguilar <davvid@xxxxxxxxx>
---

This addresses Charles' notes by making the mergetool--lib
simpler to use.

We can now simply use:
	get_merge_tool
	get_merge_tool_path
	get_merge_tool_cmd
and things will work consistently.

Previous versions used global variables that would give
unexpected results if they had been set previously.

 Documentation/git-mergetool--lib.txt |   16 ++---
 git-difftool--helper.sh              |    6 +-
 git-mergetool--lib.sh                |  109 ++++++++++++++++++----------------
 git-mergetool.sh                     |   15 +++--
 4 files changed, 76 insertions(+), 70 deletions(-)

diff --git a/Documentation/git-mergetool--lib.txt b/Documentation/git-mergetool--lib.txt
index 3d57031..78eb03f 100644
--- a/Documentation/git-mergetool--lib.txt
+++ b/Documentation/git-mergetool--lib.txt
@@ -7,7 +7,7 @@ git-mergetool--lib - Common git merge tool shell scriptlets
 
 SYNOPSIS
 --------
-'. "$(git --exec-path)/git-mergetool--lib"'
+'TOOL_MODE=(diff|merge) . "$(git --exec-path)/git-mergetool--lib"'
 
 DESCRIPTION
 -----------
@@ -20,14 +20,14 @@ 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.
+Before sourcing 'git-mergetool--lib', your script must set `TOOL_MODE`
+to define the operation mode for the functions listed below.
+'diff' and 'merge' are valid values.
 
 FUNCTIONS
 ---------
 get_merge_tool::
-	returns a merge tool
+	returns a merge tool.
 
 get_merge_tool_cmd::
 	returns the custom command for a merge tool.
@@ -38,10 +38,8 @@ get_merge_tool_path::
 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.
+	'$MERGED', '$LOCAL', '$REMOTE', and '$BASE' must be defined
+	for use by the merge tool.
 
 Author
 ------
diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh
index b450036..57e8e32 100755
--- a/git-difftool--helper.sh
+++ b/git-difftool--helper.sh
@@ -47,9 +47,9 @@ launch_merge_tool () {
 test -n "$GIT_MERGE_TOOL" && merge_tool="$GIT_MERGE_TOOL"
 test -n "$GIT_DIFF_TOOL" && merge_tool="$GIT_DIFF_TOOL"
 
-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
+if test -z "$merge_tool"; then
+	merge_tool="$(get_merge_tool)" || exit
+fi
 
 # 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
index c5db24e..a16a279 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -8,25 +8,20 @@ merge_mode() {
 }
 
 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
+	case "$1" in
+	vimdiff)
+		echo vim
+		;;
+	gvimdiff)
+		echo gvim
+		;;
+	emerge)
+		echo emacs
+		;;
+	*)
+		echo "$1"
+		;;
+	esac
 }
 
 check_unchanged () {
@@ -69,15 +64,22 @@ valid_tool () {
 }
 
 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"
+	# Prints the custom command for a merge tool
+	if test -n "$1"; then
+		merge_tool="$1"
+	else
+		merge_tool="$(get_merge_tool)"
+	fi
+	if diff_mode; then
+		echo "$(git config difftool.$merge_tool.cmd ||
+		        git config mergetool.$merge_tool.cmd)"
+	else
+		echo "$(git config mergetool.$merge_tool.cmd)"
+	fi
 }
 
 run_merge_tool () {
+	merge_tool_path="$(get_merge_tool_path "$1")" || exit
 	base_present="$2"
 	status=0
 
@@ -103,9 +105,9 @@ run_merge_tool () {
 			status=$?
 		else
 			("$merge_tool_path" --auto \
-			 --L1 "$MERGED (A)" \
-			 --L2 "$MERGED (B)" "$LOCAL" "$REMOTE" \
-			 > /dev/null 2>&1)
+				--L1 "$MERGED (A)" \
+				--L2 "$MERGED (B)" "$LOCAL" "$REMOTE" \
+			> /dev/null 2>&1)
 		fi
 		;;
 	kompare)
@@ -262,6 +264,7 @@ run_merge_tool () {
 		fi
 		;;
 	*)
+		merge_tool_cmd="$(get_merge_tool_cmd "$1")"
 		if test -z "$merge_tool_cmd"; then
 			if merge_mode; then
 				status=1
@@ -269,7 +272,9 @@ run_merge_tool () {
 			break
 		fi
 		if merge_mode; then
-			if test "$merge_tool_trust_exit_code" = "false"; then
+			trust_exit_code="$(git config --bool \
+				mergetool."$1".trustExitCode || echo false)"
+			if test "$trust_exit_code" = "false"; then
 				touch "$BACKUP"
 				( eval $merge_tool_cmd )
 				check_unchanged
@@ -315,64 +320,66 @@ guess_merge_tool () {
 	do
 		merge_tool_path="$(translate_merge_tool_path "$i")"
 		if type "$merge_tool_path" > /dev/null 2>&1; then
-			merge_tool="$i"
-			break
+			echo "$i"
+			return 0
 		fi
 	done
 
-	if test -z "$merge_tool" ; then
-		echo >&2 "No known merge resolution program available."
-		return 1
-	fi
-	echo "$merge_tool"
+	echo >&2 "No known merge resolution program available."
+	return 1
 }
 
 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)
+		merge_tool=$(git config diff.tool || git config merge.tool)
+	else
+		merge_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"
+	echo "$merge_tool"
 }
 
 get_merge_tool_path () {
 	# A merge tool has been set, so verify that it's valid.
+	if test -n "$1"; then
+		merge_tool="$1"
+	else
+		merge_tool="$(get_merge_tool)"
+	fi
 	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)
+		merge_tool_path=$(git config difftool."$merge_tool".path ||
+		                  git config mergetool."$merge_tool".path)
+	else
+		merge_tool_path=$(git config mergetool."$merge_tool".path)
 	fi
 	if test -z "$merge_tool_path"; then
-		merge_tool_path=$(git config mergetool."$merge_tool".path)
+		merge_tool_path="$(translate_merge_tool_path "$merge_tool")"
 	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'"
+	if test -z "$(get_merge_tool_cmd "$merge_tool")" &&
+	! 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
+	merge_tool=$(get_configured_merge_tool)
 	# 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
+		merge_tool="$(guess_merge_tool)" || exit
 	fi
 	echo "$merge_tool"
 }
diff --git a/git-mergetool.sh b/git-mergetool.sh
index 2e3e02b..b52a741 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -174,9 +174,11 @@ merge_file () {
 	read ans
     fi
 
-    present=false
-    base_present &&
-    present=true
+    if base_present; then
+	    present=true
+    else
+	    present=false
+    fi
 
     if ! run_merge_tool "$merge_tool" "$present"; then
 	echo "merge of $MERGED failed" 1>&2
@@ -254,12 +256,11 @@ prompt_after_failed_merge() {
     done
 }
 
-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
+if test -z "$merge_tool"; then
+    merge_tool=$(get_merge_tool "$merge_tool") || exit
+fi
 merge_keep_backup="$(git config --bool mergetool.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.2.2.471.g6da14

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