From: Jeremy Morton <admin@game-point.net>
To: David Aguilar <davvid@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: [PATCH] mergetools/p4merge: Handle "/dev/null"
Date: Sat, 27 Oct 2012 09:47:05 +0100 [thread overview]
Message-ID: <508B9F89.7050909@game-point.net> (raw)
In-Reply-To: <1349925756-87801-1-git-send-email-davvid@gmail.com>
Sorry to be replying to this so late; I hadn't noticed the post until now!
I've tried putting that code in my p4merge script and yes it does indeed
work fine. However, it puts a temporary file in the working directory
which I'm not sure is a good idea? If we look at this patch which
actually solved pretty much the same problem, but when merging and,
during a merge conflict, a file was created in both branches:
https://github.com/git/git/commit/ec245ba
... it is creating a temp file in a proper temp dir, rather than in the
working dir. I think that would be the proper solution here. However,
I really want to get this fixed so I'd be happy for this band-aid fix of
the p4merge script to be checked in until we could get a patch more like
the aforementioned one, at a later date, to create empty files in a
proper temp dir and pass them as $LOCAL and $REMOTE. :-)
--
Best regards,
Jeremy Morton (Jez)
On 11/10/2012 04:22, David Aguilar wrote:
> p4merge does not properly handle the case where "/dev/null"
> is passed as a filename.
>
> Workaround it by creating a temporary file for this purpose.
>
> Reported-by: Jeremy Morton<admin@game-point.net>
> Signed-off-by: David Aguilar<davvid@gmail.com>
> ---
> Jeremy, can you test this?
>
> mergetools/p4merge | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/mergetools/p4merge b/mergetools/p4merge
> index 1a45c1b..295361a 100644
> --- a/mergetools/p4merge
> +++ b/mergetools/p4merge
> @@ -1,5 +1,30 @@
> diff_cmd () {
> + # p4merge does not like /dev/null
> + rm_local=
> + rm_remote=
> + if test "/dev/null" = "$LOCAL"
> + then
> + LOCAL="./p4merge-dev-null.LOCAL.$$"
> + >"$LOCAL"
> + rm_local=true
> + fi
> + if test "/dev/null" = "$REMOTE"
> + then
> + REMOTE="./p4merge-dev-null.REMOTE.$$"
> + >"$REMOTE"
> + rm_remote=true
> + fi
> +
> "$merge_tool_path" "$LOCAL" "$REMOTE"
> +
> + if test -n "$rm_local"
> + then
> + rm -f "$LOCAL"
> + fi
> + if test -n "$rm_remote"
> + then
> + rm -f "$REMOTE"
> + fi
> }
>
> merge_cmd () {
next prev parent reply other threads:[~2012-10-27 8:45 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-11 3:22 [PATCH] mergetools/p4merge: Handle "/dev/null" David Aguilar
2012-10-27 8:47 ` Jeremy Morton [this message]
2012-12-20 4:41 ` David Aguilar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=508B9F89.7050909@game-point.net \
--to=admin@game-point.net \
--cc=davvid@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.