Patchwork [V2] filemerge: add non-interactive internal:merge-local and internal:merge-other

login
register
mail settings
Submitter Jordi Gutiérrez Hermoso
Date Aug. 13, 2014, 7 p.m.
Message ID <e125a4ec265f9207667e.1407956428@Iris>
Download mbox | patch
Permalink /patch/5369/
State Changes Requested
Headers show

Comments

Jordi Gutiérrez Hermoso - Aug. 13, 2014, 7 p.m.
# HG changeset patch
# User Jordi Gutiérrez Hermoso <jordigh@octave.org>
# Date 1407783206 14400
#      Mon Aug 11 14:53:26 2014 -0400
# Node ID e125a4ec265f9207667e82a62754dfd96f17c0a4
# Parent  fff8e1cec90f64b356c66aa068d3810f7efea0ac
filemerge: add non-interactive internal:merge-local and internal:merge-other

There are two non-interactive internal merge tools, internal:other and
internal:local, but they don't really merge, they just pick all
changes from the local or other version of the file. In some
situations, it is known that we want a merge and also know that all
merge conflicts should be resolved in one direction. Although external
merge tools can do this, sometimes it can be convenient to do so from
within hg, without invoking a merge tool. These new
internal:merge-local and internal:merge-other tools can do just that.
Pierre-Yves David - Aug. 18, 2014, 10:22 p.m.
On 08/13/2014 12:00 PM, Jordi Gutiérrez Hermoso wrote:
> # HG changeset patch
> # User Jordi Gutiérrez Hermoso <jordigh@octave.org>
> # Date 1407783206 14400
> #      Mon Aug 11 14:53:26 2014 -0400
> # Node ID e125a4ec265f9207667e82a62754dfd96f17c0a4
> # Parent  fff8e1cec90f64b356c66aa068d3810f7efea0ac
> filemerge: add non-interactive internal:merge-local and internal:merge-other
>
> There are two non-interactive internal merge tools, internal:other and
> internal:local, but they don't really merge, they just pick all
> changes from the local or other version of the file. In some
> situations, it is known that we want a merge and also know that all
> merge conflicts should be resolved in one direction. Although external
> merge tools can do this, sometimes it can be convenient to do so from
> within hg, without invoking a merge tool. These new
> internal:merge-local and internal:merge-other tools can do just that.

While I kindof like the idea and could see myself use it from time to 
time, I'm a bit worried about the name of this pair and the 
proliferation of this kind of variant in general.

We already have:

  - internal:other
  - internal:local

It is not clear how user will be able to distinct between your two new 
additions

  - internal:merge-other
  - internal:merge-local

Also, I can see a third type of tool that will take the content of side 
X even when there was no manifest level conflict.

I think we need to think harder about a proper UI to hand all those case.

>
> diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py
> --- a/mercurial/filemerge.py
> +++ b/mercurial/filemerge.py
> @@ -229,6 +229,40 @@ def _imerge(repo, mynode, orig, fcd, fco
>           return True, r
>       return False, 0
>
> +def _imergeauto(repo, mynode, orig, fcd, fco, fca, toolconf, files,
> +                labels=None, localorother=None):
> +    """
> +    Generic driver for _imergelocal and _imergeother
> +    """
> +    assert localorother is not None
> +    tool, toolpath, binary, symlink = toolconf
> +    if symlink:
> +        repo.ui.warn(_('warning: internal:merge-%s cannot merge symlinks '
> +                       'for %s\n') % (localorother, fcd.path()))
> +        return False, 1
> +    a, b, c, back = files
> +    r = simplemerge.simplemerge(repo.ui, a, b, c, label=labels,
> +                                localorother=localorother)
> +    return True, r
> +
> +@internaltool('merge-local', True)
> +def _imergelocal(*args, **kwargs):
> +    """
> +    Like internal:merge, but resolve all conflicts non-interactively
> +    in favor of the local changes.
> +    """
> +    success, status = _imergeauto(localorother='local', *args, **kwargs)
> +    return success, status
> +
> +@internaltool('merge-other', True)
> +def _imergelocal(*args, **kwargs):

You forgot to update the function name.

> +    """
> +    Like internal:merge, but resolve all conflicts non-interactively
> +    in favor of the other changes.
> +    """
> +    success, status = _imergeauto(localorother='other', *args, **kwargs)
> +    return success, status
> +
>   @internaltool('merge3', True,
>                 _("merging %s incomplete! "
>                   "(edit conflicts, then use 'hg resolve --mark')\n"))
> diff --git a/mercurial/simplemerge.py b/mercurial/simplemerge.py
> --- a/mercurial/simplemerge.py
> +++ b/mercurial/simplemerge.py
> @@ -82,7 +82,8 @@ class Merge3Text(object):
>                       start_marker='<<<<<<<',
>                       mid_marker='=======',
>                       end_marker='>>>>>>>',
> -                    base_marker=None):
> +                    base_marker=None,
> +                    localorother=None):
>           """Return merge in cvs-like form.

Slicing the change to simple merge in a dedicated commit would make it 
simpler to review.

>           """
>           self.conflicts = False
> @@ -111,18 +112,25 @@ class Merge3Text(object):
>                   for i in range(t[1], t[2]):
>                       yield self.b[i]
>               elif what == 'conflict':
> -                self.conflicts = True
> -                yield start_marker + newline
> -                for i in range(t[3], t[4]):
> -                    yield self.a[i]
> -                if base_marker is not None:
> -                    yield base_marker + newline
> -                    for i in range(t[1], t[2]):
> -                        yield self.base[i]
> -                yield mid_marker + newline
> -                for i in range(t[5], t[6]):
> -                    yield self.b[i]
> -                yield end_marker + newline
> +                if localorother == 'local':
> +                    for i in range(t[3], t[4]):
> +                        yield self.a[i]
> +                elif localorother == 'other':
> +                    for i in range(t[5], t[6]):
> +                        yield self.b[i]
> +                else:
> +                    self.conflicts = True
> +                    yield start_marker + newline
> +                    for i in range(t[3], t[4]):
> +                        yield self.a[i]
> +                    if base_marker is not None:
> +                        yield base_marker + newline
> +                        for i in range(t[1], t[2]):
> +                            yield self.base[i]
> +                    yield mid_marker + newline
> +                    for i in range(t[5], t[6]):
> +                        yield self.b[i]
> +                    yield end_marker + newline
>               else:
>                   raise ValueError(what)
>
> @@ -373,7 +381,7 @@ def simplemerge(ui, local, base, other,
>           out = sys.stdout
>
>       m3 = Merge3Text(basetext, localtext, othertext)
> -    extrakwargs = {}
> +    extrakwargs = {"localorother": opts.get("localorother", None)}
>       if name_base is not None:
>           extrakwargs['base_marker'] = '|||||||'
>           extrakwargs['name_base'] = name_base
> diff --git a/tests/test-conflict.t b/tests/test-conflict.t
> --- a/tests/test-conflict.t
> +++ b/tests/test-conflict.t
> @@ -232,3 +232,66 @@ internal:merge3
>     5
>     >>>>>>> other
>     Hop we are done.
> +
> +
> +Add some unconflicting changes on each head, to make sure we really
> +are merging, unlike internal:local and internal:other
> +
> +  $ hg up -C
> +  2 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +  $ printf "\n\nEnd of file\n" >> a
> +  $ hg ci -m "Add some stuff at the end"
> +  $ hg up -r 1
> +  1 files updated, 0 files merged, 1 files removed, 0 files unresolved
> +  $ printf "Start of file\n\n\n" > tmp
> +  $ cat a >> tmp
> +  $ mv tmp a
> +  $ hg ci -m "Add some stuff at the beginning"
> +
> +Now test internal:merge-other and internal:merge-local
> +
> +  $ hg merge
> +  merging a
> +  warning: conflicts during merge.
> +  merging a incomplete! (edit conflicts, then use 'hg resolve --mark')
> +  1 files updated, 0 files merged, 0 files removed, 1 files unresolved
> +  use 'hg resolve' to retry unresolved file merges or 'hg update -C .' to abandon
> +  [1]
> +  $ hg resolve --tool internal:merge-other a
> +  merging a
> +  (no more unresolved files)
> +  $ cat a
> +  Start of file
> +
> +
> +  Small Mathematical Series.
> +  1
> +  2
> +  3
> +  6
> +  8
> +  Hop we are done.
> +
> +
> +  End of file
> +
> +  $ hg up -C
> +  1 files updated, 0 files merged, 1 files removed, 0 files unresolved
> +  $ hg merge --tool internal:merge-local
> +  merging a
> +  2 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +  (branch merge, don't forget to commit)
> +  $ cat a
> +  Start of file
> +
> +
> +  Small Mathematical Series.
> +  1
> +  2
> +  300:12 < stantonk> so how would roots ever return anything?

> +  4
> +  5
> +  Hop we are done.
> +
> +
> +  End of file
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>
Jordi Gutiérrez Hermoso - Aug. 20, 2014, 8:02 p.m.
On Mon, 2014-08-18 at 15:22 -0700, Pierre-Yves David wrote:
> We already have:
> 
>   - internal:other
>   - internal:local
> 
> It is not clear how user will be able to distinct between your two new 
> additions
> 
>   - internal:merge-other
>   - internal:merge-local

Well, they show up with brief explanations in "hg help merge-tools".
How could it be made clearer? I think the problem starts that
internal:other and internal:local are kind of confusing. A lot of
people, myself included, seem to think upon first meeting them that
they merge but resolve conflicts one way.

> Also, I can see a third type of tool that will take the content of side 
> X even when there was no manifest level conflict.

You mean, when there is no merging to be done? Then no merge tool
would get called for it.

> I think we need to think harder about a proper UI to hand all those
> case.

I can't think of such a UI that wouldn't break existing things. What
do you have in mind?

> > +@internaltool('merge-other', True)
> > +def _imergelocal(*args, **kwargs):
> 
> You forgot to update the function name.

Oh, wow. I was trying to figure out how this passed tests. I suppose
because the internaltool decorator closes over the function it's
wrapping when it puts it in the function list.

> Slicing the change to simple merge in a dedicated commit would make
> it simpler to review.

Okay, so two commits, one for simplemerge, one for the rest? I can do
that.

- Jordi G. H.

Patch

diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py
--- a/mercurial/filemerge.py
+++ b/mercurial/filemerge.py
@@ -229,6 +229,40 @@  def _imerge(repo, mynode, orig, fcd, fco
         return True, r
     return False, 0
 
+def _imergeauto(repo, mynode, orig, fcd, fco, fca, toolconf, files,
+                labels=None, localorother=None):
+    """
+    Generic driver for _imergelocal and _imergeother
+    """
+    assert localorother is not None
+    tool, toolpath, binary, symlink = toolconf
+    if symlink:
+        repo.ui.warn(_('warning: internal:merge-%s cannot merge symlinks '
+                       'for %s\n') % (localorother, fcd.path()))
+        return False, 1
+    a, b, c, back = files
+    r = simplemerge.simplemerge(repo.ui, a, b, c, label=labels,
+                                localorother=localorother)
+    return True, r
+
+@internaltool('merge-local', True)
+def _imergelocal(*args, **kwargs):
+    """
+    Like internal:merge, but resolve all conflicts non-interactively
+    in favor of the local changes.
+    """
+    success, status = _imergeauto(localorother='local', *args, **kwargs)
+    return success, status
+
+@internaltool('merge-other', True)
+def _imergelocal(*args, **kwargs):
+    """
+    Like internal:merge, but resolve all conflicts non-interactively
+    in favor of the other changes.
+    """
+    success, status = _imergeauto(localorother='other', *args, **kwargs)
+    return success, status
+
 @internaltool('merge3', True,
               _("merging %s incomplete! "
                 "(edit conflicts, then use 'hg resolve --mark')\n"))
diff --git a/mercurial/simplemerge.py b/mercurial/simplemerge.py
--- a/mercurial/simplemerge.py
+++ b/mercurial/simplemerge.py
@@ -82,7 +82,8 @@  class Merge3Text(object):
                     start_marker='<<<<<<<',
                     mid_marker='=======',
                     end_marker='>>>>>>>',
-                    base_marker=None):
+                    base_marker=None,
+                    localorother=None):
         """Return merge in cvs-like form.
         """
         self.conflicts = False
@@ -111,18 +112,25 @@  class Merge3Text(object):
                 for i in range(t[1], t[2]):
                     yield self.b[i]
             elif what == 'conflict':
-                self.conflicts = True
-                yield start_marker + newline
-                for i in range(t[3], t[4]):
-                    yield self.a[i]
-                if base_marker is not None:
-                    yield base_marker + newline
-                    for i in range(t[1], t[2]):
-                        yield self.base[i]
-                yield mid_marker + newline
-                for i in range(t[5], t[6]):
-                    yield self.b[i]
-                yield end_marker + newline
+                if localorother == 'local':
+                    for i in range(t[3], t[4]):
+                        yield self.a[i]
+                elif localorother == 'other':
+                    for i in range(t[5], t[6]):
+                        yield self.b[i]
+                else:
+                    self.conflicts = True
+                    yield start_marker + newline
+                    for i in range(t[3], t[4]):
+                        yield self.a[i]
+                    if base_marker is not None:
+                        yield base_marker + newline
+                        for i in range(t[1], t[2]):
+                            yield self.base[i]
+                    yield mid_marker + newline
+                    for i in range(t[5], t[6]):
+                        yield self.b[i]
+                    yield end_marker + newline
             else:
                 raise ValueError(what)
 
@@ -373,7 +381,7 @@  def simplemerge(ui, local, base, other, 
         out = sys.stdout
 
     m3 = Merge3Text(basetext, localtext, othertext)
-    extrakwargs = {}
+    extrakwargs = {"localorother": opts.get("localorother", None)}
     if name_base is not None:
         extrakwargs['base_marker'] = '|||||||'
         extrakwargs['name_base'] = name_base
diff --git a/tests/test-conflict.t b/tests/test-conflict.t
--- a/tests/test-conflict.t
+++ b/tests/test-conflict.t
@@ -232,3 +232,66 @@  internal:merge3
   5
   >>>>>>> other
   Hop we are done.
+
+
+Add some unconflicting changes on each head, to make sure we really
+are merging, unlike internal:local and internal:other
+
+  $ hg up -C
+  2 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  $ printf "\n\nEnd of file\n" >> a
+  $ hg ci -m "Add some stuff at the end"
+  $ hg up -r 1
+  1 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  $ printf "Start of file\n\n\n" > tmp
+  $ cat a >> tmp
+  $ mv tmp a
+  $ hg ci -m "Add some stuff at the beginning"
+
+Now test internal:merge-other and internal:merge-local
+
+  $ hg merge
+  merging a
+  warning: conflicts during merge.
+  merging a incomplete! (edit conflicts, then use 'hg resolve --mark')
+  1 files updated, 0 files merged, 0 files removed, 1 files unresolved
+  use 'hg resolve' to retry unresolved file merges or 'hg update -C .' to abandon
+  [1]
+  $ hg resolve --tool internal:merge-other a
+  merging a
+  (no more unresolved files)
+  $ cat a
+  Start of file
+  
+  
+  Small Mathematical Series.
+  1
+  2
+  3
+  6
+  8
+  Hop we are done.
+  
+  
+  End of file
+
+  $ hg up -C
+  1 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  $ hg merge --tool internal:merge-local
+  merging a
+  2 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  (branch merge, don't forget to commit)
+  $ cat a
+  Start of file
+  
+  
+  Small Mathematical Series.
+  1
+  2
+  3
+  4
+  5
+  Hop we are done.
+  
+  
+  End of file