git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Bracey <kevin@bracey.fi>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
	David Aguilar <davvid@gmail.com>,
	Ciaran Jessup <ciaranj@gmail.com>,
	Scott Chacon <schacon@gmail.com>
Subject: Re: [PATCH 1/2] p4merge: swap LOCAL and REMOTE for mergetool
Date: Thu, 07 Mar 2013 08:16:13 +0200	[thread overview]
Message-ID: <513830AD.10302@bracey.fi> (raw)
In-Reply-To: <7vlia0nj0i.fsf@alter.siamese.dyndns.org>

On 07/03/2013 02:36, Junio C Hamano wrote:
> Kevin Bracey <kevin@bracey.fi> writes:
>
>> Reverse LOCAL and REMOTE when invoking P4Merge as a mergetool, so that
>> the incoming branch is now in the left-hand, blue triangle pane, and the
>> current branch is in the right-hand, green circle pane.
> Given that the ordering of the three variants has been the way it is
> since the very initial version by Scott, I'll sit on this patch
> until hearing from those Cc'ed (who presumably do use p4merge,
> unlike I who don't) that it is a good change.
>
I agree that this is the controversial patch of the two. It's going to 
chuck away 3-4 years of what Git users are used to, albeit in favour of 
a decade of what Perforce users are used to. And it also makes it 
inconsistent with all the other mergetools (at least assuming their 
display matches their command line).

I checked for any historical discussion from when this was added about 
the order, and there was none. So I'm assuming it was just done to match 
the other tools, maybe not realising P4Merge's "theirs/ours" convention. 
There was no explicit recognition at the time that they were breaking 
the Perforce convention, or that the order had an effect.

I've used both Git and Perforce for quite a while, but have only just 
started using P4Merge with Git. It seemed weirdly off and unintuitive to 
me at first, until I suddenly realised that it was just backwards.  I 
would have settled for just having to get used to driving on the other 
side of the road, and matching other mergetools, until I realised that 
it effectively broke display of common changes. That's a problem.

On consistency, personally, I think there's an argument for reversing 
all the mergetools to match this way, as I find this orientation more 
intuitively aligns with difftool. But I'm not bold enough to suggest 
that. Yet.

Kevin

  reply	other threads:[~2013-03-07  6:55 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-06 20:32 [PATCH 0/2] Improve P4Merge mergetool invocation Kevin Bracey
2013-03-06 20:32 ` [PATCH 1/2] p4merge: swap LOCAL and REMOTE for mergetool Kevin Bracey
2013-03-07  0:36   ` Junio C Hamano
2013-03-07  6:16     ` Kevin Bracey [this message]
2013-03-07  7:23       ` Junio C Hamano
2013-03-07 17:14         ` Kevin Bracey
2013-03-07 19:10           ` Junio C Hamano
2013-03-07 19:50             ` David Aguilar
2013-03-07 20:31               ` Junio C Hamano
2013-03-06 20:32 ` [PATCH 2/2] p4merge: create a virtual base if none available Kevin Bracey
2013-03-07  2:23   ` David Aguilar
2013-03-07  6:28     ` Kevin Bracey
2013-03-07  7:25     ` Junio C Hamano
2013-03-07  3:33   ` David Aguilar
2013-03-09 19:20 ` [PATCH v2 0/3] Improve P4Merge mergetool invocation Kevin Bracey
2013-03-09 19:20   ` [PATCH v2 1/3] mergetools/p4merge: swap LOCAL and REMOTE Kevin Bracey
2013-03-09 19:20   ` [PATCH v2 2/3] mergetools/p4merge: create a base if none available Kevin Bracey
2013-03-10  4:55     ` Junio C Hamano
2013-03-09 19:21   ` [PATCH v2 3/3] git-merge-one-file: revise merge error reporting Kevin Bracey
2013-03-13  1:12 ` [PATCH v3 1/3] mergetools/p4merge: swap LOCAL and REMOTE Kevin Bracey
2013-03-13  1:12   ` [PATCH v3 2/3] mergetools/p4merge: create a base if none available Kevin Bracey
2013-03-13  1:12   ` [PATCH v3 3/3] git-merge-one-file: revise merge error reporting Kevin Bracey
2013-03-13  9:03     ` David Aguilar
2013-03-24 12:26       ` [PATCH v2 0/3] git-merge-one-file " Kevin Bracey
2013-03-24 12:26         ` [PATCH v2 1/3] git-merge-one-file: style cleanup Kevin Bracey
2013-03-24 12:26         ` [PATCH v2 2/3] git-merge-one-file: send "ERROR:" messages to stderr Kevin Bracey
2013-03-24 12:26         ` [PATCH v2 3/3] git-merge-one-file: revise merge error reporting Kevin Bracey
2013-03-25 17:04           ` Junio C Hamano
2013-03-25 17:17             ` Junio C Hamano
2013-03-25 17:20               ` Junio C Hamano
2013-03-25 19:24               ` Eric Sunshine
2013-03-13 17:57     ` [PATCH v3 " Junio C Hamano
2013-03-14  6:27       ` Kevin Bracey
2013-03-14 14:56         ` Junio C Hamano
2013-03-14 17:31           ` Kevin Bracey
2013-03-14 17:39             ` Kevin Bracey
2013-03-13  2:05   ` [PATCH v3 1/3] mergetools/p4merge: swap LOCAL and REMOTE David Aguilar
2013-03-24 11:54   ` [PATCH v4 1/2] " Kevin Bracey
2013-03-24 11:54     ` [PATCH v4 2/2] mergetools/p4merge: create a base if none available Kevin Bracey
2013-03-25 17:47       ` Junio C Hamano

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=513830AD.10302@bracey.fi \
    --to=kevin@bracey.fi \
    --cc=ciaranj@gmail.com \
    --cc=davvid@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=schacon@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).