Patchwork Fix meld.args in mergetools.rc: add -o $output

login
register
mail settings
Submitter Völker Ronny
Date Feb. 21, 2013, 2:04 p.m.
Message ID <679C86927552B447B2A1522BFC65B7E323F9B223@srv-cob-vm-0059.elaxy.org>
Download mbox | patch
Permalink /patch/1040/
State Superseded
Commit 8048c519dc6a5f12d10cfdff155c6d8aac295b45
Headers show

Comments

Völker Ronny - Feb. 21, 2013, 2:04 p.m.
# HG changeset patch
# User Ronny Voelker <ronny.voelker@gmail.com>
# Date 1361454565 -3600
# Node ID 138968bb44d36a06393ecb82c74490914f7598bc
# Parent  4921b5c2aeed8a6bb0918503f7802508538f01e5
Fix meld.args in mergetools.rc: add -o $output

Without this argument meld saves the result of a merge in $base.
This is a temporary file, which is ignored by Mercurial.
So the result of the merge is lost.
Angel Ezquerra - Feb. 21, 2013, 6:10 p.m.
On Thu, Feb 21, 2013 at 3:04 PM, Völker Ronny <ronny.voelker@elaxy.de> wrote:
> # HG changeset patch
>
> # User Ronny Voelker <ronny.voelker@gmail.com>
>
> # Date 1361454565 -3600
>
> # Node ID 138968bb44d36a06393ecb82c74490914f7598bc
>
> # Parent  4921b5c2aeed8a6bb0918503f7802508538f01e5
>
> Fix meld.args in mergetools.rc: add -o $output
>
>
>
> Without this argument meld saves the result of a merge in $base.
>
> This is a temporary file, which is ignored by Mercurial.
>
> So the result of the merge is lost.
>
>
>
> diff -r 4921b5c2aeed -r 138968bb44d3 contrib/mergetools.hgrc
>
> --- a/contrib/mergetools.hgrc   Sun Feb 17 14:34:53 2013 -0600
>
> +++ b/contrib/mergetools.hgrc               Thu Feb 21 14:49:25 2013 +0100
>
> @@ -25,7 +25,7 @@
>
> gpyfm.gui=True
>
>  meld.gui=True
>
> -meld.args=--label='local' $local --label='base' $base --label='other'
> $other
>
> +meld.args=--label='local' $local --label='base' $base --label='other'
> $other -o $output
>
> meld.diffargs=-a --label='$plabel1' $parent --label='$clabel' $child
>
>  tkdiff.args=$local $other -a $base -o $output

I just tried using meld (on windows) with TortoiseHg, using the
current meld configuration (i.e. without your patch). As far as I can
tell it seems to work fine. The only slightly weird thing is that you
must make (and save) your changes on the "local" panel.

So is this really necessary? Probably I'm missing something...

What I really don't like is the fact that the order of the diff panels
that meld shows is:

local - base - other

This makes it _very_ hard to tell the difference between local and
other. It would be nice if it could be turned around (i.e.
base-local-other). Or better yet, it would be nice if Meld could do a
proper "3-way merge" (with 4 panels) as KDiff3 does.

Cheers,

Angel
Steve Borho - Feb. 21, 2013, 6:28 p.m.
On Thu, Feb 21, 2013 at 12:10 PM, Angel Ezquerra
<angel.ezquerra@gmail.com>wrote:

> On Thu, Feb 21, 2013 at 3:04 PM, Völker Ronny <ronny.voelker@elaxy.de>
> wrote:
> > # HG changeset patch
> >
> > # User Ronny Voelker <ronny.voelker@gmail.com>
> >
> > # Date 1361454565 -3600
> >
> > # Node ID 138968bb44d36a06393ecb82c74490914f7598bc
> >
> > # Parent  4921b5c2aeed8a6bb0918503f7802508538f01e5
> >
> > Fix meld.args in mergetools.rc: add -o $output
> >
> >
> >
> > Without this argument meld saves the result of a merge in $base.
> >
> > This is a temporary file, which is ignored by Mercurial.
> >
> > So the result of the merge is lost.
> >
> >
> >
> > diff -r 4921b5c2aeed -r 138968bb44d3 contrib/mergetools.hgrc
> >
> > --- a/contrib/mergetools.hgrc   Sun Feb 17 14:34:53 2013 -0600
> >
> > +++ b/contrib/mergetools.hgrc               Thu Feb 21 14:49:25 2013
> +0100
> >
> > @@ -25,7 +25,7 @@
> >
> > gpyfm.gui=True
> >
> >  meld.gui=True
> >
> > -meld.args=--label='local' $local --label='base' $base --label='other'
> > $other
> >
> > +meld.args=--label='local' $local --label='base' $base --label='other'
> > $other -o $output
> >
> > meld.diffargs=-a --label='$plabel1' $parent --label='$clabel' $child
> >
> >  tkdiff.args=$local $other -a $base -o $output
>
> I just tried using meld (on windows) with TortoiseHg, using the
> current meld configuration (i.e. without your patch). As far as I can
> tell it seems to work fine. The only slightly weird thing is that you
> must make (and save) your changes on the "local" panel.
>
> So is this really necessary? Probably I'm missing something...
>
> What I really don't like is the fact that the order of the diff panels
> that meld shows is:
>
> local - base - other
>
> This makes it _very_ hard to tell the difference between local and
> other. It would be nice if it could be turned around (i.e.
> base-local-other). Or better yet, it would be nice if Meld could do a
> proper "3-way merge" (with 4 panels) as KDiff3 does.
>

The Meld model is a lot closer to Araxis Merge than to KDiff3.

The way I would prefer for it to work would be for the center pane to be
named "merged" and for the trivial changes from both local and other to be
pre-applied to the merge pane. As it is you have to do the trivial merges
manually.
Pierre-Yves David - March 21, 2013, 10:53 a.m.
On Thu, Feb 21, 2013 at 02:04:43PM +0000, Völker Ronny wrote:
> # HG changeset patch
> # User Ronny Voelker <ronny.voelker@gmail.com>
> # Date 1361454565 -3600
> # Node ID 138968bb44d36a06393ecb82c74490914f7598bc
> # Parent  4921b5c2aeed8a6bb0918503f7802508538f01e5
> Fix meld.args in mergetools.rc: add -o $output
> 
> Without this argument meld saves the result of a merge in $base.
> This is a temporary file, which is ignored by Mercurial.
> So the result of the merge is lost.
> 
> diff -r 4921b5c2aeed -r 138968bb44d3 contrib/mergetools.hgrc
> --- a/contrib/mergetools.hgrc   Sun Feb 17 14:34:53 2013 -0600
> +++ b/contrib/mergetools.hgrc               Thu Feb 21 14:49:25 2013 +0100
> @@ -25,7 +25,7 @@
> gpyfm.gui=True
>  meld.gui=True
> -meld.args=--label='local' $local --label='base' $base --label='other' $other
> +meld.args=--label='local' $local --label='base' $base --label='other' $other -o $output
> meld.diffargs=-a --label='$plabel1' $parent --label='$clabel' $child
>  tkdiff.args=$local $other -a $base -o $output

What the status of this patch (and of the merge config in general) I could not
find out from the thread.

Merge is a critical operation in DVCS. It absolutly necessary that merge
operation are clear to the end user. I seems like the "output" panel (where
change will be kept) is unclear with the current configuration. Can we fix
that?
Angel Ezquerra - March 21, 2013, 11:23 a.m.
On Thu, Mar 21, 2013 at 11:53 AM, Pierre-Yves David
<pierre-yves.david@logilab.fr> wrote:
> On Thu, Feb 21, 2013 at 02:04:43PM +0000, Völker Ronny wrote:
>> # HG changeset patch
>> # User Ronny Voelker <ronny.voelker@gmail.com>
>> # Date 1361454565 -3600
>> # Node ID 138968bb44d36a06393ecb82c74490914f7598bc
>> # Parent  4921b5c2aeed8a6bb0918503f7802508538f01e5
>> Fix meld.args in mergetools.rc: add -o $output
>>
>> Without this argument meld saves the result of a merge in $base.
>> This is a temporary file, which is ignored by Mercurial.
>> So the result of the merge is lost.
>>
>> diff -r 4921b5c2aeed -r 138968bb44d3 contrib/mergetools.hgrc
>> --- a/contrib/mergetools.hgrc   Sun Feb 17 14:34:53 2013 -0600
>> +++ b/contrib/mergetools.hgrc               Thu Feb 21 14:49:25 2013 +0100
>> @@ -25,7 +25,7 @@
>> gpyfm.gui=True
>>  meld.gui=True
>> -meld.args=--label='local' $local --label='base' $base --label='other' $other
>> +meld.args=--label='local' $local --label='base' $base --label='other' $other -o $output
>> meld.diffargs=-a --label='$plabel1' $parent --label='$clabel' $child
>>  tkdiff.args=$local $other -a $base -o $output
>
> What the status of this patch (and of the merge config in general) I could not
> find out from the thread.
>
> Merge is a critical operation in DVCS. It absolutly necessary that merge
> operation are clear to the end user. I seems like the "output" panel (where
> change will be kept) is unclear with the current configuration. Can we fix
> that?

I am actually not quite sure of the state of this patch either. Volker
proposed some changes, I discussed them on the meld mailing list and I
got some advice which is not exactly what Volker proposed. I think we
should follow the meld devs advice, but I did not want to send a patch
stepping over Volker's patch.

Cheers,

Angel
Völker Ronny - March 21, 2013, 12:21 p.m.
Pierre-Yves David wrote:
> On Thu, Feb 21, 2013 at 02:04:43PM +0000, Völker Ronny wrote:
> > # HG changeset patch
> > # User Ronny Voelker <ronny.voelker@gmail.com> # Date 1361454565 -
> 3600
> > # Node ID 138968bb44d36a06393ecb82c74490914f7598bc
> > # Parent  4921b5c2aeed8a6bb0918503f7802508538f01e5
> > Fix meld.args in mergetools.rc: add -o $output
> >
> > Without this argument meld saves the result of a merge in $base.
> > This is a temporary file, which is ignored by Mercurial.
> > So the result of the merge is lost.
> >
> > diff -r 4921b5c2aeed -r 138968bb44d3 contrib/mergetools.hgrc
> > --- a/contrib/mergetools.hgrc   Sun Feb 17 14:34:53 2013 -0600
> > +++ b/contrib/mergetools.hgrc               Thu Feb 21 14:49:25 2013 +0100
> > @@ -25,7 +25,7 @@
> > gpyfm.gui=True
> >  meld.gui=True
> > -meld.args=--label='local' $local --label='base' $base --label='other'
> > $other
> > +meld.args=--label='local' $local --label='base' $base --label='other'
> > +$other -o $output
> > meld.diffargs=-a --label='$plabel1' $parent --label='$clabel' $child
> > tkdiff.args=$local $other -a $base -o $output
> 
> What the status of this patch (and of the merge config in general) I could not
> find out from the thread.
> 
> Merge is a critical operation in DVCS. It absolutly necessary that merge
> operation are clear to the end user. I seems like the "output" panel (where
> change will be kept) is unclear with the current configuration. Can we fix
> that?
> 
> 
> 
> --
> Pierre-Yves David
> 
> http://www.logilab.fr/

There were three separate issues discussed in this Thread:

1. The current configuration for meld is not clear and orders the panels in a way, which is different to that what Meld (and other merge tools of the same kind) seems to prefer (merge from left and right into the center panel).
This is the sole issue my patch should solve, because with the current configuration I lost code changes after rebasing.
The panel names are copied from the configuration for Araxis Merge (which has the same merging model).
Still, one can argue that from the labeling of the panels it's not obvious which is the target panel.
Angel proposed to rename the center panel from 'base' to 'merged', which would be ok for me.

2. In the discussion was raised the issue, that meld isn't configured to automatically merge on startup.
The latest version of Meld has a command line option to merge on startup, but older versions are lacking that option.
As a workaround the Meld-dev proposed a configuration, where mercurials automerge is used, with the drawback, that the information
about the base version is lost.  I think the base version is valuable and we should go without automerge on startup for now.
 Manually triggering the automatic merging after startup is only one click away.

3. While skimming over the configuration of the other merge tools, I found that the naming of the panels is not consistent across merge tools with the same merging model.
E.g. for some the center panel is 'base', for some it is 'merged'. 
I would expect that, when I use different merge tools of the same kind, the panels are named the same.
But this is too a minor issue for me.


Ronny
Angel Ezquerra - March 21, 2013, 1:05 p.m.
On Thu, Mar 21, 2013 at 1:21 PM, Völker Ronny <ronny.voelker@elaxy.de> wrote:
> Pierre-Yves David wrote:
>> On Thu, Feb 21, 2013 at 02:04:43PM +0000, Völker Ronny wrote:
>> > # HG changeset patch
>> > # User Ronny Voelker <ronny.voelker@gmail.com> # Date 1361454565 -
>> 3600
>> > # Node ID 138968bb44d36a06393ecb82c74490914f7598bc
>> > # Parent  4921b5c2aeed8a6bb0918503f7802508538f01e5
>> > Fix meld.args in mergetools.rc: add -o $output
>> >
>> > Without this argument meld saves the result of a merge in $base.
>> > This is a temporary file, which is ignored by Mercurial.
>> > So the result of the merge is lost.
>> >
>> > diff -r 4921b5c2aeed -r 138968bb44d3 contrib/mergetools.hgrc
>> > --- a/contrib/mergetools.hgrc   Sun Feb 17 14:34:53 2013 -0600
>> > +++ b/contrib/mergetools.hgrc               Thu Feb 21 14:49:25 2013 +0100
>> > @@ -25,7 +25,7 @@
>> > gpyfm.gui=True
>> >  meld.gui=True
>> > -meld.args=--label='local' $local --label='base' $base --label='other'
>> > $other
>> > +meld.args=--label='local' $local --label='base' $base --label='other'
>> > +$other -o $output
>> > meld.diffargs=-a --label='$plabel1' $parent --label='$clabel' $child
>> > tkdiff.args=$local $other -a $base -o $output
>>
>> What the status of this patch (and of the merge config in general) I could not
>> find out from the thread.
>>
>> Merge is a critical operation in DVCS. It absolutly necessary that merge
>> operation are clear to the end user. I seems like the "output" panel (where
>> change will be kept) is unclear with the current configuration. Can we fix
>> that?
>>
>>
>>
>> --
>> Pierre-Yves David
>>
>> http://www.logilab.fr/
>
> There were three separate issues discussed in this Thread:
>
> 1. The current configuration for meld is not clear and orders the panels in a way, which is different to that what Meld (and other merge tools of the same kind) seems to prefer (merge from left and right into the center panel).
> This is the sole issue my patch should solve, because with the current configuration I lost code changes after rebasing.
> The panel names are copied from the configuration for Araxis Merge (which has the same merging model).
> Still, one can argue that from the labeling of the panels it's not obvious which is the target panel.
> Angel proposed to rename the center panel from 'base' to 'merged', which would be ok for me.
>
> 2. In the discussion was raised the issue, that meld isn't configured to automatically merge on startup.
> The latest version of Meld has a command line option to merge on startup, but older versions are lacking that option.
> As a workaround the Meld-dev proposed a configuration, where mercurials automerge is used, with the drawback, that the information
> about the base version is lost.  I think the base version is valuable and we should go without automerge on startup for now.
>  Manually triggering the automatic merging after startup is only one click away.
>
> 3. While skimming over the configuration of the other merge tools, I found that the naming of the panels is not consistent across merge tools with the same merging model.
> E.g. for some the center panel is 'base', for some it is 'merged'.
> I would expect that, when I use different merge tools of the same kind, the panels are named the same.
> But this is too a minor issue for me.
>
>
> Ronny

Personally I think that we should do #2, i.e. we should configure
mercurial to do a pre-merge for us. I think it is weird to start a
merge tool and have to manually do the merge. I expect the non
conflicts to be solved for me, so that I can focus on the actual
conflicts. Since the merge output will be at the center it will be
easy to tell what changed from one revision to the other.

It would be nice to also have the base file, but if I had to choose
between having the base or having the merge I'd rather have the merge.
If I need both I can always use kdiff3.

Also, I think doing #1 (renaming 'base' to 'merge') without doing #2
(i.e. actually merging) would be confusing. The user may think that
what he sees in the merge panel is the actual merge result, which
would not be true.

Any other opinions?

Cheers,

Angel
Pierre-Yves David - March 21, 2013, 2:14 p.m.
On Thu, Mar 21, 2013 at 12:21:57PM +0000, Völker Ronny wrote:
> 
> There were three separate issues discussed in this Thread:

Awesome! thanks a lot for writing this summary version

> 1. The current configuration for meld is not clear and orders the panels in a way, which is different to that what Meld (and other merge tools of the same kind) seems to prefer (merge from left and right into the center panel).
> This is the sole issue my patch should solve, because with the current configuration I lost code changes after rebasing.
> The panel names are copied from the configuration for Araxis Merge (which has the same merging model).
> Still, one can argue that from the labeling of the panels it's not obvious which is the target panel.
> Angel proposed to rename the center panel from 'base' to 'merged', which would be ok for me.

We need to make it clear were the result of the merge is. This should be a top priority patches.

> 2. In the discussion was raised the issue, that meld isn't configured to automatically merge on startup.
> The latest version of Meld has a command line option to merge on startup, but older versions are lacking that option.
> As a workaround the Meld-dev proposed a configuration, where mercurials automerge is used, with the drawback, that the information
> about the base version is lost.  I think the base version is valuable and we should go without automerge on startup for now.
>  Manually triggering the automatic merging after startup is only one click away.

We do not automatically install the sample configuration tools. Its is packager
jobs to do so. We just need to highlight this change and they take care of
removing automerge if the distribution have a too old version of meld. Anyway
It will probably not be too much touble, the (almost) latest Debian stable have
the latest meld.

> 3. While skimming over the configuration of the other merge tools, I found
> that the naming of the panels is not consistent across merge tools with the
> same merging model.
> E.g. for some the center panel is 'base', for some it is 'merged'. 
> I would expect that, when I use different merge tools of the same kind, the panels are named the same.
> But this is too a minor issue for me.

We should probably unify that. Nice catch.

Patch

diff -r 4921b5c2aeed -r 138968bb44d3 contrib/mergetools.hgrc
--- a/contrib/mergetools.hgrc   Sun Feb 17 14:34:53 2013 -0600
+++ b/contrib/mergetools.hgrc               Thu Feb 21 14:49:25 2013 +0100
@@ -25,7 +25,7 @@ 
gpyfm.gui=True
 meld.gui=True
-meld.args=--label='local' $local --label='base' $base --label='other' $other
+meld.args=--label='local' $local --label='base' $base --label='other' $other -o $output
meld.diffargs=-a --label='$plabel1' $parent --label='$clabel' $child
 tkdiff.args=$local $other -a $base -o $output