Date: Mon, 23 Jan 2012 21:53:31 -0500
From: Austin Clements <amdragon@MIT.EDU>
To: Mark Walters <markwalters1009@gmail.com>
Subject: Re: [RFC PATCH 3/4] Add NOTMUCH_MESSAGE_FLAG_EXCLUDED flag
Message-ID: <20120124025331.GZ16740@mit.edu>
References: <20120124011609.GX16740@mit.edu>
	<1327367923-18228-3-git-send-email-markwalters1009@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1327367923-18228-3-git-send-email-markwalters1009@gmail.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Cc: notmuch@notmuchmail.org
Precedence: list

Quoth Mark Walters on Jan 24 at  1:18 am:
> Add the actual NOTMUCH_MESSAGE_FLAG_EXCLUDED flag.
> 
> ---
>  lib/notmuch-private.h |    1 +
>  lib/notmuch.h         |    3 ++-
>  lib/query.cc          |   11 +++++++----
>  lib/thread.cc         |   14 ++++++++++----
>  4 files changed, 20 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
> index e791bb0..cb3eca6 100644
> --- a/lib/notmuch-private.h
> +++ b/lib/notmuch-private.h
> @@ -216,6 +216,7 @@ _notmuch_thread_create (void *ctx,
>  			notmuch_database_t *notmuch,
>  			unsigned int seed_doc_id,
>  			notmuch_doc_id_set_t *match_set,
> +			notmuch_doc_id_set_t *excluded_doc_ids,
>  			notmuch_sort_t sort);
>  
>  /* message.cc */
> diff --git a/lib/notmuch.h b/lib/notmuch.h
> index 7929fe7..cf0d45d 100644
> --- a/lib/notmuch.h
> +++ b/lib/notmuch.h
> @@ -895,7 +895,8 @@ notmuch_message_get_filenames (notmuch_message_t *message);
>  
>  /* Message flags */
>  typedef enum _notmuch_message_flag {
> -    NOTMUCH_MESSAGE_FLAG_MATCH
> +    NOTMUCH_MESSAGE_FLAG_MATCH,
> +    NOTMUCH_MESSAGE_FLAG_EXCLUDED
>  } notmuch_message_flag_t;
>  
>  /* Get a value of a flag for the email corresponding to 'message'. */
> diff --git a/lib/query.cc b/lib/query.cc
> index 92fa834..69e32bd 100644
> --- a/lib/query.cc
> +++ b/lib/query.cc
> @@ -55,6 +55,7 @@ struct visible _notmuch_threads {
>      /* The set of matched docid's that have not been assigned to a
>       * thread. Initially, this contains every docid in doc_ids. */
>      notmuch_doc_id_set_t match_set;
> +    notmuch_doc_id_set_t *excluded_doc_ids;
>  };
>  
>  static notmuch_bool_t
> @@ -302,6 +303,9 @@ _notmuch_mset_messages_get (notmuch_messages_t *messages)
>  	INTERNAL_ERROR ("a messages iterator contains a non-existent document ID.\n");
>      }
>  
> +    if (_notmuch_doc_id_set_contains (messages->excluded_doc_ids, doc_id))
> +	notmuch_message_set_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED, TRUE);
> +
>      return message;
>  }
>  
> @@ -314,10 +318,6 @@ _notmuch_mset_messages_move_to_next (notmuch_messages_t *messages)
>  
>      mset_messages->iterator++;
>  
> -    while ((mset_messages->iterator != mset_messages->iterator_end) &&
> -	   (_notmuch_doc_id_set_contains (messages->excluded_doc_ids,
> -					  *mset_messages->iterator)))
> -	mset_messages->iterator++;
>  }
>  
>  static notmuch_bool_t
> @@ -403,6 +403,8 @@ notmuch_query_search_threads (notmuch_query_t *query)
>  	notmuch_messages_move_to_next (messages);
>      }
>      threads->doc_id_pos = 0;
> +    /* the excluded messages are in query context so this should be ok */
> +    threads->excluded_doc_ids = messages->excluded_doc_ids;
>  
>      talloc_free (messages);
>  
> @@ -452,6 +454,7 @@ notmuch_threads_get (notmuch_threads_t *threads)
>  				   threads->query->notmuch,
>  				   doc_id,
>  				   &threads->match_set,
> +				   threads->excluded_doc_ids,
>  				   threads->query->sort);
>  }
>  
> diff --git a/lib/thread.cc b/lib/thread.cc
> index 0435ee6..6ea2a44 100644
> --- a/lib/thread.cc
> +++ b/lib/thread.cc
> @@ -302,7 +302,8 @@ _thread_set_subject_from_message (notmuch_thread_t *thread,
>  static void
>  _thread_add_matched_message (notmuch_thread_t *thread,
>  			     notmuch_message_t *message,
> -			     notmuch_sort_t sort)
> +			     notmuch_sort_t sort,
> +			     notmuch_bool_t excluded)
>  {
>      time_t date;
>      notmuch_message_t *hashed_message;
> @@ -321,7 +322,8 @@ _thread_add_matched_message (notmuch_thread_t *thread,
>  	    _thread_set_subject_from_message (thread, message);
>      }
>  
> -    thread->matched_messages++;
> +    if (!excluded)
> +	thread->matched_messages++;

I interpret notmuch_thread_get_matched_messages as returning the
number of messages with the "match" flag, which this approach changes.
I would suggest introducing a new, more flexible API along the lines
of

/* Return the number of messages in thread for which
 * notmuch_message_get_flag(msg) & msg == match.
 */
int
notmuch_thread_count_flags (notmuch_thread_t *thread, int mask, int match);

This would subsume the existing
notmuch_thread_get_{matched,total}_messages APIs.  It could either
iterate over the message list (which, curiously, we don't currently
track), or it could proactively count messages in an array indexed by
the message's flags (which wouldn't scale to large numbers of flags
but, for now, would only be of length 4).

>  
>      if (g_hash_table_lookup_extended (thread->message_hash,
>  			    notmuch_message_get_message_id (message), NULL,
> @@ -392,6 +394,7 @@ _notmuch_thread_create (void *ctx,
>  			notmuch_database_t *notmuch,
>  			unsigned int seed_doc_id,
>  			notmuch_doc_id_set_t *match_set,
> +			notmuch_doc_id_set_t *excluded_doc_ids,
>  			notmuch_sort_t sort)
>  {
>      notmuch_thread_t *thread;
> @@ -456,7 +459,9 @@ _notmuch_thread_create (void *ctx,
>       * oldest or newest subject is desired. */
>      notmuch_query_set_sort (thread_id_query, NOTMUCH_SORT_OLDEST_FIRST);
>  
> -    for (messages = notmuch_query_search_messages (thread_id_query);
> +    messages = notmuch_query_search_messages (thread_id_query);
> +    messages->excluded_doc_ids = excluded_doc_ids;
> +    for (;
>  	 notmuch_messages_valid (messages);
>  	 notmuch_messages_move_to_next (messages))
>      {
> @@ -471,7 +476,8 @@ _notmuch_thread_create (void *ctx,
>  
>  	if ( _notmuch_doc_id_set_contains (match_set, doc_id)) {
>  	    _notmuch_doc_id_set_remove (match_set, doc_id);
> -	    _thread_add_matched_message (thread, message, sort);
> +	    _thread_add_matched_message (thread, message, sort,
> +					 _notmuch_doc_id_set_contains (excluded_doc_ids, doc_id));
>  	}
>  
>  	_notmuch_message_close (message);