git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Jay Soffian <jaysoffian@gmail.com>
Date
Oct 30, 2009, 00:47 UTC
Message-ID
<76718490910291747l165baf49tab781727d010610a@mail.gmail.com>
In-Reply-To
<20091029221234.GB32590@hashpling.org>
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.
Previous: Charles BaileyNext: Markus Heidelberg
Message 10 of 18 in “mergetool--lib: add p4merge as a pre-configured mergetool option”
  1. mergetool--lib: add p4merge as a pre-configured mergetool optionScott Chacon, Oct 27, 2009
  2. Charles BaileyOct 27, 2009
  3. Junio C HamanoOct 28, 2009
  4. David AguilarOct 28, 2009
  5. Scott ChaconOct 28, 2009
  6. mergetool--lib: add p4merge as a pre-configured mergetool optionScott Chacon, Oct 28, 2009
  7. Junio C HamanoOct 28, 2009
  8. Jay SoffianOct 29, 2009
  9. Charles BaileyOct 29, 2009
  10. Jay SoffianOct 30, 2009
  11. Markus HeidelbergOct 30, 2009
  12. Jay SoffianOct 30, 2009
  13. Markus HeidelbergOct 30, 2009
  14. Reece DunnOct 30, 2009
  15. Jay SoffianOct 30, 2009
  16. Markus HeidelbergOct 30, 2009
  17. Charles BaileyOct 30, 2009
  18. Junio C HamanoOct 30, 2009

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.