Merge lp:~gz/brz/lt_by_dirs into lp:brz

[Martin Packman]

Replace dirstate helpers cmp_by_dirs with lt_by_dirs

Change dirstate helpers function cmp_by_dirs into lt_by_dirs. It's only used as lt in the dirstate code, and Python 3 doesn't have a builtin cmp.

[Jelmer Vernooij (jelmer)]

review: Approve

=== modified file 'breezy/_dirstate_helpers_py.py'
--- breezy/_dirstate_helpers_py.py	2017-06-04 18:09:30 +0000
+++ breezy/_dirstate_helpers_py.py	2017-06-10 00:42:09 +0000
@@ -163,22 +163,20 @@
     return lo
 
 
-def cmp_by_dirs(path1, path2):
+def lt_by_dirs(path1, path2):
     """Compare two paths directory by directory.
 
     This is equivalent to doing::
 
-       cmp(path1.split('/'), path2.split('/'))
+       operator.lt(path1.split('/'), path2.split('/'))
 
     The idea is that you should compare path components separately. This
-    differs from plain ``cmp(path1, path2)`` for paths like ``'a-b'`` and
-    ``a/b``. "a-b" comes after "a" but would come before "a/b" lexically.
+    differs from plain ``path1 < path2`` for paths like ``'a-b'`` and ``a/b``.
+    "a-b" comes after "a" but would come before "a/b" lexically.
 
     :param path1: first path
     :param path2: second path
-    :return: negative number if ``path1`` comes first,
-        0 if paths are equal,
-        and positive number if ``path2`` sorts first
+    :return: True if path1 comes first, otherwise False
     """
     if not isinstance(path1, str):
         raise TypeError("'path1' must be a plain string, not %s: %r"
@@ -186,7 +184,7 @@
     if not isinstance(path2, str):
         raise TypeError("'path2' must be a plain string, not %s: %r"
                         % (type(path2), path2))
-    return cmp(path1.split('/'), path2.split('/'))
+    return path1.split('/') < path2.split('/')
 
 
 def _cmp_path_by_dirblock(path1, path2):

=== modified file 'breezy/_dirstate_helpers_pyx.pyx'
--- breezy/_dirstate_helpers_pyx.pyx	2017-06-10 00:09:13 +0000
+++ breezy/_dirstate_helpers_pyx.pyx	2017-06-10 00:42:09 +0000
@@ -244,22 +244,20 @@
     return 0
 
 
-def cmp_by_dirs(path1, path2):
+def lt_by_dirs(path1, path2):
     """Compare two paths directory by directory.
 
     This is equivalent to doing::
 
-       cmp(path1.split('/'), path2.split('/'))
+       operator.lt(path1.split('/'), path2.split('/'))
 
     The idea is that you should compare path components separately. This
-    differs from plain ``cmp(path1, path2)`` for paths like ``'a-b'`` and
-    ``a/b``. "a-b" comes after "a" but would come before "a/b" lexically.
+    differs from plain ``path1 < path2`` for paths like ``'a-b'`` and ``a/b``.
+    "a-b" comes after "a" but would come before "a/b" lexically.
 
     :param path1: first path
     :param path2: second path
-    :return: negative number if ``path1`` comes first,
-        0 if paths are equal,
-        and positive number if ``path2`` sorts first
+    :return: True if path1 comes first, otherwise False
     """
     if not PyString_CheckExact(path1):
         raise TypeError("'path1' must be a plain string, not %s: %r"
@@ -267,10 +265,10 @@
     if not PyString_CheckExact(path2):
         raise TypeError("'path2' must be a plain string, not %s: %r"
                         % (type(path2), path2))
-    return _cmp_by_dirs(PyString_AsString(path1),
-                        PyString_Size(path1),
-                        PyString_AsString(path2),
-                        PyString_Size(path2))
+    return -1 == _cmp_by_dirs(PyString_AsString(path1),
+                              PyString_Size(path1),
+                              PyString_AsString(path2),
+                              PyString_Size(path2))
 
 
 def _cmp_path_by_dirblock(path1, path2):

=== modified file 'breezy/dirstate.py'
--- breezy/dirstate.py	2017-06-05 20:48:31 +0000
+++ breezy/dirstate.py	2017-06-10 00:42:09 +0000
@@ -2861,7 +2861,7 @@
                 # both sides are dealt with, move on
                 current_old = advance(old_iterator)
                 current_new = advance(new_iterator)
-            elif (cmp_by_dirs(new_dirname, current_old[0][0]) < 0
+            elif (lt_by_dirs(new_dirname, current_old[0][0])
                   or (new_dirname == current_old[0][0]
                       and new_entry_key[1:] < current_old[0][1:])):
                 # new comes before:
@@ -3803,7 +3803,7 @@
     def iter_changes(self):
         """Iterate over the changes."""
         utf8_decode = cache_utf8._utf8_decode
-        _cmp_by_dirs = cmp_by_dirs
+        _lt_by_dirs = lt_by_dirs
         _process_entry = self._process_entry
         search_specific_files = self.search_specific_files
         searched_specific_files = self.searched_specific_files
@@ -3947,7 +3947,7 @@
                    current_block is not None):
                 if (current_dir_info and current_block
                     and current_dir_info[0][0] != current_block[0]):
-                    if _cmp_by_dirs(current_dir_info[0][0], current_block[0]) < 0:
+                    if _lt_by_dirs(current_dir_info[0][0], current_block[0]):
                         # filesystem data refers to paths not covered by the dirblock.
                         # this has two possibilities:
                         # A) it is versioned but empty, so there is no block for it
@@ -4269,7 +4269,7 @@
         bisect_dirblock,
         _bisect_path_left,
         _bisect_path_right,
-        cmp_by_dirs,
+        lt_by_dirs,
         pack_stat,
         ProcessEntryC as _process_entry,
         update_entry as update_entry,
@@ -4281,7 +4281,7 @@
         bisect_dirblock,
         _bisect_path_left,
         _bisect_path_right,
-        cmp_by_dirs,
+        lt_by_dirs,
         pack_stat,
         )
     # FIXME: It would be nice to be able to track moved lines so that the

=== modified file 'breezy/tests/test__dirstate_helpers.py'
--- breezy/tests/test__dirstate_helpers.py	2017-05-22 00:56:52 +0000
+++ breezy/tests/test__dirstate_helpers.py	2017-06-10 00:42:09 +0000
@@ -376,20 +376,20 @@
         return bisect_dirblock
 
 
-class TestCmpByDirs(tests.TestCase):
-    """Test an implementation of cmp_by_dirs()
+class TestLtByDirs(tests.TestCase):
+    """Test an implementation of lt_by_dirs()
 
-    cmp_by_dirs() compares 2 paths by their directory sections, rather than as
+    lt_by_dirs() compares 2 paths by their directory sections, rather than as
     plain strings.
 
-    Child test cases can override ``get_cmp_by_dirs`` to test a specific
+    Child test cases can override ``get_lt_by_dirs`` to test a specific
     implementation.
     """
 
-    def get_cmp_by_dirs(self):
-        """Get a specific implementation of cmp_by_dirs."""
-        from breezy._dirstate_helpers_py import cmp_by_dirs
-        return cmp_by_dirs
+    def get_lt_by_dirs(self):
+        """Get a specific implementation of lt_by_dirs."""
+        from breezy._dirstate_helpers_py import lt_by_dirs
+        return lt_by_dirs
 
     def assertCmpByDirs(self, expected, str1, str2):
         """Compare the two strings, in both directions.
@@ -399,17 +399,17 @@
         :param str1: string to compare
         :param str2: string to compare
         """
-        cmp_by_dirs = self.get_cmp_by_dirs()
+        lt_by_dirs = self.get_lt_by_dirs()
         if expected == 0:
             self.assertEqual(str1, str2)
-            self.assertEqual(0, cmp_by_dirs(str1, str2))
-            self.assertEqual(0, cmp_by_dirs(str2, str1))
+            self.assertFalse(lt_by_dirs(str1, str2))
+            self.assertFalse(lt_by_dirs(str2, str1))
         elif expected > 0:
-            self.assertPositive(cmp_by_dirs(str1, str2))
-            self.assertNegative(cmp_by_dirs(str2, str1))
+            self.assertFalse(lt_by_dirs(str1, str2))
+            self.assertTrue(lt_by_dirs(str2, str1))
         else:
-            self.assertNegative(cmp_by_dirs(str1, str2))
-            self.assertPositive(cmp_by_dirs(str2, str1))
+            self.assertTrue(lt_by_dirs(str1, str2))
+            self.assertFalse(lt_by_dirs(str2, str1))
 
     def test_cmp_empty(self):
         """Compare against the empty string."""
@@ -475,10 +475,10 @@
         self.assertCmpByDirs(-1, 'ab/cd', 'ab-cd')
 
     def test_cmp_unicode_not_allowed(self):
-        cmp_by_dirs = self.get_cmp_by_dirs()
-        self.assertRaises(TypeError, cmp_by_dirs, u'Unicode', 'str')
-        self.assertRaises(TypeError, cmp_by_dirs, 'str', u'Unicode')
-        self.assertRaises(TypeError, cmp_by_dirs, u'Unicode', u'Unicode')
+        lt_by_dirs = self.get_lt_by_dirs()
+        self.assertRaises(TypeError, lt_by_dirs, u'Unicode', 'str')
+        self.assertRaises(TypeError, lt_by_dirs, 'str', u'Unicode')
+        self.assertRaises(TypeError, lt_by_dirs, u'Unicode', u'Unicode')
 
     def test_cmp_non_ascii(self):
         self.assertCmpByDirs(-1, '\xc2\xb5', '\xc3\xa5') # u'\xb5', u'\xe5'
@@ -488,14 +488,14 @@
         self.assertCmpByDirs(-1, 'b/a', 'b/\xc2\xb5') # u'b/a', u'b/\xb5'
 
 
-class TestCompiledCmpByDirs(TestCmpByDirs):
-    """Test the pyrex implementation of cmp_by_dirs"""
+class TestCompiledLtByDirs(TestLtByDirs):
+    """Test the pyrex implementation of lt_by_dirs"""
 
     _test_needs_features = [compiled_dirstate_helpers_feature]
 
-    def get_cmp_by_dirs(self):
-        from breezy._dirstate_helpers_pyx import cmp_by_dirs
-        return cmp_by_dirs
+    def get_lt_by_dirs(self):
+        from breezy._dirstate_helpers_pyx import lt_by_dirs
+        return lt_by_dirs
 
 
 class TestCmpPathByDirblock(tests.TestCase):
@@ -778,12 +778,12 @@
             from breezy._dirstate_helpers_py import _bisect_path_right
         self.assertIs(_bisect_path_right, dirstate._bisect_path_right)
 
-    def test_cmp_by_dirs(self):
+    def test_lt_by_dirs(self):
         if compiled_dirstate_helpers_feature.available():
-            from breezy._dirstate_helpers_pyx import cmp_by_dirs
+            from breezy._dirstate_helpers_pyx import lt_by_dirs
         else:
-            from breezy._dirstate_helpers_py import cmp_by_dirs
-        self.assertIs(cmp_by_dirs, dirstate.cmp_by_dirs)
+            from breezy._dirstate_helpers_py import lt_by_dirs
+        self.assertIs(lt_by_dirs, dirstate.lt_by_dirs)
 
     def test__read_dirblocks(self):
         if compiled_dirstate_helpers_feature.available():