Floris Bruynooghe venit, vidit, dixit 2021-01-11 21:33:16:
> Hi Micahel,
>
> Thanks for adding this feature!
>
> On Wed 06 Jan 2021 at 10:08 +0100, Michael J. Gruber wrote:
> > diff --git a/bindings/python-cffi/notmuch2/_query.py
> > b/bindings/python-cffi/notmuch2/_query.py
> > index 1db6ec96..a1310944 100644
> > --- a/bindings/python-cffi/notmuch2/_query.py
> > +++ b/bindings/python-cffi/notmuch2/_query.py
> > @@ -2,6 +2,7 @@ from notmuch2 import _base as base
> > from notmuch2 import _capi as capi
> > from notmuch2 import _errors as errors
> > from notmuch2 import _message as message
> > +from notmuch2 import _tags as tags
> > from notmuch2 import _thread as thread
> >
> >
> > @@ -66,6 +67,17 @@ class Query(base.NotmuchObject):
> > raise errors.NotmuchError(ret)
> > return count_p[0]
> >
> > + def collect_tags(self):
> > + """Return the tags of messages matching this query."""
> > + msgs_pp = capi.ffi.new('notmuch_messages_t**')
> > + ret = capi.lib.notmuch_query_search_messages(self._query_p,
> > msgs_pp)
> > + if ret != capi.lib.NOTMUCH_STATUS_SUCCESS:
> > + raise errors.NotmuchError(ret)
> > + self._msgs_p = msgs_pp[0]
> > + tagset = tags.ImmutableTagSet(
> > + self, '_msgs_p', capi.lib.notmuch_messages_collect_tags)
> > + return tags.ImmutableTagSet._from_iterable(tagset)
> > +
> > def threads(self):
> > """Return an iterator over all the threads found by the query."""
> > threads_pp = capi.ffi.new('notmuch_threads_t **')
>
> How this this look on the C level for the memory allocator? If the
> query gets freed the messages also get freed? In that case indeed the
> messages do need to be kept alive together with the query indeed.
> Keeping them as an attribute on the query object seems reasonable I
> think, they'd be freed at the same time and I don't think this creates a
> circular reference.
I'm afraid I don't have the full picture of the memory model and object
lifetimes in the bindings.
> There's only a small detail though: to get the correct ObjectDestroyed
> exceptions you need Query._msgs_p to be a _base.MemoryPoiniter
> descriptor, like Query._query_p is currently. And then you also need to
> set it to None in Query._destroy.
I was trying to mimick what Query.messages() does: it creates msgs_pp
locally. The only reason for self._msgs_p in Thread.collect_tags() is
the way in which tags.ImmutableTagSet() expects the pointer: as an
attribute of self. But maybe there is a better way to call into
capi.lib.notmuch_messages_collect_tags()?
If not, and if I understand you correctly, then I should apply
diff --git i/bindings/python-cffi/notmuch2/_query.py
w/bindings/python-cffi/notmuch2/_query.py
index a1310944..fa58825d 100644
--- i/bindings/python-cffi/notmuch2/_query.py
+++ w/bindings/python-cffi/notmuch2/_query.py
@@ -17,6 +17,7 @@ class Query(base.NotmuchObject):
match libnotmuch's memory management.
"""
_query_p = base.MemoryPointer()
+ _msgs_p = base.MemoryPointer()
def __init__(self, db, query_p):
self._db = db
@@ -40,6 +41,7 @@ class Query(base.NotmuchObject):
if self.alive:
capi.lib.notmuch_query_destroy(self._query_p)
self._query_p = None
+ self._msgs_p = None
on top of this patch, right?
> It would also be really good to add at least one happy-path test for
> this, ensuring that you get the tags in the query. You could add some
> to TestQuery in test_database.py. The database is created with unread
> and inbox tags i think (see conftest.py:75) so the messages should
> already have some tags which is sufficient as you're not testing
> notmuch's behaviour itself but only that the bindings give you a right
> looking tagset (the tagset itself also has tests already, so no need to
> test this extensively, just see if you have some tags).
Thanks for the pointer. Will do for v2, comparing:
set(db.collect_tags('*')) == set(db.tags)
Michael
_______________________________________________
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org
