Re: [PATCH] mergetools: Add tortoisegitmerge helper

Sven Strickroth [off-list ref] writes:

- The TortoiseGit team renamed TortoiseMerge.exe to TortoiseGitMerge.exe
  (starting with 1.8.0) in order to make clear that this one has special
  support for git and prevent confusion with the TortoiseSVN TortoiseMerge
  version.

Wouldn't it make more sense in such a situation if your users can
keep using the old "tortoisemerge" configured in their configuration
and when the renamed one is found the mergetool automatically used
it, rather than the way your patch is done?  It seems that you are
forcing all the users to reconfigure or retrain their fingers.  Is
that the best we can do?  Is it too cumbersome to autodetect the
presense of tortoisegitmerge and redirect a request for tortoisemerge
to it, perhaps using translate_merge_tool_path (cf. mergetools/bc3)?

Assuming that people that have both variants will always want
mergetool to use tortoisegitmerge, that is.  If there are some
features missing from or extra bugs in tortoisegitmerge that makes
some people favor tortoisemerge, then giving two choices like your
patch does may make more sense.  I only know the difference between
the two from your four-line description above, but it does not look
like it is the case.

So in short, two tortoises and kompare are only valid as mergetool
but cannot be used as difftool?  No, I am reading it wrong.
merge.tool can be used for both, kompare can be used as difftool,
and two tortoises can only be used as mergetool.

This paragraph needs to be rewritten to unconfuse readers.  The
original is barely intelligible, and it becomes unreadable as the
set of tools subtracted by "minus" and added by "plus" grows.

Am 24.01.2013 20:51 schrieb Junio C Hamano:

Sven Strickroth [off-list ref] writes:

quoted

- The TortoiseGit team renamed TortoiseMerge.exe to TortoiseGitMerge.exe
  (starting with 1.8.0) in order to make clear that this one has special
  support for git and prevent confusion with the TortoiseSVN TortoiseMerge
  version.

Wouldn't it make more sense in such a situation if your users can
keep using the old "tortoisemerge" configured in their configuration
and when the renamed one is found the mergetool automatically used
it, rather than the way your patch is done?

That was also my first idea, however, TortoiseMerge uses parameters as
follows: '-base:"$BASE"'. TortoiseGitMerge uses values separated by
space from keys: '-base "$BASE"'. So both are incompatible (the first
approach has problems with spaces in filenames, the TortoiseGitMerge
approach fixes this).

quoted

diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
index 4314ad0..13cbe5b 100644
--- a/Documentation/diff-config.txt
+++ b/Documentation/diff-config.txt

@@ -151,7 +151,7 @@ diff.<driver>.cachetextconv::
 diff.tool::
 	The diff tool to be used by linkgit:git-difftool[1].  This
 	option overrides `merge.tool`, and has the same valid built-in
-	values as `merge.tool` minus "tortoisemerge" and plus
-	"kompare".  Any other value is treated as a custom diff tool,
+	values as `merge.tool` minus "tortoisemerge"/"tortoisegitmerge" and
+	plus "kompare".  Any other value is treated as a custom diff tool,
 	and there must be a corresponding `difftool.<tool>.cmd`
 	option.

So in short, two tortoises and kompare are only valid as mergetool
but cannot be used as difftool?  No, I am reading it wrong.
merge.tool can be used for both, kompare can be used as difftool,
and two tortoises can only be used as mergetool.

This paragraph needs to be rewritten to unconfuse readers.  The
original is barely intelligible, and it becomes unreadable as the
set of tools subtracted by "minus" and added by "plus" grows.

But I think this should not be part of this patch.

-- 
Best regards,
 Sven Strickroth
 PGP key id F5A9D4C4 @ any key-server

TortoiseGitMerge and filenames with spaces

- The TortoiseGit team renamed TortoiseMerge.exe to TortoiseGitMerge.exe
  (starting with 1.8.0) in order to make clear that this one has special
  support for git, (uses spaces as cli parameter key-value separators)
  and prevent confusion with the TortoiseSVN TortoiseMerge version.
- The tortoisemerge mergetool does not work with filenames which have
  a space in it. Fixing this required changes in git and also in
  TortoiseGitMerge; see https://github.com/msysgit/msysgit/issues/57.

Signed-off-by: Sven Strickroth <redacted>
Reported-by: Sebastian Schuberth <redacted>
---
 mergetools/tortoisemerge | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/mergetools/tortoisemerge b/mergetools/tortoisemerge
index ed7db49..9890737 100644
--- a/mergetools/tortoisemerge
+++ b/mergetools/tortoisemerge

@@ -6,12 +6,28 @@ merge_cmd () {
 	if $base_present
 	then
 		touch "$BACKUP"
-		"$merge_tool_path" \
-			-base:"$BASE" -mine:"$LOCAL" \
-			-theirs:"$REMOTE" -merged:"$MERGED"
+		if test "$merge_tool_path" == "tortoisegitmerge"
+		then
+			"$merge_tool_path" \
+				-base "$BASE" -mine "$LOCAL" \
+				-theirs "$REMOTE" -merged "$MERGED"
+		else 
+			"$merge_tool_path" \
+				-base:"$BASE" -mine:"$LOCAL" \
+				-theirs:"$REMOTE" -merged:"$MERGED"
+		fi
 		check_unchanged
 	else
-		echo "TortoiseMerge cannot be used without a base" 1>&2
+		echo "$merge_tool_path cannot be used without a base" 1>&2
 		return 1
 	fi
 }
+
+translate_merge_tool_path() {
+	if type tortoisegitmerge >/dev/null 2>/dev/null
+	then
+		echo tortoisegitmerge
+	else
+		echo tortoisemerge
+	fi
+}

-- 
Best regards,
 Sven Strickroth
 PGP key id F5A9D4C4 @ any key-server

On Fri, Jan 25, 2013 at 1:06 AM, Sven Strickroth
[off-list ref] wrote:

I like the approach this is taking.  Thank you.
I have one small note:

I think this should use "=" instead of "==" here.

It might also make sense to wrap a basename call around it
so that users can set their own mergetool.tortoisemerge.path

basename="$(basename "$merge_tool_path" .exe)"
if test "$basename" = "tortoisegitmerge"
...

+               then
+                       "$merge_tool_path" \
+                               -base "$BASE" -mine "$LOCAL" \
+                               -theirs "$REMOTE" -merged "$MERGED"
+               else
+                       "$merge_tool_path" \
+                               -base:"$BASE" -mine:"$LOCAL" \
+                               -theirs:"$REMOTE" -merged:"$MERGED"
+               fi
                check_unchanged
        else
-               echo "TortoiseMerge cannot be used without a base" 1>&2
+               echo "$merge_tool_path cannot be used without a base" 1>&2
                return 1
        fi
 }
+
+translate_merge_tool_path() {
+       if type tortoisegitmerge >/dev/null 2>/dev/null
+       then
+               echo tortoisegitmerge
+       else
+               echo tortoisemerge
+       fi
+}
--
Best regards,
 Sven Strickroth
 PGP key id F5A9D4C4 @ any key-server

-- 
David

 TortoiseGitMerge and filenames with spaces

- The TortoiseGit team renamed TortoiseMerge.exe to TortoiseGitMerge.exe
  (starting with 1.8.0) in order to make clear that this one has special
  support for git, (uses spaces as cli parameter key-value separators)
  and prevent confusion with the TortoiseSVN TortoiseMerge version.
- The tortoisemerge mergetool does not work with filenames which have
  a space in it. Fixing this required changes in git and also in
  TortoiseGitMerge; see https://github.com/msysgit/msysgit/issues/57.

Signed-off-by: Sven Strickroth <redacted>
Reported-by: Sebastian Schuberth <redacted>
---
 mergetools/tortoisemerge | 51 ++++++++++++++++++++++++++++++++----------------
 1 file changed, 34 insertions(+), 17 deletions(-)

diff --git a/mergetools/tortoisemerge b/mergetools/tortoisemerge
index ed7db49..8ee99a5 100644
--- a/mergetools/tortoisemerge
+++ b/mergetools/tortoisemerge

@@ -1,17 +1,34 @@
-can_diff () {
-	return 1
-}
-
-merge_cmd () {
-	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
-		return 1
-	fi
-}
+can_diff () {
+	return 1
+}
+
+merge_cmd () {
+	if $base_present
+	then
+		touch "$BACKUP"
+		basename="$(basename "$merge_tool_path" .exe)"
+		if test "$basename" = "tortoisegitmerge"
+		then
+			"$merge_tool_path" \
+				-base "$BASE" -mine "$LOCAL" \
+				-theirs "$REMOTE" -merged "$MERGED"
+		else 
+			"$merge_tool_path" \
+				-base:"$BASE" -mine:"$LOCAL" \
+				-theirs:"$REMOTE" -merged:"$MERGED"
+		fi
+		check_unchanged
+	else
+		echo "$merge_tool_path cannot be used without a base" 1>&2
+		return 1
+	fi
+}
+
+translate_merge_tool_path() {
+	if type tortoisegitmerge >/dev/null 2>/dev/null
+	then
+		echo tortoisegitmerge
+	else
+		echo tortoisemerge
+	fi
+}

-- 
Best regards,
 Sven Strickroth
 PGP key id F5A9D4C4 @ any key-server