Re: [PATCH] mergetools: Enable tortoisemerge to handle filenames with spaces with TortoiseGitMerge

2013-02-01 Thread Junio C Hamano
David Aguilar  writes:

> On Fri, Feb 1, 2013 at 12:16 PM, Sven Strickroth
>  wrote:
>> TortoiseMerge caused to whole
>> argument instead of just the file name to be quoted
>
> s/caused to whole/caused the whole/
>
> I think this commit message is very nice.  Is it too late to replace
> the current patch with this one?

Haven't merged it to 'next'; I will replace with this, with a bit of
retitling to make it shorter.


commit 81ed7b9581f7eafb334824264abb492d85a5ffb8
Author: Sven Strickroth 
Date:   Fri Feb 1 21:16:30 2013 +0100

mergetools: teach tortoisemerge to handle filenames with SP correctly

TortoiseGitMerge, unlike TortoiseMerge, can be told to handle paths
with spaces in them by using -option "$FILE" (not -option:"$FILE",
which does not work for such paths) syntax.

This change was necessary because of MSYS path mangling [1], the ":"
after the "base" etc. arguments to TortoiseMerge caused the whole
argument instead of just the file name to be quoted in case of file
names with spaces. So TortoiseMerge was passed

"-base:new file.txt"

instead of

-base:"new file.txt"

(including the quotes). To work around this, TortoiseGitMerge does not
require the ":" after the arguments anymore which fixes handling file
names with spaces [2] (as written above).

[1] http://www.mingw.org/wiki/Posix_path_conversion
[2] https://github.com/msysgit/msysgit/issues/57

Signed-off-by: Sven Strickroth 
Reported-by: Sebastian Schuberth 
Signed-off-by: Junio C Hamano 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] mergetools: Enable tortoisemerge to handle filenames with spaces with TortoiseGitMerge

2013-02-01 Thread David Aguilar
On Fri, Feb 1, 2013 at 12:16 PM, Sven Strickroth
 wrote:
> TortoiseMerge caused to whole
> argument instead of just the file name to be quoted

s/caused to whole/caused the whole/

I think this commit message is very nice.  Is it too late to replace
the current patch with this one?
-- 
David
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] mergetools: Enable tortoisemerge to handle filenames with spaces with TortoiseGitMerge

2013-02-01 Thread Sven Strickroth
TortoiseGitMerge, unlike TortoiseMerge, can be told to handle paths
with spaces in them by using -option "$FILE" (not -option:"$FILE",
which does not work for such paths) syntax.

This change was necessary because of MSYS path mangling [1], the ":"
after the "base" etc. arguments to TortoiseMerge caused to whole
argument instead of just the file name to be quoted in case of file
names with spaces. So TortoiseMerge was passed

"-base:new file.txt"

instead of

-base:"new file.txt"

(including the quotes). To work around this, TortoiseGitMerge does not
require the ":" after the arguments anymore which fixes handling file
names with spaces [2] (as written above).

[1] http://www.mingw.org/wiki/Posix_path_conversion
[2] https://github.com/msysgit/msysgit/issues/57

Signed-off-by: Sven Strickroth 
Reported-by: Sebastian Schuberth 
---
 mergetools/tortoisemerge | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/mergetools/tortoisemerge b/mergetools/tortoisemerge
index 8476afa..3b89f1c 100644
--- a/mergetools/tortoisemerge
+++ b/mergetools/tortoisemerge
@@ -6,9 +6,17 @@ merge_cmd () {
if $base_present
then
touch "$BACKUP"
-   "$merge_tool_path" \
-   -base:"$BASE" -mine:"$LOCAL" \
-   -theirs:"$REMOTE" -merged:"$MERGED"
+   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
-- 
1.8.1.msysgit.1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] mergetools: Enable tortoisemerge to handle filenames with

2013-02-01 Thread Sven Strickroth
Am 01.02.2013 21:15 schrieb Junio C Hamano:
>> TortoiseGitMerge is an improved version of TortoiseMerge specifically
>> for use with Git on Windows. Due to MSYS path mangling [1], the ":"
>> after the "base" etc. arguments to TortoiseMerge caused to whole
>> argument instead of just the file name to be quoted in case of file
>> names with spaces. So TortoiseMerge was passed
>>
>> "-base:new file.txt"
>>
>> instead of
>>
>> -base:"new file.txt"
>>
>> (including the quotes). To work around this, TortoiseGitMerge does not
>> require the ":" after the arguments anymore which fixes handling file
>> names with spaces.
>>
>> [1] http://www.mingw.org/wiki/Posix_path_conversion
> 
> Sven?

I just mailed a new patch. Thanks to Sebastian for pointing this out!

@Junio: Feel free to optimize the commit message.

-- 
Best regards,
 Sven Strickroth
 PGP key id F5A9D4C4 @ any key-server
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] mergetools: Enable tortoisemerge to handle filenames with

2013-02-01 Thread Sven Strickroth
Am 01.02.2013 21:07 schrieb Sebastian Schuberth:
> mergetools: Teach tortoisemerge about TortoiseGitMerge

This subject doesn't make any sense if we don't combine the two patches.

-- 
Best regards,
 Sven Strickroth
 PGP key id F5A9D4C4 @ any key-server
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] mergetools: Enable tortoisemerge to handle filenames with

2013-02-01 Thread Junio C Hamano
Sebastian Schuberth  writes:

> The commit message still does not mention MSYS path mangling at all,
> which probably is why the reasoning of this patch was not yet fully
> understood.

Ahh, you are very right.  I didn't realize that was what this funny
"with colon, with SP" business was about.

> I'd recommend something like the following:
>
> mergetools: Teach tortoisemerge about TortoiseGitMerge
>
> TortoiseGitMerge is an improved version of TortoiseMerge specifically
> for use with Git on Windows. Due to MSYS path mangling [1], the ":"
> after the "base" etc. arguments to TortoiseMerge caused to whole
> argument instead of just the file name to be quoted in case of file
> names with spaces. So TortoiseMerge was passed
>
> "-base:new file.txt"
>
> instead of
>
> -base:"new file.txt"
>
> (including the quotes). To work around this, TortoiseGitMerge does not
> require the ":" after the arguments anymore which fixes handling file
> names with spaces.
>
> [1] http://www.mingw.org/wiki/Posix_path_conversion

Sven?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] mergetools: Enable tortoisemerge to handle filenames with

2013-02-01 Thread Sebastian Schuberth
On Fri, Feb 1, 2013 at 8:33 PM, Sven Strickroth
 wrote:

> TortoiseGitMerge, unlike TortoiseMerge, can be told to handle paths
> with spaces in them by using -option "$FILE" (not -option:"$FILE",
> which does not work for such paths) syntax. Both do not have a fully
> posix compatible cli parameter parser, however, TortoiseGitMerge was
> modified in order to handle filenames with spaces correctly. The
> "-key value" form was choosen because this way no escaping for
> quotes within quotes is necessary; see
> https://github.com/msysgit/msysgit/issues/57

The commit message still does not mention MSYS path mangling at all,
which probably is why the reasoning of this patch was not yet fully
understood. I'd recommend something like the following:

mergetools: Teach tortoisemerge about TortoiseGitMerge

TortoiseGitMerge is an improved version of TortoiseMerge specifically
for use with Git on Windows. Due to MSYS path mangling [1], the ":"
after the "base" etc. arguments to TortoiseMerge caused to whole
argument instead of just the file name to be quoted in case of file
names with spaces. So TortoiseMerge was passed

"-base:new file.txt"

instead of

-base:"new file.txt"

(including the quotes). To work around this, TortoiseGitMerge does not
require the ":" after the arguments anymore which fixes handling file
names with spaces.

[1] http://www.mingw.org/wiki/Posix_path_conversion

-- 
Sebastian Schuberth
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] mergetools: Enable tortoisemerge to handle filenames with

2013-02-01 Thread Sven Strickroth
spaces with TortoiseGitMerge

TortoiseGitMerge, unlike TortoiseMerge, can be told to handle paths
with spaces in them by using -option "$FILE" (not -option:"$FILE",
which does not work for such paths) syntax. Both do not have a fully
posix compatible cli parameter parser, however, TortoiseGitMerge was
modified in order to handle filenames with spaces correctly. The
"-key value" form was choosen because this way no escaping for
quotes within quotes is necessary; see
https://github.com/msysgit/msysgit/issues/57

Signed-off-by: Sven Strickroth 
Reported-by: Sebastian Schuberth 
---
 mergetools/tortoisemerge | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/mergetools/tortoisemerge b/mergetools/tortoisemerge
index 8476afa..3b89f1c 100644
--- a/mergetools/tortoisemerge
+++ b/mergetools/tortoisemerge
@@ -6,9 +6,17 @@ merge_cmd () {
if $base_present
then
touch "$BACKUP"
-   "$merge_tool_path" \
-   -base:"$BASE" -mine:"$LOCAL" \
-   -theirs:"$REMOTE" -merged:"$MERGED"
+   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
-- 
Best regards,
 Sven Strickroth
 PGP key id F5A9D4C4 @ any key-server

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html