Re: [PATCH v3 1/3] mergetools/p4merge: swap LOCAL and REMOTE
- From
David Aguilar <davvid@gmail.com>
- Date
- Mar 13, 2013, 02:05 UTC
- Message-ID
- <CAJDDKr7NJsmB3R_kYtZeocSZAz-kfP9k6GssZ+AM-qfPCTzrdg@mail.gmail.com>
- In-Reply-To
- <1363137142-18606-1-git-send-email-kevin@bracey.fi>
On Tue, Mar 12, 2013 at 6:12 PM, Kevin Bracey <kevin@bracey.fi> wrote:
Show 33 quoted lines
> 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. > > This change makes use of P4Merge consistent with its built-in help, its > reference documentation, and Perforce itself. But most importantly, it > makes merge results clearer. P4Merge is not totally symmetrical between > left and right; despite changing a few text labels from "theirs/ours" to > "left/right" when invoked manually, it still retains its original > Perforce "theirs/ours" viewpoint. > > Most obviously, in the result pane P4Merge shows changes that are common > to both branches in green. This is on the basis of the current branch > being green, as it is when invoked from Perforce; it means that lines in > the result are blue if and only if they are being changed by the merge, > making the resulting diff clearer. > > Note that P4Merge now shows "ours" on the right for both diff and merge, > unlike other diff/mergetools, which always have REMOTE on the right. > But observe that REMOTE is the working tree (ie "ours") for a diff, > while it's another branch (ie "theirs") for a merge. > > Ours and theirs are reversed for a rebase - see "git help rebase". > However, this does produce the desired "show the results of this commit" > effect in P4Merge - changes that remain in the rebased commit (in your > branch, but not in the new base) appear in blue; changes that do not > appear in the rebased commit (from the new base, or common to both) are > in green. If Perforce had rebase, they'd probably not swap ours/theirs, > but make P4Merge show common changes in blue, picking out our changes in > green. We can't do that, so this is next best. > > Signed-off-by: Kevin Bracey <kevin@bracey.fi> > ---
This seems sensible to apply. The commit message is a bit long, but I think it's justified since this is exactly the kind of thing I would tend to forget after enough time has passed.
Ditto on the create_virtual_base patch. Your latest patch addressed Junio's note about making it take 2 args.
FWIW, please feel free to add:
Reviewed-by: David Aguilar <davvid@gmail.com>
Thanks.
Show 19 quoted lines
> mergetools/p4merge | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mergetools/p4merge b/mergetools/p4merge
> index 8a36916..46b3a5a 100644
> --- a/mergetools/p4merge
> +++ b/mergetools/p4merge
> @@ -22,7 +22,7 @@ diff_cmd () {
> merge_cmd () {
> touch "$BACKUP"
> $base_present || >"$BASE"
> - "$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" "$MERGED"
> + "$merge_tool_path" "$BASE" "$REMOTE" "$LOCAL" "$MERGED"
> check_unchanged
> }
>
> --
> 1.8.2.rc3.7.g1100d09.dirty
>-- David