Re: [PATCH] Make sure to use Araxis' "compare" and not e.g. ImageMagick's

Junio C Hamano [off-list ref] writes:

If we limit the problem space by special casing Windows installation
(e.g. check "uname -s" or something), would it make the problem
easier to solve?  Perhaps it is much more likely that the path the
program is installed in can be safely identified with a call to
"type --path compare" (bash is the only shell shipped in msysgit,
isn't it?).

E.g. something along the lines of your original patch.  I do not
know what other commands are typically installed in the same
directory as "compare", so it is likely you need to fix the name of
the file to let us positively identify "compare" is from the Araxis
suite.

 mergetools/araxis | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/mergetools/araxis b/mergetools/araxis
index 64f97c5..1180a32 100644
--- a/mergetools/araxis
+++ b/mergetools/araxis

@@ -16,5 +16,18 @@ merge_cmd () {
 }
 
 translate_merge_tool_path() {
-	echo compare
+	case "$BASH_VERSION" in
+	??*)
+		# we can safely use "type --path"
+		if test -f $(dirname "$(type --path compare)")/AraxisMerge
+		then
+			echo compare
+		else
+			echo "$1"
+		fi
+		;;
+	*)
+		echo compare
+		;;
+	esac
 }

On 24.07.2012 00:41, Junio C Hamano wrote:

+		if test -f $(dirname "$(type --path compare)")/AraxisMerge

We would need additional quotes around the whole path here as the Windows installation path is usually something like "C:\Program Files\Araxis\Araxis Merge" and contains spaces.

Moreover, "test -f" requires the ".exe" extension to be explicitly present for the file to test. But I'd rather not do that because the test would be specific to Windows then and e.g. not work on Mac OS X. That's why I'd still like to use ls like in my first patch:

 mergetools/araxis | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/mergetools/araxis b/mergetools/araxis
index 64f97c5..c406ead 100644
--- a/mergetools/araxis
+++ b/mergetools/araxis

@@ -16,5 +16,18 @@ merge_cmd () {
 }
 
 translate_merge_tool_path() {
-	echo compare
+	case "$BASH_VERSION" in
+	??*)
+		# we can safely use "type --path"
+		if ls "$(dirname "$(type --path compare)")"/Araxis* >/dev/null 2>&1
+		then
+			echo compare
+		else
+			echo "$1"
+		fi
+		;;
+	*)
+		echo compare
+		;;
+	esac
 }

-- 
Sebastian Schuberth