* [PATCH] python: fix get_filenames() and make it actually usable
@ 2015-07-31 1:36 Jan Malakhovski
2015-07-31 9:42 ` Tomi Ollila
0 siblings, 1 reply; 2+ messages in thread
From: Jan Malakhovski @ 2015-07-31 1:36 UTC (permalink / raw)
To: notmuch
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
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH] python: fix get_filenames() and make it actually usable
2015-07-31 1:36 [PATCH] python: fix get_filenames() and make it actually usable Jan Malakhovski
@ 2015-07-31 9:42 ` Tomi Ollila
0 siblings, 0 replies; 2+ messages in thread
From: Tomi Ollila @ 2015-07-31 9:42 UTC (permalink / raw)
To: Jan Malakhovski, notmuch
On Fri, Jul 31 2015, Jan Malakhovski <oxij@oxij.org> wrote:
> 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.
Few comments:
There seems to be some code move -- to make the review effort easier
those could be in separate patch.
New tests for len() & unicode() usage in test/T390-python.sh would be nice
(first test that succeeds in one and fails in another (as iter exhausted),
marked known broken, then patch which "fixes" it)
(I also thought of "lazily" filling list of filenames when it is first
requested but doing that probably does not make sense).
Tomi
> ---
> 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
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2015-07-31 9:42 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-31 1:36 [PATCH] python: fix get_filenames() and make it actually usable Jan Malakhovski
2015-07-31 9:42 ` Tomi Ollila
Code repositories for project(s) associated with this public inbox
https://yhetil.org/notmuch.git/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).