Re: [PATCH 2/2] mergetools: Make tortoisemerge work with

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

 



Sven Strickroth <sven.strickroth@xxxxxxxxxxxxxxx> writes:

> Am 26.01.2013 08:10 schrieb David Aguilar:
>> These patches look correct (I do not have the tool to test)
>> but I think we should fixup this commit message.
>> 
>> How about something like...
>> 
>> mergetools: Teach tortoisemerge about TortoiseGitMerge
>> 
>> TortoiseGitMerge improved its syntax to allow for file paths
>> with spaces.  Detect when it is installed and prefer it over
>> TortoiseMerge.
>
> This message implies, that I have to combine two patches again?!

We can see that [1/2] teaches mergetool to use tortoisegitmerge when
the user tells it to use tortoisemerge and the former is available.

The change in [2/2] is to use -base "$BASE" instead of -base:"$BASE"
when the real tool is tortoisegitmerge (when it is tortoisemerge,
nothing changes).

By reading these two patches, I would imagine that tortoisegitmerge
can accept both forms, i.e. -base:"$BASE" and -base "$BASE", but the
patch [2/2] considers that the latter form is preferrable in some
way.  As you talked about "paths with SPs in them", I would imagine
that is the difference?  That is -base:"$BASE" form will not work if
the varilable $BASE has a SP in it (even though it is encolosed in
dq, which does not make much sense from my POSIXy point of view, but
perhaps command line argument processing in the Windows land may
have different rules) but if you write -base "$BASE" then "$BASE"
will be taken as a single thing even it has a SP in it?  Also I
would guess that the reason why patch [2/2] does this only for
tortoisegitmerge is either because tortoisemerge will break paths
with SPs even if it is given -base "$BASE" form, or because it only
accepts -base:"$BASE" form? I cannot read it from your description,
but let's assume that is the reason.

If that is the case, then the log message for the second patch would
be easier to understand if it says so in a more explicit way,
perhaps like this:

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

	Use it to allow such paths to be handled correctly.

But I cannot read exactly why the patch [2/2] considers -base "$BASE"
is preferrable over -base:"$BASE" from your original description, so
this may well be way off the mark.

In short, I think proposed log message for [2/2] was not clear what
is being fixed and how.
--
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]