Re: [PATCH] mergetool--lib: add p4merge as a pre-configured mergetool option

[Jay Soffian <jaysoffian@gmail.com>]

On Thu, Oct 29, 2009 at 6:12 PM, Charles Bailey <charles@hashpling.org> wrote:
> I'm not sure I understand why only p4merge on Mac OS X is special, we
> don't seem to treat any other mergetool specially and we don't seem to
> need absolute paths anywhere else.

On other platforms, the merge tool is very likely to be in your PATH.

On OS X, p4merge is going to be installed as part of an application
bundle (/Applications/p4merge.app or $HOME/Applications/p4merge.app).
This is virtually never going to be in a user's PATH.

So in order to provide equivalent behavior for OS X as Linux (i.e., so
that you can just specify p4merge as the mergetool without having to
provide it's path), we need to look in these additional locations.

> If it's a Mac OS X only thing, can we (and should we) avoid special
> treatment for p4merge on other platforms?

It is a Mac OS X only thing. Yes, we could avoid looking in these
locations on other platforms, but why? Using type to look for the
executable is virtually no cost. The alternative (calling uname to
determine the platform) requires running a separate process.

> The only other question I have is what are the merits of using
> /dev/null as the base vs. a second copy of the local version in the
> baseless merge case? It's the only other difference between the two
> p4merge patches that I noticed.

p4merge's argument handling is stupid, you need to pass it a dummy
argument in some cases. A second copy of the local version is probably
better than /dev/null. Actually, I think just passing it an empty
argument ("") works too.

> Perhaps we could consider having both p4merge and launchp4merge as
> separate options?

I think that would be over-engineered. Decide on one or the other.
They have slightly different semantics as I've previously described.
Personally I think calling launchp4merge provides a more Mac-like
behavior, but honestly it doesn't make much difference.

j.