Re: [PATCH 3/7] lib: Make notmuch_query_search_messages set the exclude flag

Subject: Re: [PATCH 3/7] lib: Make notmuch_query_search_messages set the exclude flag

Date: Wed, 01 Feb 2012 18:00:34 +0000

To: Austin Clements

Cc: notmuch@notmuchmail.org

From: Mark Walters


On Tue, 31 Jan 2012 11:25:06 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> Quoth Mark Walters on Jan 31 at 11:45 am:
> > On Mon, 30 Jan 2012 23:43:52 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> > > Quoth Mark Walters on Jan 29 at  6:39 pm:
> > > >      notmuch_message_node_t *iterator;
> > > >  };
> > > >  
> > > > diff --git a/lib/notmuch.h b/lib/notmuch.h
> > > > index 7929fe7..740d005 100644
> > > > --- a/lib/notmuch.h
> > > > +++ b/lib/notmuch.h
> > > > @@ -449,6 +449,11 @@ typedef enum {
> > > >  const char *
> > > >  notmuch_query_get_query_string (notmuch_query_t *query);
> > > >  
> > > > +/* specify whether to results should omit the excluded results rather
> > > > + * than just marking them excluded */
> > > > +void
> > > > +notmuch_query_set_omit_excluded_messages (notmuch_query_t *query, notmuch_bool_t omit);
> > > > +
> > > 
> > > I don't think we should add this API.  The library behavior will not
> > > change for library users that don't use excludes and library users
> > > that do use excludes should by aware of the excluded flag and do the
> > > appropriate thing.
> > > 
> > > I can see why this is handy in some cases, but I don't think it
> > > provides enough utility to warrant becoming part of the permanent and
> > > minimal library interface.
> > 
> > This is really a performance improvement: suppose that there are lots of
> > threads that only match in excluded messages. Then without this flag we
> > will spend lots of time constructing the thread only for it to be
> > ignored. (In contrived situations this could be arbitrarily slower.)
> 
> I would prefer to keep the public API minimal and only introduce
> something like this if people actually experience performance
> problems.

I have looked at removing this api and it does make things
inconvenient. It is nice to be able to pass a notmuch_messages_t around
without having to teach every function that takes notmuch_messages_t
(e.g., notmuch_messages_collect_tags) about the exclude flag.

An alternative might be to make it possible to set an omit excludes flag
on a notmuch_messages_t object but that seems worse than the current
patches.

Any thoughts?

Mark

PS I think my current version fixes all the other queries so I will post
that later today (in a new thread!).

Thread: