Patchwork [1,of,6,V2] simplemerge: burn "minimal" feature to the ground

login
register
mail settings
Submitter Pierre-Yves David
Date Aug. 5, 2014, 11:30 p.m.
Message ID <3630e859cb13ace36f31.1407281441@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/5269/
State Accepted
Headers show

Comments

Pierre-Yves David - Aug. 5, 2014, 11:30 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1407275785 25200
#      Tue Aug 05 14:56:25 2014 -0700
# Node ID 3630e859cb13ace36f3171142732dc8718d62979
# Parent  2ea6d906cf9b7b7338594bff33dffb7d6a43384f
simplemerge: burn "minimal" feature to the ground

Matt Mackall said:

  The goal of simplemerge should have always been to be a drop-in
  replacement for RCS merge. Please nuke this minimization thing entirely.

This whole things is now dead.

Patch

diff --git a/contrib/simplemerge b/contrib/simplemerge
--- a/contrib/simplemerge
+++ b/contrib/simplemerge
@@ -9,12 +9,11 @@  from mercurial import simplemerge, fancy
 
 options = [('L', 'label', [], _('labels to use on conflict markers')),
            ('a', 'text', None, _('treat all files as text')),
            ('p', 'print', None,
             _('print results instead of overwriting LOCAL')),
-           ('', 'no-minimal', None,
-            _('do not try to minimize conflict regions')),
+           ('', 'no-minimal', None, _('no effect (DEPRECATED)')),
            ('h', 'help', None, _('display help and exit')),
            ('q', 'quiet', None, _('suppress output'))]
 
 usage = _('''simplemerge [OPTS] LOCAL BASE OTHER
 
diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py
--- a/mercurial/filemerge.py
+++ b/mercurial/filemerge.py
@@ -189,12 +189,11 @@  def _premerge(repo, toolconf, files, lab
             raise error.ConfigError(_("%s.premerge not valid "
                                       "('%s' is neither boolean nor %s)") %
                                     (tool, premerge, _valid))
 
     if premerge:
-        r = simplemerge.simplemerge(ui, a, b, c, quiet=True, label=labels,
-                                    no_minimal=True)
+        r = simplemerge.simplemerge(ui, a, b, c, quiet=True, label=labels)
         if not r:
             ui.debug(" premerge successful\n")
             return 0
         if premerge != 'keep':
             util.copyfile(back, a) # restore from backup and try again
@@ -217,11 +216,11 @@  def _imerge(repo, mynode, orig, fcd, fco
     if r:
         a, b, c, back = files
 
         ui = repo.ui
 
-        r = simplemerge.simplemerge(ui, a, b, c, label=labels, no_minimal=True)
+        r = simplemerge.simplemerge(ui, a, b, c, label=labels)
         return True, r
     return False, 0
 
 @internaltool('tagmerge', True,
               _("automatic tag merging of %s failed! "
diff --git a/mercurial/simplemerge.py b/mercurial/simplemerge.py
--- a/mercurial/simplemerge.py
+++ b/mercurial/simplemerge.py
@@ -80,32 +80,27 @@  class Merge3Text(object):
                     name_b=None,
                     name_base=None,
                     start_marker='<<<<<<<',
                     mid_marker='=======',
                     end_marker='>>>>>>>',
-                    base_marker=None,
-                    reprocess=False):
+                    base_marker=None):
         """Return merge in cvs-like form.
         """
         self.conflicts = False
         newline = '\n'
         if len(self.a) > 0:
             if self.a[0].endswith('\r\n'):
                 newline = '\r\n'
             elif self.a[0].endswith('\r'):
                 newline = '\r'
-        if base_marker and reprocess:
-            raise CantReprocessAndShowBase
         if name_a:
             start_marker = start_marker + ' ' + name_a
         if name_b:
             end_marker = end_marker + ' ' + name_b
         if name_base and base_marker:
             base_marker = base_marker + ' ' + name_base
         merge_regions = self.merge_regions()
-        if reprocess is True:
-            merge_regions = self.reprocess_merge_regions(merge_regions)
         for t in merge_regions:
             what = t[0]
             if what == 'unchanged':
                 for i in range(t[1], t[2]):
                     yield self.base[i]
@@ -276,41 +271,10 @@  class Merge3Text(object):
                 yield 'unchanged', zmatch, zend
                 iz = zend
                 ia = aend
                 ib = bend
 
-    def reprocess_merge_regions(self, merge_regions):
-        """Where there are conflict regions, remove the agreed lines.
-
-        Lines where both A and B have made the same changes are
-        eliminated.
-        """
-        for region in merge_regions:
-            if region[0] != "conflict":
-                yield region
-                continue
-            type, iz, zmatch, ia, amatch, ib, bmatch = region
-            a_region = self.a[ia:amatch]
-            b_region = self.b[ib:bmatch]
-            matches = mdiff.get_matching_blocks(''.join(a_region),
-                                                ''.join(b_region))
-            next_a = ia
-            next_b = ib
-            for region_ia, region_ib, region_len in matches[:-1]:
-                region_ia += ia
-                region_ib += ib
-                reg = self.mismatch_region(next_a, region_ia, next_b,
-                                           region_ib)
-                if reg is not None:
-                    yield reg
-                yield 'same', region_ia, region_len + region_ia
-                next_a = region_ia + region_len
-                next_b = region_ib + region_len
-            reg = self.mismatch_region(next_a, amatch, next_b, bmatch)
-            if reg is not None:
-                yield reg
-
     def mismatch_region(next_a, region_ia,  next_b, region_ib):
         if next_a < region_ia or next_b < region_ib:
             return 'conflict', None, None, next_a, region_ia, next_b, region_ib
     mismatch_region = staticmethod(mismatch_region)
 
@@ -435,15 +399,12 @@  def simplemerge(ui, local, base, other, 
         opener = scmutil.opener(os.path.dirname(local))
         out = opener(os.path.basename(local), "w", atomictemp=True)
     else:
         out = sys.stdout
 
-    reprocess = not opts.get('no_minimal')
-
     m3 = Merge3Text(basetext, localtext, othertext)
-    for line in m3.merge_lines(name_a=name_a, name_b=name_b,
-                               reprocess=reprocess):
+    for line in m3.merge_lines(name_a=name_a, name_b=name_b):
         out.write(line)
 
     if not opts.get('print'):
         out.close()
 
diff --git a/tests/test-contrib.t b/tests/test-contrib.t
--- a/tests/test-contrib.t
+++ b/tests/test-contrib.t
@@ -141,27 +141,15 @@  conflicts
   $ cp base conflict-local
   $ cp other conflict-other
   $ echo not other >> conflict-local
   $ echo end >> conflict-local
   $ echo end >> conflict-other
+
   $ python simplemerge -p conflict-local base conflict-other
   base
   <<<<<<< conflict-local
   not other
-  =======
-  other
-  >>>>>>> conflict-other
-  end
-  warning: conflicts during merge.
-  [1]
-
---no-minimal
-
-  $ python simplemerge -p --no-minimal conflict-local base conflict-other
-  base
-  <<<<<<< conflict-local
-  not other
   end
   =======
   other
   end
   >>>>>>> conflict-other
@@ -172,27 +160,29 @@  1 label
 
   $ python simplemerge -p -L foo conflict-local base conflict-other
   base
   <<<<<<< foo
   not other
+  end
   =======
   other
+  end
   >>>>>>> conflict-other
-  end
   warning: conflicts during merge.
   [1]
 
 2 labels
 
   $ python simplemerge -p -L foo -L bar conflict-local base conflict-other
   base
   <<<<<<< foo
   not other
+  end
   =======
   other
+  end
   >>>>>>> bar
-  end
   warning: conflicts during merge.
   [1]
 
 too many labels
 
@@ -229,11 +219,11 @@  help
   
   options:
    -L --label       labels to use on conflict markers
    -a --text        treat all files as text
    -p --print       print results instead of overwriting LOCAL
-      --no-minimal  do not try to minimize conflict regions
+      --no-minimal  no effect (DEPRECATED)
    -h --help        display help and exit
    -q --quiet       suppress output
 
 wrong number of arguments
 
@@ -249,11 +239,11 @@  wrong number of arguments
   
   options:
    -L --label       labels to use on conflict markers
    -a --text        treat all files as text
    -p --print       print results instead of overwriting LOCAL
-      --no-minimal  do not try to minimize conflict regions
+      --no-minimal  no effect (DEPRECATED)
    -h --help        display help and exit
    -q --quiet       suppress output
   [1]
 
 bad option
@@ -270,9 +260,9 @@  bad option
   
   options:
    -L --label       labels to use on conflict markers
    -a --text        treat all files as text
    -p --print       print results instead of overwriting LOCAL
-      --no-minimal  do not try to minimize conflict regions
+      --no-minimal  no effect (DEPRECATED)
    -h --help        display help and exit
    -q --quiet       suppress output
   [1]
diff --git a/tests/test-simplemerge.py b/tests/test-simplemerge.py
--- a/tests/test-simplemerge.py
+++ b/tests/test-simplemerge.py
@@ -318,70 +318,10 @@  bbb
         ml = list(m3.merge_lines('LAO', 'TAO'))
         self.log('merge result:')
         self.log(''.join(ml))
         self.assertEquals(ml, MERGED_RESULT)
 
-    def test_minimal_conflicts_common(self):
-        """Reprocessing"""
-        base_text = ("a\n" * 20).splitlines(True)
-        this_text = ("a\n"*10+"b\n" * 10).splitlines(True)
-        other_text = ("a\n"*10+"c\n"+"b\n" * 8 + "c\n").splitlines(True)
-        m3 = Merge3(base_text, other_text, this_text)
-        m_lines = m3.merge_lines('OTHER', 'THIS', reprocess=True)
-        merged_text = "".join(list(m_lines))
-        optimal_text = ("a\n" * 10 + "<<<<<<< OTHER\nc\n=======\n"
-            + ">>>>>>> THIS\n"
-            + 8* "b\n" + "<<<<<<< OTHER\nc\n=======\n"
-            + 2* "b\n" + ">>>>>>> THIS\n")
-        self.assertEquals(optimal_text, merged_text)
-
-    def test_minimal_conflicts_unique(self):
-        def add_newline(s):
-            """Add a newline to each entry in the string"""
-            return [(x+'\n') for x in s]
-
-        base_text = add_newline("abcdefghijklm")
-        this_text = add_newline("abcdefghijklmNOPQRSTUVWXYZ")
-        other_text = add_newline("abcdefghijklm1OPQRSTUVWXY2")
-        m3 = Merge3(base_text, other_text, this_text)
-        m_lines = m3.merge_lines('OTHER', 'THIS', reprocess=True)
-        merged_text = "".join(list(m_lines))
-        optimal_text = ''.join(add_newline("abcdefghijklm")
-            + ["<<<<<<< OTHER\n1\n=======\nN\n>>>>>>> THIS\n"]
-            + add_newline('OPQRSTUVWXY')
-            + ["<<<<<<< OTHER\n2\n=======\nZ\n>>>>>>> THIS\n"]
-            )
-        self.assertEquals(optimal_text, merged_text)
-
-    def test_minimal_conflicts_nonunique(self):
-        def add_newline(s):
-            """Add a newline to each entry in the string"""
-            return [(x+'\n') for x in s]
-
-        base_text = add_newline("abacddefgghij")
-        this_text = add_newline("abacddefgghijkalmontfprz")
-        other_text = add_newline("abacddefgghijknlmontfprd")
-        m3 = Merge3(base_text, other_text, this_text)
-        m_lines = m3.merge_lines('OTHER', 'THIS', reprocess=True)
-        merged_text = "".join(list(m_lines))
-        optimal_text = ''.join(add_newline("abacddefgghijk")
-            + ["<<<<<<< OTHER\nn\n=======\na\n>>>>>>> THIS\n"]
-            + add_newline('lmontfpr')
-            + ["<<<<<<< OTHER\nd\n=======\nz\n>>>>>>> THIS\n"]
-            )
-        self.assertEquals(optimal_text, merged_text)
-
-    def test_reprocess_and_base(self):
-        """Reprocessing and showing base breaks correctly"""
-        base_text = ("a\n" * 20).splitlines(True)
-        this_text = ("a\n"*10+"b\n" * 10).splitlines(True)
-        other_text = ("a\n"*10+"c\n"+"b\n" * 8 + "c\n").splitlines(True)
-        m3 = Merge3(base_text, other_text, this_text)
-        m_lines = m3.merge_lines('OTHER', 'THIS', reprocess=True,
-                                 base_marker='|||||||')
-        self.assertRaises(CantReprocessAndShowBase, list, m_lines)
-
     def test_binary(self):
         self.assertRaises(util.Abort, Merge3, ['\x00'], ['a'], ['b'])
 
     def test_dos_text(self):
         base_text = 'a\r\n'
diff --git a/tests/test-simplemerge.py.out b/tests/test-simplemerge.py.out
--- a/tests/test-simplemerge.py.out
+++ b/tests/test-simplemerge.py.out
@@ -1,5 +1,5 @@ 
-....................
+................
 ----------------------------------------------------------------------
-Ran 20 tests in 0.000s
+Ran 16 tests in 0.000s
 
 OK