Patchwork [2,of,2,V2] lazymanifest: make __iter__ generate filenames, not 3-tuples

login
register
mail settings
Submitter Martin von Zweigbergk
Date March 13, 2015, 4:03 a.m.
Message ID <294b9f920fb261f3f01b.1426219433@martinvonz.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/8045/
State Accepted
Commit 49cd847fd69ace75047cd4e32611f86b8e937a1b
Headers show

Comments

Martin von Zweigbergk - March 13, 2015, 4:03 a.m.
# HG changeset patch
# User Martin von Zweigbergk <martinvonz@google.com>
# Date 1426209509 25200
#      Thu Mar 12 18:18:29 2015 -0700
# Node ID 294b9f920fb261f3f01bf3c1fdc9e4a97f1d36ba
# Parent  d38410efbb182767cc61abb8f3a5bcee6f671b39
lazymanifest: make __iter__ generate filenames, not 3-tuples

The _lazymanifest type(s) behave very much like a sorted dict with
filenames as keys and (nodeid, flags) as values. It therefore seems
surprising that its __iter__ generates 3-tuples of (path, nodeid,
flags). Let's make it match dict's behavior of generating the keys
instead, and add a new iterentries method for the 3-tuples. With this
change, the "x" in "if x in lm" and "for x in lm" now have the same
type (a filename string).
Matt Mackall - March 13, 2015, 5:13 p.m.
On Thu, 2015-03-12 at 21:03 -0700, Martin von Zweigbergk wrote:
> # HG changeset patch
> # User Martin von Zweigbergk <martinvonz@google.com>
> # Date 1426209509 25200
> #      Thu Mar 12 18:18:29 2015 -0700
> # Node ID 294b9f920fb261f3f01bf3c1fdc9e4a97f1d36ba
> # Parent  d38410efbb182767cc61abb8f3a5bcee6f671b39
> lazymanifest: make __iter__ generate filenames, not 3-tuples

These are queued for default, thanks.

Patch

diff -r d38410efbb18 -r 294b9f920fb2 mercurial/manifest.c
--- a/mercurial/manifest.c	Thu Mar 12 18:53:44 2015 -0700
+++ b/mercurial/manifest.c	Thu Mar 12 18:18:29 2015 -0700
@@ -234,7 +234,7 @@ 
 	return self->m->lines + self->pos;
 }
 
-static PyObject *lmiter_iternext(PyObject *o)
+static PyObject *lmiter_iterentriesnext(PyObject *o)
 {
 	size_t pl;
 	line *l;
@@ -261,10 +261,10 @@ 
 	return NULL;
 }
 
-static PyTypeObject lazymanifestIterator = {
+static PyTypeObject lazymanifestEntriesIterator = {
 	PyObject_HEAD_INIT(NULL)
 	0,                               /*ob_size */
-	"parsers.lazymanifest.iterator", /*tp_name */
+	"parsers.lazymanifest.entriesiterator", /*tp_name */
 	sizeof(lmIter),                  /*tp_basicsize */
 	0,                               /*tp_itemsize */
 	lmiter_dealloc,                  /*tp_dealloc */
@@ -285,13 +285,13 @@ 
 	/* tp_flags: Py_TPFLAGS_HAVE_ITER tells python to
 	   use tp_iter and tp_iternext fields. */
 	Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_ITER,
-	"Iterator for a lazymanifest.",  /* tp_doc */
+	"Iterator for 3-tuples in a lazymanifest.",  /* tp_doc */
 	0,                               /* tp_traverse */
 	0,                               /* tp_clear */
 	0,                               /* tp_richcompare */
 	0,                               /* tp_weaklistoffset */
 	PyObject_SelfIter,               /* tp_iter: __iter__() method */
-	lmiter_iternext,                 /* tp_iternext: next() method */
+	lmiter_iterentriesnext,          /* tp_iternext: next() method */
 };
 
 static PyObject *lmiter_iterkeysnext(PyObject *o)
@@ -340,7 +340,7 @@ 
 
 static lazymanifest *lazymanifest_copy(lazymanifest *self);
 
-static PyObject *lazymanifest_getiter(lazymanifest *self)
+static PyObject *lazymanifest_getentriesiter(lazymanifest *self)
 {
 	lmIter *i = NULL;
 	lazymanifest *t = lazymanifest_copy(self);
@@ -348,7 +348,7 @@ 
 		PyErr_NoMemory();
 		return NULL;
 	}
-	i = PyObject_New(lmIter, &lazymanifestIterator);
+	i = PyObject_New(lmIter, &lazymanifestEntriesIterator);
 	if (i) {
 		i->m = t;
 		i->pos = -1;
@@ -860,6 +860,8 @@ 
 static PyMethodDef lazymanifest_methods[] = {
 	{"iterkeys", (PyCFunction)lazymanifest_getkeysiter, METH_NOARGS,
 	 "Iterate over file names in this lazymanifest."},
+	{"iterentries", (PyCFunction)lazymanifest_getentriesiter, METH_NOARGS,
+	 "Iterate over (path, nodeid, flags) typles in this lazymanifest."},
 	{"copy", (PyCFunction)lazymanifest_copy, METH_NOARGS,
 	 "Make a copy of this lazymanifest."},
 	{"filtercopy", (PyCFunction)lazymanifest_filtercopy, METH_O,
@@ -898,7 +900,7 @@ 
 	0,                                                /* tp_clear */
 	0,                                                /* tp_richcompare */
 	0,                                             /* tp_weaklistoffset */
-	(getiterfunc)lazymanifest_getiter,                /* tp_iter */
+	(getiterfunc)lazymanifest_getkeysiter,                /* tp_iter */
 	0,                                                /* tp_iternext */
 	lazymanifest_methods,                             /* tp_methods */
 	0,                                                /* tp_members */
diff -r d38410efbb18 -r 294b9f920fb2 mercurial/manifest.py
--- a/mercurial/manifest.py	Thu Mar 12 18:53:44 2015 -0700
+++ b/mercurial/manifest.py	Thu Mar 12 18:18:29 2015 -0700
@@ -44,11 +44,14 @@ 
         dict.__setitem__(self, k, (node, flag))
 
     def __iter__(self):
-        return ((f, e[0], e[1]) for f, e in sorted(self.iteritems()))
+        return iter(sorted(dict.keys(self)))
 
     def iterkeys(self):
         return iter(sorted(dict.keys(self)))
 
+    def iterentries(self):
+        return ((f, e[0], e[1]) for f, e in sorted(self.iteritems()))
+
     def copy(self):
         c = _lazymanifest('')
         c.update(self)
@@ -76,14 +79,14 @@ 
 
     def filtercopy(self, filterfn):
         c = _lazymanifest('')
-        for f, n, fl in self:
+        for f, n, fl in self.iterentries():
             if filterfn(f):
                 c[f] = n, fl
         return c
 
     def text(self):
         """Get the full data of this manifest as a bytestring."""
-        fl = sorted(self)
+        fl = sorted(self.iterentries())
 
         _hex = revlog.hex
         # if this is changed to support newlines in filenames,
@@ -119,7 +122,7 @@ 
         del self._lm[key]
 
     def __iter__(self):
-        return self._lm.iterkeys()
+        return self._lm.__iter__()
 
     def iterkeys(self):
         return self._lm.iterkeys()
@@ -140,8 +143,8 @@ 
 
     def filesnotin(self, m2):
         '''Set of files in this manifest that are not in the other'''
-        files = set(self.iterkeys())
-        files.difference_update(m2.iterkeys())
+        files = set(self)
+        files.difference_update(m2)
         return files
 
     def matches(self, match):
@@ -196,7 +199,7 @@ 
         return c
 
     def iteritems(self):
-        return (x[:2] for x in self._lm)
+        return (x[:2] for x in self._lm.iterentries())
 
     def text(self):
         return self._lm.text()
diff -r d38410efbb18 -r 294b9f920fb2 tests/test-manifest.py
--- a/tests/test-manifest.py	Thu Mar 12 18:53:44 2015 -0700
+++ b/tests/test-manifest.py	Thu Mar 12 18:18:29 2015 -0700
@@ -40,7 +40,7 @@ 
     def testEmptyManifest(self):
         m = manifestmod._lazymanifest('')
         self.assertEqual(0, len(m))
-        self.assertEqual([], list(m))
+        self.assertEqual([], list(m.iterentries()))
 
     def testManifest(self):
         m = manifestmod._lazymanifest(A_SHORT_MANIFEST)
@@ -49,7 +49,7 @@ 
             ('foo', binascii.unhexlify(HASH_1), ''),
             ]
         self.assertEqual(len(want), len(m))
-        self.assertEqual(want, list(m))
+        self.assertEqual(want, list(m.iterentries()))
         self.assertEqual((binascii.unhexlify(HASH_1), ''), m['foo'])
         self.assertRaises(KeyError, lambda : m['wat'])
         self.assertEqual((binascii.unhexlify(HASH_2), 'l'),
@@ -88,7 +88,7 @@ 
         self.assertEqual((h2, ''), m['beta'])
         self.assertRaises(KeyError, lambda : m['foo'])
         w = [('alpha', h1, ''), ('bar/baz/qux.py', h2, 'l'), ('beta', h2, '')]
-        self.assertEqual(w, list(m))
+        self.assertEqual(w, list(m.iterentries()))
 
     def testSetGetNodeSuffix(self):
         clean = manifestmod._lazymanifest(A_SHORT_MANIFEST)
@@ -100,7 +100,7 @@ 
         self.assertEqual(want, m['foo'])
         self.assertEqual([('bar/baz/qux.py', binascii.unhexlify(HASH_2), 'l'),
                           ('foo', binascii.unhexlify(HASH_1) + 'a', '')],
-                         list(m))
+                         list(m.iterentries()))
         # Sometimes it even tries a 22-byte fake hash, but we can
         # return 21 and it'll work out
         m['foo'] = want[0] + '+', f
@@ -114,7 +114,7 @@ 
         self.assertEqual(want, m2['foo'])
         # suffix with iteration
         self.assertEqual([('bar/baz/qux.py', binascii.unhexlify(HASH_2), 'l'),
-                          ('foo', want[0], '')], list(m))
+                          ('foo', want[0], '')], list(m.iterentries()))
         # shows up in diff
         self.assertEqual({'foo': (want, (h, ''))}, m.diff(clean))
         self.assertEqual({'foo': ((h, ''), want)}, clean.diff(m))