From: Jan Malakhovski <oxij@oxij.org>
To: notmuch@notmuchmail.org
Subject: [PATCH] python: fix get_filenames() and make it actually usable
Date: Fri, 31 Jul 2015 01:36:26 +0000
Message-Id: <1438306586-7597-1-git-send-email-oxij@oxij.org>
Precedence: list

The problem with the previous implementation is that different
versions of python exhaust __iter__() differently and the
implementation that can be exhausted is not only absolutely unusable
in the user code, but it also can not be consistently used with both
python 2.* and 3.*.

This doesn't change the interface.
---
 bindings/python/notmuch/filenames.py | 84 +++++++++++-------------------------
 1 file changed, 26 insertions(+), 58 deletions(-)

diff --git a/bindings/python/notmuch/filenames.py b/bindings/python/notmuch/filenames.py
index 229f414..e2b8886 100644
--- a/bindings/python/notmuch/filenames.py
+++ b/bindings/python/notmuch/filenames.py
@@ -65,6 +65,18 @@ class Filenames(Python3StringMixIn):
     _get.argtypes = [NotmuchFilenamesP]
     _get.restype = c_char_p
 
+    _valid = nmlib.notmuch_filenames_valid
+    _valid.argtypes = [NotmuchFilenamesP]
+    _valid.restype = bool
+
+    _move_to_next = nmlib.notmuch_filenames_move_to_next
+    _move_to_next.argtypes = [NotmuchFilenamesP]
+    _move_to_next.restype = None
+
+    _destroy = nmlib.notmuch_filenames_destroy
+    _destroy.argtypes = [NotmuchFilenamesP]
+    _destroy.restype = None
+
     def __init__(self, files_p, parent):
         """
         :param files_p: A pointer to an underlying *notmuch_tags_t*
@@ -83,68 +95,24 @@ class Filenames(Python3StringMixIn):
         if not files_p:
             raise NullPointerError()
 
-        self._files_p = files_p
+        self._list = []
+        while self._valid(files_p):
+            file_ = self._get(files_p)
+            self._move_to_next(files_p)
+            self._list.append(file_.decode('utf-8', 'ignore'))
+        self._destroy(files_p)
+
         #save reference to parent object so we keep it alive
         self._parent = parent
 
     def __iter__(self):
-        """ Make Filenames an iterator """
-        return self
-
-    _valid = nmlib.notmuch_filenames_valid
-    _valid.argtypes = [NotmuchFilenamesP]
-    _valid.restype = bool
-
-    _move_to_next = nmlib.notmuch_filenames_move_to_next
-    _move_to_next.argtypes = [NotmuchFilenamesP]
-    _move_to_next.restype = None
-
-    def __next__(self):
-        if not self._files_p:
-            raise NotInitializedError()
-
-        if not self._valid(self._files_p):
-            self._files_p = None
-            raise StopIteration
-
-        file_ = Filenames._get(self._files_p)
-        self._move_to_next(self._files_p)
-        return file_.decode('utf-8', 'ignore')
-    next = __next__ # python2.x iterator protocol compatibility
-
-    def __unicode__(self):
-        """Represent Filenames() as newline-separated list of full paths
-
-        .. note::
-
-            This method exhausts the iterator object, so you will not be able to
-            iterate over them again.
-        """
-        return "\n".join(self)
-
-    _destroy = nmlib.notmuch_filenames_destroy
-    _destroy.argtypes = [NotmuchMessageP]
-    _destroy.restype = None
-
-    def __del__(self):
-        """Close and free the notmuch filenames"""
-        if self._files_p:
-            self._destroy(self._files_p)
+        """Make Filenames an iterator"""
+        return self._list.__iter__()
 
     def __len__(self):
-        """len(:class:`Filenames`) returns the number of contained files
+        """len(:class:`Filenames`) returns the number of contained files"""
+        return self._list.__len__()
 
-        .. note::
-
-            This method exhausts the iterator object, so you will not be able to
-            iterate over them again.
-        """
-        if not self._files_p:
-            raise NotInitializedError()
-
-        i = 0
-        while self._valid(self._files_p):
-            self._move_to_next(self._files_p)
-            i += 1
-        self._files_p = None
-        return i
+    def __unicode__(self):
+        """Represent Filenames() as newline-separated list of full paths"""
+        return "\n".join(self)
-- 
2.4.1