| 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
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
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.
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?
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
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
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
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