Patchwork [08,of,10,V2] manifest: rename matches to _matches

login
register
mail settings
Submitter Durham Goode
Date March 8, 2017, 3:22 a.m.
Message ID <b76e7da4fb9ab00c3b98.1488943360@dev111.prn1.facebook.com>
Download mbox | patch
Permalink /patch/18984/
State Changes Requested
Delegated to: Martin von Zweigbergk
Headers show

Comments

Durham Goode - March 8, 2017, 3:22 a.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1488937983 28800
#      Tue Mar 07 17:53:03 2017 -0800
# Node ID b76e7da4fb9ab00c3b98e5613d7caa0b4aceaf8b
# Parent  822259e9f117b11cd54d6a13565a3202b508eae8
manifest: rename matches to _matches

Now that there are no external consumers of manifest.matches, let's rename it to
_matches so it remains internal. This means alternative manifest implementations
no longer have to implement it, and can instead implement efficient versions of
diff() and filesnotin().
via Mercurial-devel - March 8, 2017, 8:40 p.m.
On Tue, Mar 7, 2017 at 7:22 PM, Durham Goode <durham@fb.com> wrote:
> # HG changeset patch
> # User Durham Goode <durham@fb.com>
> # Date 1488937983 28800
> #      Tue Mar 07 17:53:03 2017 -0800
> # Node ID b76e7da4fb9ab00c3b98e5613d7caa0b4aceaf8b
> # Parent  822259e9f117b11cd54d6a13565a3202b508eae8
> manifest: rename matches to _matches
>
> Now that there are no external consumers of manifest.matches, let's rename it to
> _matches so it remains internal. This means alternative manifest implementations
> no longer have to implement it, and can instead implement efficient versions of
> diff() and filesnotin().
>
> diff --git a/mercurial/manifest.py b/mercurial/manifest.py
> --- a/mercurial/manifest.py
> +++ b/mercurial/manifest.py
> @@ -448,8 +448,8 @@ class manifestdict(object):
>      def filesnotin(self, m2, match=None):
>          '''Set of files in this manifest that are not in the other'''
>          if match:
> -            m1 = self.matches(match)
> -            m2 = m2.matches(match)
> +            m1 = self._matches(match)
> +            m2 = m2._matches(match)
>              return m1.filesnotin(m2)
>          diff = self.diff(m2)
>          files = set(filepath
> @@ -510,7 +510,7 @@ class manifestdict(object):
>              if not self.hasdir(fn):
>                  match.bad(fn, None)
>
> -    def matches(self, match):
> +    def _matches(self, match):
>          '''generate a new manifest filtered by the match argument'''
>          if match.always():
>              return self.copy()
> @@ -543,8 +543,8 @@ class manifestdict(object):
>          string.
>          '''
>          if match:
> -            m1 = self.matches(match)
> -            m2 = m2.matches(match)
> +            m1 = self._matches(match)
> +            m2 = m2._matches(match)
>              return m1.diff(m2, clean=clean)
>          return self._lm.diff(m2._lm, clean)
>
> @@ -917,8 +917,8 @@ class treemanifest(object):
>      def filesnotin(self, m2, match=None):
>          '''Set of files in this manifest that are not in the other'''
>          if match:
> -            m1 = self.matches(match)
> -            m2 = m2.matches(match)
> +            m1 = self._matches(match)
> +            m2 = m2._matches(match)
>              return m1.filesnotin(m2)
>
>          files = set()
> @@ -1002,7 +1002,7 @@ class treemanifest(object):
>                  for f in self._dirs[p]._walk(match):
>                      yield f
>
> -    def matches(self, match):
> +    def _matches(self, match):

The class (treemanifest) already had a _matches() method, so I'll drop
this patch for now. I may queue later patches in the series (still
reviewing).

>          '''generate a new manifest filtered by the match argument'''
>          if match.always():
>              return self.copy()
> @@ -1054,8 +1054,8 @@ class treemanifest(object):
>          string.
>          '''
>          if match:
> -            m1 = self.matches(match)
> -            m2 = m2.matches(match)
> +            m1 = self._matches(match)
> +            m2 = m2._matches(match)
>              return m1.diff(m2, clean=clean)
>          result = {}
>          emptytree = treemanifest()
> diff --git a/tests/test-manifest.py b/tests/test-manifest.py
> --- a/tests/test-manifest.py
> +++ b/tests/test-manifest.py
> @@ -233,7 +233,7 @@ class basemanifesttests(object):
>          self.assertEqual(want, m['foo'])
>          # make sure the suffix survives a copy
>          match = matchmod.match('', '', ['re:foo'])
> -        m2 = m.matches(match)
> +        m2 = m._matches(match)
>          self.assertEqual(want, m2['foo'])
>          self.assertEqual(1, len(m2))
>          m2 = m.copy()
> @@ -255,7 +255,7 @@ class basemanifesttests(object):
>                  assert False
>              return True
>          match.matchfn = filt
> -        self.assertRaises(AssertionError, m.matches, match)
> +        self.assertRaises(AssertionError, m._matches, match)
>
>      def testRemoveItem(self):
>          m = self.parsemanifest(A_SHORT_MANIFEST)
> @@ -358,7 +358,7 @@ class basemanifesttests(object):
>
>          match = matchmod.match('/', '',
>                  ['file1', 'file200', 'file300'], exact=True)
> -        m2 = m.matches(match)
> +        m2 = m._matches(match)
>
>          w = ('file1\0%sx\n'
>               'file200\0%sl\n'
> @@ -374,7 +374,7 @@ class basemanifesttests(object):
>          match = matchmod.match('/', '',
>                  ['a/b/c/bar.txt', 'a/b/d/qux.py', 'readme.txt', 'nonexistent'],
>                  exact=True)
> -        m2 = m.matches(match)
> +        m2 = m._matches(match)
>
>          self.assertEqual(
>                  ['a/b/c/bar.txt', 'a/b/d/qux.py', 'readme.txt'],
> @@ -386,7 +386,7 @@ class basemanifesttests(object):
>          m = self.parsemanifest(A_DEEPER_MANIFEST)
>
>          match = matchmod.match('/', '', ['a/f'], default='relpath')
> -        m2 = m.matches(match)
> +        m2 = m._matches(match)
>
>          self.assertEqual([], m2.keys())
>
> @@ -397,7 +397,7 @@ class basemanifesttests(object):
>
>          flist = m.keys()[80:300]
>          match = matchmod.match('/', '', flist, exact=True)
> -        m2 = m.matches(match)
> +        m2 = m._matches(match)
>
>          self.assertEqual(flist, m2.keys())
>
> @@ -406,7 +406,7 @@ class basemanifesttests(object):
>          m = self.parsemanifest(A_DEEPER_MANIFEST)
>
>          match = matchmod.match('/', '', [''])
> -        m2 = m.matches(match)
> +        m2 = m._matches(match)
>
>          self.assertEqual(m.keys(), m2.keys())
>
> @@ -416,7 +416,7 @@ class basemanifesttests(object):
>          m = self.parsemanifest(A_DEEPER_MANIFEST)
>
>          match = matchmod.match('/', '', ['a/b'], default='relpath')
> -        m2 = m.matches(match)
> +        m2 = m._matches(match)
>
>          self.assertEqual([
>              'a/b/c/bar.py', 'a/b/c/bar.txt', 'a/b/c/foo.py', 'a/b/c/foo.txt',
> @@ -430,7 +430,7 @@ class basemanifesttests(object):
>          m = self.parsemanifest(A_DEEPER_MANIFEST)
>
>          match = matchmod.match('/', '', ['a/b'], exact=True)
> -        m2 = m.matches(match)
> +        m2 = m._matches(match)
>
>          self.assertEqual([], m2.keys())
>
> @@ -440,7 +440,7 @@ class basemanifesttests(object):
>          m = self.parsemanifest(A_DEEPER_MANIFEST)
>
>          match = matchmod.match('/', 'a/b', ['.'], default='relpath')
> -        m2 = m.matches(match)
> +        m2 = m._matches(match)
>
>          self.assertEqual([
>              'a/b/c/bar.py', 'a/b/c/bar.txt', 'a/b/c/foo.py', 'a/b/c/foo.txt',
> @@ -453,7 +453,7 @@ class basemanifesttests(object):
>          m = self.parsemanifest(A_DEEPER_MANIFEST)
>
>          match = matchmod.match('/', '', ['a/b/*/*.txt'])
> -        m2 = m.matches(match)
> +        m2 = m._matches(match)
>
>          self.assertEqual(
>                  ['a/b/c/bar.txt', 'a/b/c/foo.txt', 'a/b/d/ten.txt'],
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/manifest.py b/mercurial/manifest.py
--- a/mercurial/manifest.py
+++ b/mercurial/manifest.py
@@ -448,8 +448,8 @@  class manifestdict(object):
     def filesnotin(self, m2, match=None):
         '''Set of files in this manifest that are not in the other'''
         if match:
-            m1 = self.matches(match)
-            m2 = m2.matches(match)
+            m1 = self._matches(match)
+            m2 = m2._matches(match)
             return m1.filesnotin(m2)
         diff = self.diff(m2)
         files = set(filepath
@@ -510,7 +510,7 @@  class manifestdict(object):
             if not self.hasdir(fn):
                 match.bad(fn, None)
 
-    def matches(self, match):
+    def _matches(self, match):
         '''generate a new manifest filtered by the match argument'''
         if match.always():
             return self.copy()
@@ -543,8 +543,8 @@  class manifestdict(object):
         string.
         '''
         if match:
-            m1 = self.matches(match)
-            m2 = m2.matches(match)
+            m1 = self._matches(match)
+            m2 = m2._matches(match)
             return m1.diff(m2, clean=clean)
         return self._lm.diff(m2._lm, clean)
 
@@ -917,8 +917,8 @@  class treemanifest(object):
     def filesnotin(self, m2, match=None):
         '''Set of files in this manifest that are not in the other'''
         if match:
-            m1 = self.matches(match)
-            m2 = m2.matches(match)
+            m1 = self._matches(match)
+            m2 = m2._matches(match)
             return m1.filesnotin(m2)
 
         files = set()
@@ -1002,7 +1002,7 @@  class treemanifest(object):
                 for f in self._dirs[p]._walk(match):
                     yield f
 
-    def matches(self, match):
+    def _matches(self, match):
         '''generate a new manifest filtered by the match argument'''
         if match.always():
             return self.copy()
@@ -1054,8 +1054,8 @@  class treemanifest(object):
         string.
         '''
         if match:
-            m1 = self.matches(match)
-            m2 = m2.matches(match)
+            m1 = self._matches(match)
+            m2 = m2._matches(match)
             return m1.diff(m2, clean=clean)
         result = {}
         emptytree = treemanifest()
diff --git a/tests/test-manifest.py b/tests/test-manifest.py
--- a/tests/test-manifest.py
+++ b/tests/test-manifest.py
@@ -233,7 +233,7 @@  class basemanifesttests(object):
         self.assertEqual(want, m['foo'])
         # make sure the suffix survives a copy
         match = matchmod.match('', '', ['re:foo'])
-        m2 = m.matches(match)
+        m2 = m._matches(match)
         self.assertEqual(want, m2['foo'])
         self.assertEqual(1, len(m2))
         m2 = m.copy()
@@ -255,7 +255,7 @@  class basemanifesttests(object):
                 assert False
             return True
         match.matchfn = filt
-        self.assertRaises(AssertionError, m.matches, match)
+        self.assertRaises(AssertionError, m._matches, match)
 
     def testRemoveItem(self):
         m = self.parsemanifest(A_SHORT_MANIFEST)
@@ -358,7 +358,7 @@  class basemanifesttests(object):
 
         match = matchmod.match('/', '',
                 ['file1', 'file200', 'file300'], exact=True)
-        m2 = m.matches(match)
+        m2 = m._matches(match)
 
         w = ('file1\0%sx\n'
              'file200\0%sl\n'
@@ -374,7 +374,7 @@  class basemanifesttests(object):
         match = matchmod.match('/', '',
                 ['a/b/c/bar.txt', 'a/b/d/qux.py', 'readme.txt', 'nonexistent'],
                 exact=True)
-        m2 = m.matches(match)
+        m2 = m._matches(match)
 
         self.assertEqual(
                 ['a/b/c/bar.txt', 'a/b/d/qux.py', 'readme.txt'],
@@ -386,7 +386,7 @@  class basemanifesttests(object):
         m = self.parsemanifest(A_DEEPER_MANIFEST)
 
         match = matchmod.match('/', '', ['a/f'], default='relpath')
-        m2 = m.matches(match)
+        m2 = m._matches(match)
 
         self.assertEqual([], m2.keys())
 
@@ -397,7 +397,7 @@  class basemanifesttests(object):
 
         flist = m.keys()[80:300]
         match = matchmod.match('/', '', flist, exact=True)
-        m2 = m.matches(match)
+        m2 = m._matches(match)
 
         self.assertEqual(flist, m2.keys())
 
@@ -406,7 +406,7 @@  class basemanifesttests(object):
         m = self.parsemanifest(A_DEEPER_MANIFEST)
 
         match = matchmod.match('/', '', [''])
-        m2 = m.matches(match)
+        m2 = m._matches(match)
 
         self.assertEqual(m.keys(), m2.keys())
 
@@ -416,7 +416,7 @@  class basemanifesttests(object):
         m = self.parsemanifest(A_DEEPER_MANIFEST)
 
         match = matchmod.match('/', '', ['a/b'], default='relpath')
-        m2 = m.matches(match)
+        m2 = m._matches(match)
 
         self.assertEqual([
             'a/b/c/bar.py', 'a/b/c/bar.txt', 'a/b/c/foo.py', 'a/b/c/foo.txt',
@@ -430,7 +430,7 @@  class basemanifesttests(object):
         m = self.parsemanifest(A_DEEPER_MANIFEST)
 
         match = matchmod.match('/', '', ['a/b'], exact=True)
-        m2 = m.matches(match)
+        m2 = m._matches(match)
 
         self.assertEqual([], m2.keys())
 
@@ -440,7 +440,7 @@  class basemanifesttests(object):
         m = self.parsemanifest(A_DEEPER_MANIFEST)
 
         match = matchmod.match('/', 'a/b', ['.'], default='relpath')
-        m2 = m.matches(match)
+        m2 = m._matches(match)
 
         self.assertEqual([
             'a/b/c/bar.py', 'a/b/c/bar.txt', 'a/b/c/foo.py', 'a/b/c/foo.txt',
@@ -453,7 +453,7 @@  class basemanifesttests(object):
         m = self.parsemanifest(A_DEEPER_MANIFEST)
 
         match = matchmod.match('/', '', ['a/b/*/*.txt'])
-        m2 = m.matches(match)
+        m2 = m._matches(match)
 
         self.assertEqual(
                 ['a/b/c/bar.txt', 'a/b/c/foo.txt', 'a/b/d/ten.txt'],