From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by arlo.cworth.org (Postfix) with ESMTP id 4ECD16DE0B20 for ; Fri, 31 Jul 2015 02:42:30 -0700 (PDT) X-Virus-Scanned: Debian amavisd-new at cworth.org X-Spam-Flag: NO X-Spam-Score: 0.932 X-Spam-Level: X-Spam-Status: No, score=0.932 tagged_above=-999 required=5 tests=[AWL=0.280, SPF_NEUTRAL=0.652] autolearn=disabled Received: from arlo.cworth.org ([127.0.0.1]) by localhost (arlo.cworth.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Nn8bcNVIejQ6 for ; Fri, 31 Jul 2015 02:42:27 -0700 (PDT) Received: from guru.guru-group.fi (guru.guru-group.fi [46.183.73.34]) by arlo.cworth.org (Postfix) with ESMTP id 607B76DE0B44 for ; Fri, 31 Jul 2015 02:42:26 -0700 (PDT) Received: from guru.guru-group.fi (localhost [IPv6:::1]) by guru.guru-group.fi (Postfix) with ESMTP id CC24B10007F; Fri, 31 Jul 2015 12:42:08 +0300 (EEST) From: Tomi Ollila To: Jan Malakhovski , notmuch@notmuchmail.org Subject: Re: [PATCH] python: fix get_filenames() and make it actually usable In-Reply-To: <1438306586-7597-1-git-send-email-oxij@oxij.org> References: <1438306586-7597-1-git-send-email-oxij@oxij.org> User-Agent: Notmuch/0.20.2+31~g967a7a3 (http://notmuchmail.org) Emacs/24.3.1 (x86_64-unknown-linux-gnu) X-Face: HhBM'cA~ MIME-Version: 1.0 Content-Type: text/plain X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 31 Jul 2015 09:42:30 -0000 On Fri, Jul 31 2015, Jan Malakhovski 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