[PATCH 00/11] add recipients to search output

[PATCH 00/11] add recipients to search output

[Jameson Graef Rollins]

This series is an attempt to add thread recipients to the search
output.

My personal overall goal of this series is to support the handling of
drafts in the emacs ui.  For drafts we want to see recipients, instead
of authors, in the search output.  I can imagine other uses for this
series as well, though.

The first four patches generalize the author list handling in thread
objects to handle any address list.  These patches could be applied
regardless of if the rest of the series is accepted.

After that we modify the thread constructor such that it can hold
thread recipients as well.  Since there is overhead in retrieving
thread recipients from the message files (recipients are not stored in
the database) this is handled with a switch.

Further patches add the new switch to the search CLI that adds thread
recipients to the structured output formats.  I didn't modify the text
output format, since there is no way to extend it.  I can imagine
tweaking the text output such that the author field is instead
replaced by the recipients (as is done for the emacs UI at the end of
the series), but that's not done here.

In the emacs UI, I add a new toggle function that will toggle display
of thread authors or recipients in the 'authors' field of the search
output.  It's unfortunate that this ambiguity in that field name
remains, but I didn't know how to change that cleanly.  I'm working on
some tests for the new emacs functionality that I'll include in the
inevitable v2 of this series.

The last patch is mostly just a tickle to suggest adding the
recipients to the database.  It would make the --include-recipient
searches much faster of course, but it might be overhead in the
database that folks aren't interested in.

As always, feedback, review, and comments are much appreciated.

jamie.

[PATCH 01/11] lib: new thread addresses structure

[Jameson Graef Rollins]

This new structure holds addresses associated with a thread, both
matched and unmatched.  Initially this will be used to replace the
existing infrastructure for storing the addresses of thread authors.
Further patches will use it to store the addresses of threads
recipients.

Init and destructor functions are included, as well as a function to
add addresses to a struct, either "matched" or not.
---
 lib/notmuch.h |    1 +
 lib/thread.cc |  116 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 117 insertions(+)

diff --git a/lib/notmuch.h b/lib/notmuch.h
index 3633bed..6acd38d 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -118,6 +118,7 @@ typedef struct _notmuch_database notmuch_database_t;
 typedef struct _notmuch_query notmuch_query_t;
 typedef struct _notmuch_threads notmuch_threads_t;
 typedef struct _notmuch_thread notmuch_thread_t;
+typedef struct _notmuch_thread_addresses notmuch_thread_addresses_t;
 typedef struct _notmuch_messages notmuch_messages_t;
 typedef struct _notmuch_message notmuch_message_t;
 typedef struct _notmuch_tags notmuch_tags_t;
diff --git a/lib/thread.cc b/lib/thread.cc
index e976d64..7af9eeb 100644
--- a/lib/thread.cc
+++ b/lib/thread.cc
@@ -24,6 +24,14 @@
 #include <gmime/gmime.h>
 #include <glib.h> /* GHashTable */
 
+struct visible _notmuch_thread_addresses {
+    GHashTable *matched_hash;
+    GPtrArray *matched_array;
+    GHashTable *unmatched_hash;
+    GPtrArray *unmatched_array;
+    char *string;
+};
+
 struct visible _notmuch_thread {
     notmuch_database_t *notmuch;
     char *thread_id;
@@ -44,6 +52,18 @@ struct visible _notmuch_thread {
 };
 
 static int
+_notmuch_thread_addresses_destructor (notmuch_thread_addresses_t *addresses)
+{
+    g_hash_table_unref (addresses->matched_hash);
+    g_hash_table_unref (addresses->unmatched_hash);
+    g_ptr_array_free (addresses->matched_array, TRUE);
+    g_ptr_array_free (addresses->unmatched_array, TRUE);
+    addresses->matched_array = NULL;
+    addresses->unmatched_array = NULL;
+    return 0;
+}
+
+static int
 _notmuch_thread_destructor (notmuch_thread_t *thread)
 {
     g_hash_table_unref (thread->authors_hash);
@@ -64,6 +84,81 @@ _notmuch_thread_destructor (notmuch_thread_t *thread)
     return 0;
 }
 
+/* Add address to a thread addresses struct.  If matched is TRUE, then
+ * the address will be added to the matched list.*/
+static void
+_thread_add_address (notmuch_thread_addresses_t *addresses,
+		     const char *address,
+		     notmuch_bool_t matched)
+{
+    char *address_copy;
+    GHashTable *hash;
+    GPtrArray *array;
+
+    if (matched) {
+	hash = addresses->matched_hash;
+	array = addresses->matched_array;
+    } else {
+	hash = addresses->unmatched_hash;
+	array = addresses->unmatched_array;
+    }
+
+    if (address == NULL)
+	return;
+
+    if (g_hash_table_lookup_extended (hash, address, NULL, NULL))
+	return;
+
+    address_copy = talloc_strdup (addresses, address);
+
+    g_hash_table_insert (hash, address_copy, NULL);
+
+    g_ptr_array_add (array, address_copy);
+}
+
+/* Construct an addresses string from matched and unmatched addresses
+ * in notmuch_thread_addresses_t. The string contains matched
+ * addresses first, then non-matched addresses (with the two groups
+ * separated by '|'). Within each group, addresses are listed in date
+ * order. */
+static void
+_resolve_thread_addresses_string (notmuch_thread_addresses_t *addresses)
+{
+    unsigned int i;
+    char *address;
+    int first_non_matched_address = 1;
+
+    /* First, list all matched addressses in date order. */
+    for (i = 0; i < addresses->matched_array->len; i++) {
+	address = (char *) g_ptr_array_index (addresses->matched_array, i);
+	if (addresses->string)
+	    addresses->string = talloc_asprintf (addresses, "%s, %s",
+						 addresses->string,
+						 address);
+	else
+	    addresses->string = address;
+    }
+
+    /* Next, append any non-matched addresses that haven't already appeared. */
+    for (i = 0; i < addresses->unmatched_array->len; i++) {
+	address = (char *) g_ptr_array_index (addresses->unmatched_array, i);
+	if (g_hash_table_lookup_extended (addresses->matched_hash,
+					  address, NULL, NULL))
+	    continue;
+	if (first_non_matched_address) {
+	    addresses->string = talloc_asprintf (addresses, "%s| %s",
+						 addresses->string,
+						 address);
+	} else {
+	    addresses->string = talloc_asprintf (addresses, "%s, %s",
+						 addresses->string,
+						 address);
+	}
+
+	first_non_matched_address = 0;
+    }
+}
+
 /* Add each author of the thread to the thread's authors_hash and to
  * the thread's authors_array. */
 static void
@@ -382,6 +477,27 @@ _resolve_thread_relationships (unused (notmuch_thread_t *thread))
      */
 }
 
+/* Initialize a thread addresses struct. */
+notmuch_thread_addresses_t *
+_thread_addresses_init (const void *ctx)
+{
+    notmuch_thread_addresses_t *addresses;
+
+    addresses = talloc (ctx, notmuch_thread_addresses_t);
+    if (unlikely (addresses == NULL))
+	return NULL;
+
+    addresses->matched_hash = g_hash_table_new_full (g_str_hash, g_str_equal,
+						     NULL, NULL);
+    addresses->matched_array = g_ptr_array_new ();
+    addresses->unmatched_hash = g_hash_table_new_full (g_str_hash, g_str_equal,
+						       NULL, NULL);
+    addresses->unmatched_array = g_ptr_array_new ();
+    addresses->string = NULL;
+
+    return addresses;
+}
+
 /* Create a new notmuch_thread_t object by finding the thread
  * containing the message with the given doc ID, treating any messages
  * contained in match_set as "matched".  Remove all messages in the
-- 
1.7.10.4

[PATCH 02/11] lib: use new addresses structure for thread authors

[Jameson Graef Rollins]

Now that we have the infrastructure in place, we modify the thread
object and associated functions to use the new addresses structure for
storing thread authors.
---
 lib/thread.cc |   41 +++++++++++------------------------------
 1 file changed, 11 insertions(+), 30 deletions(-)

diff --git a/lib/thread.cc b/lib/thread.cc
index 7af9eeb..9e0e5cb 100644
--- a/lib/thread.cc
+++ b/lib/thread.cc
@@ -36,11 +36,7 @@ struct visible _notmuch_thread {
     notmuch_database_t *notmuch;
     char *thread_id;
     char *subject;
-    GHashTable *authors_hash;
-    GPtrArray *authors_array;
-    GHashTable *matched_authors_hash;
-    GPtrArray *matched_authors_array;
-    char *authors;
+    notmuch_thread_addresses_t *authors;
     GHashTable *tags;
 
     notmuch_message_list_t *message_list;
@@ -66,21 +62,9 @@ _notmuch_thread_addresses_destructor (notmuch_thread_addresses_t *addresses)
 static int
 _notmuch_thread_destructor (notmuch_thread_t *thread)
 {
-    g_hash_table_unref (thread->authors_hash);
-    g_hash_table_unref (thread->matched_authors_hash);
+    _notmuch_thread_addresses_destructor (thread->authors);
     g_hash_table_unref (thread->tags);
     g_hash_table_unref (thread->message_hash);
-
-    if (thread->authors_array) {
-	g_ptr_array_free (thread->authors_array, TRUE);
-	thread->authors_array = NULL;
-    }
-
-    if (thread->matched_authors_array) {
-	g_ptr_array_free (thread->matched_authors_array, TRUE);
-	thread->matched_authors_array = NULL;
-    }
-
     return 0;
 }
 
@@ -341,7 +325,7 @@ _thread_add_message (notmuch_thread_t *thread,
 		author = internet_address_mailbox_get_addr (mailbox);
 	    }
 	    clean_author = _thread_cleanup_author (thread, author, from);
-	    _thread_add_author (thread, clean_author);
+	    _thread_add_address (thread->authors, clean_author, FALSE);
 	    notmuch_message_set_author (message, clean_author);
 	}
 	g_object_unref (G_OBJECT (list));
@@ -436,7 +420,7 @@ _thread_add_matched_message (notmuch_thread_t *thread,
 				  NOTMUCH_MESSAGE_FLAG_MATCH, 1);
     }
 
-    _thread_add_matched_author (thread, notmuch_message_get_author (hashed_message));
+    _thread_add_address (thread->authors, notmuch_message_get_author (hashed_message), TRUE);
 }
 
 static void
@@ -555,14 +539,11 @@ _notmuch_thread_create (void *ctx,
     thread->notmuch = notmuch;
     thread->thread_id = talloc_strdup (thread, thread_id);
     thread->subject = NULL;
-    thread->authors_hash = g_hash_table_new_full (g_str_hash, g_str_equal,
-						  NULL, NULL);
-    thread->authors_array = g_ptr_array_new ();
-    thread->matched_authors_hash = g_hash_table_new_full (g_str_hash,
-							  g_str_equal,
-							  NULL, NULL);
-    thread->matched_authors_array = g_ptr_array_new ();
-    thread->authors = NULL;
+
+    thread->authors = _thread_addresses_init (thread);
+    if (unlikely (thread->authors == NULL))
+	return NULL;
+
     thread->tags = g_hash_table_new_full (g_str_hash, g_str_equal,
 					  free, NULL);
 
@@ -607,7 +588,7 @@ _notmuch_thread_create (void *ctx,
 
     notmuch_query_destroy (thread_id_query);
 
-    _resolve_thread_authors_string (thread);
+    _resolve_thread_addresses_string (thread->authors);
 
     _resolve_thread_relationships (thread);
 
@@ -641,7 +622,7 @@ notmuch_thread_get_matched_messages (notmuch_thread_t *thread)
 const char *
 notmuch_thread_get_authors (notmuch_thread_t *thread)
 {
-    return thread->authors;
+    return thread->authors->string;
 }
 
 const char *
-- 
1.7.10.4

[PATCH 03/11] lib: give _thread_cleanup_author a more generic name

[Jameson Graef Rollins]

We will use this for cleaning non-author address fields, so we give it
the more generic name _thread_cleanup_address.
---
 lib/thread.cc |   43 ++++++++++++++++++++++---------------------
 1 file changed, 22 insertions(+), 21 deletions(-)

diff --git a/lib/thread.cc b/lib/thread.cc
index 9e0e5cb..b53ccb0 100644
--- a/lib/thread.cc
+++ b/lib/thread.cc
@@ -242,48 +242,49 @@ _resolve_thread_authors_string (notmuch_thread_t *thread)
  * "Last, First MI" <first.mi.last@company.com>
  */
 static char *
-_thread_cleanup_author (notmuch_thread_t *thread,
-			const char *author, const char *from)
+_thread_cleanup_address (notmuch_thread_t *thread,
+			 const char *address,
+			 const char *original)
 {
-    char *clean_author,*test_author;
+    char *clean_address,*test_address;
     const char *comma;
     char *blank;
     int fname,lname;
 
-    if (author == NULL)
+    if (address == NULL)
 	return NULL;
-    clean_author = talloc_strdup(thread, author);
-    if (clean_author == NULL)
+    clean_address = talloc_strdup(thread, address);
+    if (clean_address == NULL)
 	return NULL;
     /* check if there's a comma in the name and that there's a
      * component of the name behind it (so the name doesn't end with
      * the comma - in which case the string that strchr finds is just
      * one character long ",\0").
-     * Otherwise just return the copy of the original author name that
+     * Otherwise just return the copy of the original address name that
      * we just made*/
-    comma = strchr(author,',');
+    comma = strchr(address,',');
     if (comma && strlen(comma) > 1) {
 	/* let's assemble what we think is the correct name */
-	lname = comma - author;
-	fname = strlen(author) - lname - 2;
-	strncpy(clean_author, comma + 2, fname);
-	*(clean_author+fname) = ' ';
-	strncpy(clean_author + fname + 1, author, lname);
-	*(clean_author+fname+1+lname) = '\0';
+	lname = comma - address;
+	fname = strlen(address) - lname - 2;
+	strncpy(clean_address, comma + 2, fname);
+	*(clean_address+fname) = ' ';
+	strncpy(clean_address + fname + 1, address, lname);
+	*(clean_address+fname+1+lname) = '\0';
 	/* make a temporary copy and see if it matches the email */
-	test_author = talloc_strdup(thread,clean_author);
+	test_address = talloc_strdup(thread,clean_address);
 
-	blank=strchr(test_author,' ');
+	blank=strchr(test_address,' ');
 	while (blank != NULL) {
 	    *blank = '.';
-	    blank=strchr(test_author,' ');
+	    blank=strchr(test_address,' ');
 	}
-	if (strcasestr(from, test_author) == NULL)
+	if (strcasestr(original, test_address) == NULL)
 	    /* we didn't identify this as part of the email address
-	    * so let's punt and return the original author */
-	    strcpy (clean_author, author);
+	    * so let's punt and return the original address */
+	    strcpy (clean_address, address);
     }
-    return clean_author;
+    return clean_address;
 }
 
 /* Add 'message' as a message that belongs to 'thread'.
-- 
1.7.10.4

[PATCH 04/11] lib: remove no longer needed author-specific thread functions

[Jameson Graef Rollins]

The _add_author, _add_matched_author, and
_resolve_thread_authors_string functions have been replaced by the
more generic _address functions that utilize the new
notmuch_thread_addresses_t.
---
 lib/thread.cc |   91 ---------------------------------------------------------
 1 file changed, 91 deletions(-)

diff --git a/lib/thread.cc b/lib/thread.cc
index b53ccb0..757e143 100644
--- a/lib/thread.cc
+++ b/lib/thread.cc
@@ -143,97 +143,6 @@ _resolve_thread_addresses_string (notmuch_thread_addresses_t *addresses)
     }
 }
 
-/* Add each author of the thread to the thread's authors_hash and to
- * the thread's authors_array. */
-static void
-_thread_add_author (notmuch_thread_t *thread,
-		    const char *author)
-{
-    char *author_copy;
-
-    if (author == NULL)
-	return;
-
-    if (g_hash_table_lookup_extended (thread->authors_hash,
-				      author, NULL, NULL))
-	return;
-
-    author_copy = talloc_strdup (thread, author);
-
-    g_hash_table_insert (thread->authors_hash, author_copy, NULL);
-
-    g_ptr_array_add (thread->authors_array, author_copy);
-}
-
-/* Add each matched author of the thread to the thread's
- * matched_authors_hash and to the thread's matched_authors_array. */
-static void
-_thread_add_matched_author (notmuch_thread_t *thread,
-			    const char *author)
-{
-    char *author_copy;
-
-    if (author == NULL)
-	return;
-
-    if (g_hash_table_lookup_extended (thread->matched_authors_hash,
-				      author, NULL, NULL))
-	return;
-
-    author_copy = talloc_strdup (thread, author);
-
-    g_hash_table_insert (thread->matched_authors_hash, author_copy, NULL);
-
-    g_ptr_array_add (thread->matched_authors_array, author_copy);
-}
-
-/* Construct an authors string from matched_authors_array and
- * authors_array. The string contains matched authors first, then
- * non-matched authors (with the two groups separated by '|'). Within
- * each group, authors are listed in date order. */
-static void
-_resolve_thread_authors_string (notmuch_thread_t *thread)
-{
-    unsigned int i;
-    char *author;
-    int first_non_matched_author = 1;
-
-    /* First, list all matched authors in date order. */
-    for (i = 0; i < thread->matched_authors_array->len; i++) {
-	author = (char *) g_ptr_array_index (thread->matched_authors_array, i);
-	if (thread->authors)
-	    thread->authors = talloc_asprintf (thread, "%s, %s",
-					       thread->authors,
-					       author);
-	else
-	    thread->authors = author;
-    }
-
-    /* Next, append any non-matched authors that haven't already appeared. */
-    for (i = 0; i < thread->authors_array->len; i++) {
-	author = (char *) g_ptr_array_index (thread->authors_array, i);
-	if (g_hash_table_lookup_extended (thread->matched_authors_hash,
-					  author, NULL, NULL))
-	    continue;
-	if (first_non_matched_author) {
-	    thread->authors = talloc_asprintf (thread, "%s| %s",
-					       thread->authors,
-					       author);
-	} else {
-	    thread->authors = talloc_asprintf (thread, "%s, %s",
-					       thread->authors,
-					       author);
-	}
-
-	first_non_matched_author = 0;
-    }
-
-    g_ptr_array_free (thread->authors_array, TRUE);
-    thread->authors_array = NULL;
-    g_ptr_array_free (thread->matched_authors_array, TRUE);
-    thread->matched_authors_array = NULL;
-}
-
 /* clean up the ugly "Lastname, Firstname" format that some mail systems
  * (most notably, Exchange) are creating to be "Firstname Lastname"
  * To make sure that we don't change other potential situations where a
-- 
1.7.10.4

[PATCH 05/11] lib: add ability to store recipients in message structure

[Jameson Graef Rollins]

Added as a string, in parallel to the authors element.
---
 lib/message.cc        |   18 ++++++++++++++++++
 lib/notmuch-private.h |    9 +++++++++
 2 files changed, 27 insertions(+)

diff --git a/lib/message.cc b/lib/message.cc
index 978de06..fa28073 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -36,6 +36,7 @@ struct visible _notmuch_message {
     notmuch_string_list_t *filename_term_list;
     notmuch_string_list_t *filename_list;
     char *author;
+    char *recipients;
     notmuch_message_file_t *message_file;
     notmuch_message_list_t *replies;
     unsigned long flags;
@@ -109,6 +110,7 @@ _notmuch_message_create_for_document (const void *talloc_owner,
     message->filename_list = NULL;
     message->message_file = NULL;
     message->author = NULL;
+    message->recipients = NULL;
 
     message->replies = _notmuch_message_list_create (message);
     if (unlikely (message->replies == NULL)) {
@@ -808,6 +810,22 @@ notmuch_message_set_author (notmuch_message_t *message,
     return;
 }
 
+const char *
+notmuch_message_get_recipients (notmuch_message_t *message)
+{
+    return message->recipients;
+}
+
+void
+notmuch_message_set_recipients (notmuch_message_t *message,
+				const char *recipients)
+{
+    if (message->recipients)
+	talloc_free(message->recipients);
+    message->recipients = talloc_strdup(message, recipients);
+    return;
+}
+
 void
 _notmuch_message_set_header_values (notmuch_message_t *message,
 				    const char *date,
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index bfb4111..27a41b6 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -325,6 +325,15 @@ notmuch_message_set_author (notmuch_message_t *message, const char *author);
 const char *
 notmuch_message_get_author (notmuch_message_t *message);
 
+/* Set the recipients of 'message' - this is the representation used
+ * when displaying the message */
+void
+notmuch_message_set_recipients (notmuch_message_t *message, const char *recipients);
+
+/* Get the authors of 'message' */
+const char *
+notmuch_message_get_recipients (notmuch_message_t *message);
+
 
 /* index.cc */
 
-- 
1.7.10.4

[PATCH 06/11] lib: store thread recipients in thread structure

[Jameson Graef Rollins]

This utilizes the new thread addresses struct to store thread
recipients, again in parallel to authors.

Since message recipients are not stored in the database, including
recipients in the thread structure exacts a significant overhead as
the recipients are retrieved from the original message files.  Because
of this, a new boolean argument, include_recipients, is added to the
necessary functions (_notmuch_thread_create, _thread_add_message and
_thread_add_matched_message) that controls whether the recipients are
fetched and included.  If message recipients are ever stored in the
database this new argument could probably be removed.
---
 lib/notmuch-private.h |    3 +-
 lib/notmuch.h         |   14 +++++++++
 lib/query.cc          |    3 +-
 lib/thread.cc         |   77 +++++++++++++++++++++++++++++++++++++------------
 4 files changed, 76 insertions(+), 21 deletions(-)

diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 27a41b6..32d1523 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -232,7 +232,8 @@ _notmuch_thread_create (void *ctx,
 			unsigned int seed_doc_id,
 			notmuch_doc_id_set_t *match_set,
 			notmuch_string_list_t *excluded_terms,
-			notmuch_sort_t sort);
+			notmuch_sort_t sort,
+			notmuch_bool_t include_recipients);
 
 /* message.cc */
 
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 6acd38d..f9e71c1 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -759,6 +759,20 @@ notmuch_thread_get_matched_messages (notmuch_thread_t *thread);
 const char *
 notmuch_thread_get_authors (notmuch_thread_t *thread);
 
+/* Get the recipients of 'thread'
+ *
+ * The returned string is a comma-separated list of the names of the
+ * recipients of mail messages in the query results that belong to this
+ * thread.
+ *
+ * The returned string belongs to 'thread' and as such, should not be
+ * modified by the caller and will only be valid for as long as the
+ * thread is valid, (which is until notmuch_thread_destroy or until
+ * the query from which it derived is destroyed).
+ */
+const char *
+notmuch_thread_get_recipients (notmuch_thread_t *thread);
+
 /* Get the subject of 'thread'
  *
  * The subject is taken from the first message (according to the query
diff --git a/lib/query.cc b/lib/query.cc
index e9c1a2d..54833a7 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -486,7 +486,8 @@ notmuch_threads_get (notmuch_threads_t *threads)
 				   doc_id,
 				   &threads->match_set,
 				   threads->query->exclude_terms,
-				   threads->query->sort);
+				   threads->query->sort,
+				   FALSE);
 }
 
 void
diff --git a/lib/thread.cc b/lib/thread.cc
index 757e143..baf07c2 100644
--- a/lib/thread.cc
+++ b/lib/thread.cc
@@ -37,6 +37,7 @@ struct visible _notmuch_thread {
     char *thread_id;
     char *subject;
     notmuch_thread_addresses_t *authors;
+    notmuch_thread_addresses_t *recipients;
     GHashTable *tags;
 
     notmuch_message_list_t *message_list;
@@ -63,6 +64,7 @@ static int
 _notmuch_thread_destructor (notmuch_thread_t *thread)
 {
     _notmuch_thread_addresses_destructor (thread->authors);
+    _notmuch_thread_addresses_destructor (thread->recipients);
     g_hash_table_unref (thread->tags);
     g_hash_table_unref (thread->message_hash);
     return 0;
@@ -204,14 +206,17 @@ _thread_cleanup_address (notmuch_thread_t *thread,
 static void
 _thread_add_message (notmuch_thread_t *thread,
 		     notmuch_message_t *message,
-		     notmuch_string_list_t *exclude_terms)
+		     notmuch_string_list_t *exclude_terms,
+		     notmuch_bool_t include_recipients)
 {
     notmuch_tags_t *tags;
     const char *tag;
-    InternetAddressList *list = NULL;
+    InternetAddressList *from_list = NULL;
+    InternetAddressList *to_list = NULL;
     InternetAddress *address;
     const char *from, *author;
-    char *clean_author;
+    const char *to, *recipient;
+    char *clean_address;
 
     _notmuch_message_list_add_message (thread->message_list,
 				       talloc_steal (thread, message));
@@ -223,10 +228,9 @@ _thread_add_message (notmuch_thread_t *thread,
 
     from = notmuch_message_get_header (message, "from");
     if (from)
-	list = internet_address_list_parse_string (from);
-
-    if (list) {
-	address = internet_address_list_get_address (list, 0);
+	from_list = internet_address_list_parse_string (from);
+    if (from_list) {
+	address = internet_address_list_get_address (from_list, 0);
 	if (address) {
 	    author = internet_address_get_name (address);
 	    if (author == NULL) {
@@ -234,11 +238,32 @@ _thread_add_message (notmuch_thread_t *thread,
 		mailbox = INTERNET_ADDRESS_MAILBOX (address);
 		author = internet_address_mailbox_get_addr (mailbox);
 	    }
-	    clean_author = _thread_cleanup_author (thread, author, from);
-	    _thread_add_address (thread->authors, clean_author, FALSE);
-	    notmuch_message_set_author (message, clean_author);
+	    clean_address = _thread_cleanup_address (thread, author, from);
+	    _thread_add_address (thread->authors, clean_address, FALSE);
+	    notmuch_message_set_author (message, clean_address);
+	}
+	g_object_unref (G_OBJECT (from_list));
+    }
+
+    if (include_recipients) {
+    to = notmuch_message_get_header (message, "to");
+    if (to)
+	to_list = internet_address_list_parse_string (to);
+    if (to_list) {
+	address = internet_address_list_get_address (to_list, 0);
+	if (address) {
+	    recipient = internet_address_get_name (address);
+	    if (recipient == NULL) {
+		InternetAddressMailbox *mailbox;
+		mailbox = INTERNET_ADDRESS_MAILBOX (address);
+		recipient = internet_address_mailbox_get_addr (mailbox);
+	    }
+	    clean_address = _thread_cleanup_address (thread, recipient, to);
+	    _thread_add_address (thread->recipients, clean_address, FALSE);
+	    notmuch_message_set_recipients (message, clean_address);
 	}
-	g_object_unref (G_OBJECT (list));
+	g_object_unref (G_OBJECT (to_list));
+    }
     }
 
     if (! thread->subject) {
@@ -301,7 +326,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 include_recipients)
 {
     time_t date;
     notmuch_message_t *hashed_message;
@@ -331,6 +357,8 @@ _thread_add_matched_message (notmuch_thread_t *thread,
     }
 
     _thread_add_address (thread->authors, notmuch_message_get_author (hashed_message), TRUE);
+    if (include_recipients)
+    _thread_add_address (thread->recipients, notmuch_message_get_recipients (hashed_message), TRUE);
 }
 
 static void
@@ -399,10 +427,10 @@ _thread_addresses_init (const void *ctx)
  *
  * Creating the thread will perform a database search to get all
  * messages belonging to the thread and will get the first subject
- * line, the total count of messages, and all authors in the thread.
- * Each message in the thread is checked against match_set to allow
- * for a separate count of matched messages, and to allow a viewer to
- * display these messages differently.
+ * line, the total count of messages, and all authors and recipients
+ * of the thread.  Each message in the thread is checked against
+ * match_set to allow for a separate count of matched messages, and to
+ * allow a viewer to display these messages differently.
  *
  * Here, 'ctx' is talloc context for the resulting thread object.
  *
@@ -414,7 +442,8 @@ _notmuch_thread_create (void *ctx,
 			unsigned int seed_doc_id,
 			notmuch_doc_id_set_t *match_set,
 			notmuch_string_list_t *exclude_terms,
-			notmuch_sort_t sort)
+			notmuch_sort_t sort,
+			notmuch_bool_t include_recipients)
 {
     notmuch_thread_t *thread;
     notmuch_message_t *seed_message;
@@ -453,6 +482,9 @@ _notmuch_thread_create (void *ctx,
     thread->authors = _thread_addresses_init (thread);
     if (unlikely (thread->authors == NULL))
 	return NULL;
+    thread->recipients = _thread_addresses_init (thread);
+    if (unlikely (thread->recipients == NULL))
+	return NULL;
 
     thread->tags = g_hash_table_new_full (g_str_hash, g_str_equal,
 					  free, NULL);
@@ -486,11 +518,11 @@ _notmuch_thread_create (void *ctx,
 	if (doc_id == seed_doc_id)
 	    message = seed_message;
 
-	_thread_add_message (thread, message, exclude_terms);
+	_thread_add_message (thread, message, exclude_terms, include_recipients);
 
 	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, include_recipients);
 	}
 
 	_notmuch_message_close (message);
@@ -499,6 +531,7 @@ _notmuch_thread_create (void *ctx,
     notmuch_query_destroy (thread_id_query);
 
     _resolve_thread_addresses_string (thread->authors);
+    _resolve_thread_addresses_string (thread->recipients);
 
     _resolve_thread_relationships (thread);
 
@@ -536,6 +569,12 @@ notmuch_thread_get_authors (notmuch_thread_t *thread)
 }
 
 const char *
+notmuch_thread_get_recipients (notmuch_thread_t *thread)
+{
+    return thread->recipients->string;
+}
+
+const char *
 notmuch_thread_get_subject (notmuch_thread_t *thread)
 {
     return thread->subject;
-- 
1.7.10.4

[PATCH 07/11] test: search recipient output

[Jameson Graef Rollins]

Simple json test for a new additional recipient field.  known_broken
to be removed with next patch.
---
 test/json |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/test/json b/test/json
index ac8fa8e..ac423a8 100755
--- a/test/json
+++ b/test/json
@@ -60,4 +60,18 @@ test_expect_equal_json "$output" "[{\"thread\": \"XXX\",
  \"tags\": [\"inbox\",
  \"unread\"]}]"
 
+test_begin_subtest "Search message: include recipients"
+test_subtest_known_broken
+output=$(notmuch search --format=json --include-recipients "json-search-message" | notmuch_search_sanitize)
+test_expect_equal_json "$output" "[{\"thread\": \"XXX\",
+ \"timestamp\": 946728000,
+ \"date_relative\": \"2000-01-01\",
+ \"matched\": 1,
+ \"total\": 1,
+ \"authors\": \"Notmuch Test Suite\",
+ \"recipients\": \"Notmuch Test Suite\",
+ \"subject\": \"json-search-subject\",
+ \"tags\": [\"inbox\",
+ \"unread\"]}]"
+
 test_done
-- 
1.7.10.4

[PATCH 08/11] cli: add thread recipients to search output

[Jameson Graef Rollins]

This adds a "--include-recipients" option to notmuch search.  With
structured output formats (e.g. json), a new recipients field will be
included that holds recipients of the thread.  Matched and non-matched
recipients are delineated as with authors.

As mentioned in the previous patch for the underlying lib functions,
the need for the option is because message recipients are not stored
in the database and therefore retrieving them adds a significant
overhead.  If they were included, this option would not be necessary.
---
 lib/notmuch.h    |    6 +++++-
 lib/query.cc     |    5 +++--
 notmuch-search.c |   14 +++++++++++---
 notmuch-show.c   |    2 +-
 test/json        |    1 -
 5 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/lib/notmuch.h b/lib/notmuch.h
index f9e71c1..8eb455e 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -642,6 +642,9 @@ notmuch_threads_valid (notmuch_threads_t *threads);
 
 /* Get the current thread from 'threads' as a notmuch_thread_t.
  *
+ * If the include_recipients flag is TRUE, thread recipients will be
+ * included in the returned thread object.
+ *
  * Note: The returned thread belongs to 'threads' and has a lifetime
  * identical to it (and the query to which it belongs).
  *
@@ -652,7 +655,8 @@ notmuch_threads_valid (notmuch_threads_t *threads);
  * NULL.
  */
 notmuch_thread_t *
-notmuch_threads_get (notmuch_threads_t *threads);
+notmuch_threads_get (notmuch_threads_t *threads,
+		     notmuch_bool_t include_recipients);
 
 /* Move the 'threads' iterator to the next thread.
  *
diff --git a/lib/query.cc b/lib/query.cc
index 54833a7..0a4f058 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -472,7 +472,8 @@ notmuch_threads_valid (notmuch_threads_t *threads)
 }
 
 notmuch_thread_t *
-notmuch_threads_get (notmuch_threads_t *threads)
+notmuch_threads_get (notmuch_threads_t *threads,
+		     notmuch_bool_t include_recipients)
 {
     unsigned int doc_id;
 
@@ -487,7 +488,7 @@ notmuch_threads_get (notmuch_threads_t *threads)
 				   &threads->match_set,
 				   threads->query->exclude_terms,
 				   threads->query->sort,
-				   FALSE);
+				   include_recipients);
 }
 
 void
diff --git a/notmuch-search.c b/notmuch-search.c
index 830c4e4..f610a84 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -52,7 +52,8 @@ do_search_threads (sprinter_t *format,
 		   notmuch_sort_t sort,
 		   output_t output,
 		   int offset,
-		   int limit)
+		   int limit,
+		   notmuch_bool_t include_recipients)
 {
     notmuch_thread_t *thread;
     notmuch_threads_t *threads;
@@ -76,7 +77,7 @@ do_search_threads (sprinter_t *format,
 	 notmuch_threads_valid (threads) && (limit < 0 || i < offset + limit);
 	 notmuch_threads_move_to_next (threads), i++)
     {
-	thread = notmuch_threads_get (threads);
+	thread = notmuch_threads_get (threads, include_recipients);
 
 	if (i < offset) {
 	    notmuch_thread_destroy (thread);
@@ -91,6 +92,7 @@ do_search_threads (sprinter_t *format,
 	} else { /* output == OUTPUT_SUMMARY */
 	    void *ctx_quote = talloc_new (thread);
 	    const char *authors = notmuch_thread_get_authors (thread);
+	    const char *recipients = notmuch_thread_get_recipients (thread);
 	    const char *subject = notmuch_thread_get_subject (thread);
 	    const char *thread_id = notmuch_thread_get_thread_id (thread);
 	    int matched = notmuch_thread_get_matched_messages (thread);
@@ -129,6 +131,10 @@ do_search_threads (sprinter_t *format,
 		format->integer (format, total);
 		format->map_key (format, "authors");
 		format->string (format, authors);
+		if (include_recipients) {
+		    format->map_key (format, "recipients");
+		    format->string (format, recipients);
+		}
 		format->map_key (format, "subject");
 		format->string (format, subject);
 	    }
@@ -303,6 +309,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
     int offset = 0;
     int limit = -1; /* unlimited */
     int exclude = EXCLUDE_TRUE;
+    notmuch_bool_t include_recipients = FALSE;
     unsigned int i;
 
     enum { NOTMUCH_FORMAT_JSON, NOTMUCH_FORMAT_TEXT }
@@ -331,6 +338,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
                                   { 0, 0 } } },
 	{ NOTMUCH_OPT_INT, &offset, "offset", 'O', 0 },
 	{ NOTMUCH_OPT_INT, &limit, "limit", 'L', 0  },
+	{ NOTMUCH_OPT_BOOLEAN, &include_recipients, "include-recipients", 'r', 0 },
 	{ 0, 0, 0, 0, 0 }
     };
 
@@ -402,7 +410,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
     default:
     case OUTPUT_SUMMARY:
     case OUTPUT_THREADS:
-	ret = do_search_threads (format, query, sort, output, offset, limit);
+	ret = do_search_threads (format, query, sort, output, offset, limit, include_recipients);
 	break;
     case OUTPUT_MESSAGES:
     case OUTPUT_FILES:
diff --git a/notmuch-show.c b/notmuch-show.c
index 3556293..cc4b428 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -965,7 +965,7 @@ do_show (void *ctx,
 	 notmuch_threads_valid (threads);
 	 notmuch_threads_move_to_next (threads))
     {
-	thread = notmuch_threads_get (threads);
+	thread = notmuch_threads_get (threads, FALSE);
 
 	messages = notmuch_thread_get_toplevel_messages (thread);
 
diff --git a/test/json b/test/json
index ac423a8..f441b59 100755
--- a/test/json
+++ b/test/json
@@ -61,7 +61,6 @@ test_expect_equal_json "$output" "[{\"thread\": \"XXX\",
  \"unread\"]}]"
 
 test_begin_subtest "Search message: include recipients"
-test_subtest_known_broken
 output=$(notmuch search --format=json --include-recipients "json-search-message" | notmuch_search_sanitize)
 test_expect_equal_json "$output" "[{\"thread\": \"XXX\",
  \"timestamp\": 946728000,
-- 
1.7.10.4

[PATCH 09/11] emacs: add ability to show recipients instead of author in search

[Jameson Graef Rollins]

A new boolean argument is added to notmuch-search to control whether
thread authors or recipients are shown in the 'authors' field.
---
 emacs/notmuch-hello.el |    6 +++---
 emacs/notmuch.el       |   40 +++++++++++++++++++++++++++++-----------
 2 files changed, 32 insertions(+), 14 deletions(-)

diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
index 684bedc..4615cf6 100644
--- a/emacs/notmuch-hello.el
+++ b/emacs/notmuch-hello.el
@@ -26,7 +26,7 @@
 (require 'notmuch-lib)
 (require 'notmuch-mua)
 
-(declare-function notmuch-search "notmuch" (query &optional oldest-first target-thread target-line continuation))
+(declare-function notmuch-search "notmuch" (query &optional oldest-first show-recipients target-thread target-line continuation))
 (declare-function notmuch-poll "notmuch" ())
 
 (defcustom notmuch-hello-recent-searches-max 10
@@ -280,7 +280,7 @@ afterwards.")
     (setq search (notmuch-hello-trim search))
     (let ((history-delete-duplicates t))
       (add-to-history 'notmuch-search-history search)))
-  (notmuch-search search notmuch-search-oldest-first nil nil
+  (notmuch-search search notmuch-search-oldest-first nil nil nil
 		  #'notmuch-hello-search-continuation))
 
 (defun notmuch-hello-add-saved-search (widget)
@@ -331,7 +331,7 @@ diagonal."
   (notmuch-search (widget-get widget
 			      :notmuch-search-terms)
 		  notmuch-search-oldest-first
-		  nil nil #'notmuch-hello-search-continuation))
+		  nil nil nil #'notmuch-hello-search-continuation))
 
 (defun notmuch-saved-search-count (search)
   (car (process-lines notmuch-command "count" search)))
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 7b61e9b..d05b1e8 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -254,6 +254,7 @@ For a mouse binding, return nil."
   (notmuch-common-do-stash (notmuch-search-find-thread-id)))
 
 (defvar notmuch-search-query-string)
+(defvar notmuch-search-show-recipients)
 (defvar notmuch-search-target-thread)
 (defvar notmuch-search-target-line)
 (defvar notmuch-search-continuation)
@@ -405,6 +406,7 @@ Complete list of currently available key bindings:
   (make-local-variable 'notmuch-search-oldest-first)
   (make-local-variable 'notmuch-search-target-thread)
   (make-local-variable 'notmuch-search-target-line)
+  (make-local-variable 'notmuch-search-show-recipients)
   (set (make-local-variable 'notmuch-search-continuation) nil)
   (set (make-local-variable 'scroll-preserve-screen-position) t)
   (add-to-invisibility-spec (cons 'ellipsis t))
@@ -781,7 +783,10 @@ non-authors is found, assume that all of the authors match."
 			'face 'notmuch-search-subject)))
 
    ((string-equal field "authors")
-    (notmuch-search-insert-authors format-string (plist-get result :authors)))
+    (notmuch-search-insert-authors format-string
+				   (if notmuch-search-show-recipients
+				       (plist-get result :recipients)
+				     (plist-get result :authors))))
 
    ((string-equal field "tags")
     (let ((tags-str (mapconcat 'identity (plist-get result :tags) " ")))
@@ -931,13 +936,16 @@ PROMPT is the string to prompt with."
 			      'notmuch-search-history nil nil)))))
 
 ;;;###autoload
-(defun notmuch-search (&optional query oldest-first target-thread target-line continuation)
+(defun notmuch-search (&optional query oldest-first show-recipients target-thread target-line continuation)
   "Run \"notmuch search\" with the given `query' and display results.
 
 If `query' is nil, it is read interactively from the minibuffer.
 Other optional parameters are used as follows:
 
   oldest-first: A Boolean controlling the sort order of returned threads
+  show-recipients: A Boolean controlling whether or not thread
+                   recipients instead of authors are shown in the
+                   'authors' field.
   target-thread: A thread ID (without the thread: prefix) that will be made
                  current if it appears in the search results.
   target-line: The line number to move to if the target thread does not
@@ -952,25 +960,34 @@ Other optional parameters are used as follows:
     (set 'buffer-undo-list t)
     (set 'notmuch-search-query-string query)
     (set 'notmuch-search-oldest-first oldest-first)
+    (set 'notmuch-search-show-recipients show-recipients)
     (set 'notmuch-search-target-thread target-thread)
     (set 'notmuch-search-target-line target-line)
     (set 'notmuch-search-continuation continuation)
     (let ((proc (get-buffer-process (current-buffer)))
+	  (proc-args (list
+		      "notmuch-search" buffer
+		      notmuch-command "search"
+		      "--format=json"
+		      (if oldest-first
+			  "--sort=oldest-first"
+			"--sort=newest-first")))
 	  (inhibit-read-only t))
+      (if show-recipients
+	  (setq proc-args (append proc-args '("--include-recipients"))))
       (if proc
 	  (error "notmuch search process already running for query `%s'" query)
 	)
       (erase-buffer)
       (goto-char (point-min))
       (save-excursion
-	(let ((proc (start-process
-		     "notmuch-search" buffer
-		     notmuch-command "search"
-		     "--format=json"
-		     (if oldest-first
-			 "--sort=oldest-first"
-		       "--sort=newest-first")
-		     query))
+	(let (
+	      ;; start-process insists on non-nil string arguments.
+	      ;; This is annoying for variable length argument lists.
+	      ;; We use apply here so that we can construct the
+	      ;; start-process argument list ahead of time (instead of
+	      ;; at invocation) to avoid nils.
+	      (proc (apply 'start-process (append proc-args (list query))))
 	      ;; Use a scratch buffer to accumulate partial output.
 	      ;; This buffer will be killed by the sentinel, which
 	      ;; should be called no matter how the process dies.
@@ -995,11 +1012,12 @@ same relative position within the new buffer."
   (interactive)
   (let ((target-line (line-number-at-pos))
 	(oldest-first notmuch-search-oldest-first)
+	(show-recipients notmuch-search-show-recipients)
 	(target-thread (notmuch-search-find-thread-id 'bare))
 	(query notmuch-search-query-string)
 	(continuation notmuch-search-continuation))
     (notmuch-kill-this-buffer)
-    (notmuch-search query oldest-first target-thread target-line continuation)
+    (notmuch-search query oldest-first show-recipients target-thread target-line continuation)
     (goto-char (point-min))))
 
 (defcustom notmuch-poll-script nil
-- 
1.7.10.4

[PATCH 10/11] emacs: add function to toggle showing authors/recipients in search

[Jameson Graef Rollins]

New function, notmuch-search-toggle-tofrom, toggles whether thread
authors or recipients are shown in the 'authors' fields in the search
results.  This is bound to the '/' key.
---
 emacs/notmuch.el |   11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index d05b1e8..ca16583 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -226,6 +226,7 @@ For a mouse binding, return nil."
     (define-key map "m" 'notmuch-mua-new-mail)
     (define-key map "s" 'notmuch-search)
     (define-key map "o" 'notmuch-search-toggle-order)
+    (define-key map "/" 'notmuch-search-toggle-tofrom)
     (define-key map "c" 'notmuch-search-stash-map)
     (define-key map "=" 'notmuch-search-refresh-view)
     (define-key map "G" 'notmuch-search-poll-and-refresh-view)
@@ -1082,6 +1083,16 @@ search."
   (set 'notmuch-search-oldest-first (not notmuch-search-oldest-first))
   (notmuch-search-refresh-view))
 
+(defun notmuch-search-toggle-tofrom ()
+  "Toggle showing thread authors or recipients.
+
+By default, search results display the authors of the threads in
+the \"authors\" field.  This command toggles the display of
+thread recipients in that field instead."
+  (interactive)
+  (set 'notmuch-search-show-recipients (not notmuch-search-show-recipients))
+  (notmuch-search-refresh-view))
+
 (defun notmuch-search-filter (query)
   "Filter the current search results based on an additional query string.
 
-- 
1.7.10.4

[PATCH 11/11] lib: add recipients to database

[Jameson Graef Rollins]

This adds just the "to" recipients, but probably "cc"s should be
included as well.
---
 lib/database.cc       |    2 +-
 lib/message.cc        |    4 ++++
 lib/notmuch-private.h |    2 ++
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/lib/database.cc b/lib/database.cc
index 761dc1a..4c1d578 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -1814,7 +1814,7 @@ notmuch_database_add_message (notmuch_database_t *notmuch,
 		goto DONE;
 
 	    date = notmuch_message_file_get_header (message_file, "date");
-	    _notmuch_message_set_header_values (message, date, from, subject);
+	    _notmuch_message_set_header_values (message, date, from, to, subject);
 
 	    _notmuch_message_index_file (message, filename);
 	} else {
diff --git a/lib/message.cc b/lib/message.cc
index fa28073..cc5c8a0 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -420,6 +420,8 @@ notmuch_message_get_header (notmuch_message_t *message, const char *header)
      * available */
     if (strcasecmp (header, "from") == 0)
 	value = message->doc.get_value (NOTMUCH_VALUE_FROM);
+    if (strcasecmp (header, "to") == 0)
+	value = message->doc.get_value (NOTMUCH_VALUE_TO);
     else if (strcasecmp (header, "subject") == 0)
 	value = message->doc.get_value (NOTMUCH_VALUE_SUBJECT);
     else if (strcasecmp (header, "message-id") == 0)
@@ -830,6 +832,7 @@ void
 _notmuch_message_set_header_values (notmuch_message_t *message,
 				    const char *date,
 				    const char *from,
+				    const char *to,
 				    const char *subject)
 {
     time_t time_value;
@@ -844,6 +847,7 @@ _notmuch_message_set_header_values (notmuch_message_t *message,
     message->doc.add_value (NOTMUCH_VALUE_TIMESTAMP,
 			    Xapian::sortable_serialise (time_value));
     message->doc.add_value (NOTMUCH_VALUE_FROM, from);
+    message->doc.add_value (NOTMUCH_VALUE_TO, to);
     message->doc.add_value (NOTMUCH_VALUE_SUBJECT, subject);
 }
 
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 32d1523..f56c580 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -95,6 +95,7 @@ typedef enum {
     NOTMUCH_VALUE_TIMESTAMP = 0,
     NOTMUCH_VALUE_MESSAGE_ID,
     NOTMUCH_VALUE_FROM,
+    NOTMUCH_VALUE_TO,
     NOTMUCH_VALUE_SUBJECT
 } notmuch_value_t;
 
@@ -291,6 +292,7 @@ void
 _notmuch_message_set_header_values (notmuch_message_t *message,
 				    const char *date,
 				    const char *from,
+				    const char *to,
 				    const char *subject);
 void
 _notmuch_message_sync (notmuch_message_t *message);
-- 
1.7.10.4

Re: [PATCH 00/11] add recipients to search output

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 225 bytes --]

Apparently a recent addition to master is having a bad interaction with
this series that breaks notmuch-reply.  I'll try to fix and send a new
version.

In the mean time, comments on this series would be appreciated.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH 00/11] add recipients to search output

[Tomi Ollila]

On Mon, Aug 20 2012, Jameson Graef Rollins wrote:

> This series is an attempt to add thread recipients to the search
> output.
>
> My personal overall goal of this series is to support the handling of
> drafts in the emacs ui.  For drafts we want to see recipients, instead
> of authors, in the search output.  I can imagine other uses for this
> series as well, though.
>
> The first four patches generalize the author list handling in thread
> objects to handle any address list.  These patches could be applied
> regardless of if the rest of the series is accepted.
>
> After that we modify the thread constructor such that it can hold
> thread recipients as well.  Since there is overhead in retrieving
> thread recipients from the message files (recipients are not stored in
> the database) this is handled with a switch.
>
> Further patches add the new switch to the search CLI that adds thread
> recipients to the structured output formats.  I didn't modify the text
> output format, since there is no way to extend it.  I can imagine
> tweaking the text output such that the author field is instead
> replaced by the recipients (as is done for the emacs UI at the end of
> the series), but that's not done here.
>
> In the emacs UI, I add a new toggle function that will toggle display
> of thread authors or recipients in the 'authors' field of the search
> output.  It's unfortunate that this ambiguity in that field name
> remains, but I didn't know how to change that cleanly.  I'm working on
> some tests for the new emacs functionality that I'll include in the
> inevitable v2 of this series.

I did not read much of this introduction before browsing to the code, I
was about to comment whether attempt yo do less trivial tests are
to be done.

> The last patch is mostly just a tickle to suggest adding the
> recipients to the database.  It would make the --include-recipient
> searches much faster of course, but it might be overhead in the
> database that folks aren't interested in.

I got tickled... adding To (and Cc?!) to the database would also give
(future notmuch?) address completion more addresses to match for.

We should discuss whether to add other headers too? IIRC someone (Austin?)
mentioned that everything (except Received:) headers could be there ?

> As always, feedback, review, and comments are much appreciated.

Overall, the code looks good (to me).

> jamie.

Tomi

Re: [PATCH 01/11] lib: new thread addresses structure

[Michal Sojka]

Hi Jameson,

some comments below.

On Mon, Aug 20 2012, Jameson Graef Rollins wrote:
> This new structure holds addresses associated with a thread, both
> matched and unmatched.  Initially this will be used to replace the
> existing infrastructure for storing the addresses of thread authors.
> Further patches will use it to store the addresses of threads
> recipients.
>
> Init and destructor functions are included, as well as a function to
> add addresses to a struct, either "matched" or not.
> ---
>  lib/notmuch.h |    1 +
>  lib/thread.cc |  116 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 117 insertions(+)
>
> diff --git a/lib/notmuch.h b/lib/notmuch.h
> index 3633bed..6acd38d 100644
> --- a/lib/notmuch.h
> +++ b/lib/notmuch.h
> @@ -118,6 +118,7 @@ typedef struct _notmuch_database notmuch_database_t;
>  typedef struct _notmuch_query notmuch_query_t;
>  typedef struct _notmuch_threads notmuch_threads_t;
>  typedef struct _notmuch_thread notmuch_thread_t;
> +typedef struct _notmuch_thread_addresses notmuch_thread_addresses_t;
>  typedef struct _notmuch_messages notmuch_messages_t;
>  typedef struct _notmuch_message notmuch_message_t;
>  typedef struct _notmuch_tags notmuch_tags_t;
> diff --git a/lib/thread.cc b/lib/thread.cc
> index e976d64..7af9eeb 100644
> --- a/lib/thread.cc
> +++ b/lib/thread.cc
> @@ -24,6 +24,14 @@
>  #include <gmime/gmime.h>
>  #include <glib.h> /* GHashTable */
>  
> +struct visible _notmuch_thread_addresses {
> +    GHashTable *matched_hash;
> +    GPtrArray *matched_array;
> +    GHashTable *unmatched_hash;
> +    GPtrArray *unmatched_array;
> +    char *string;
> +};
> +
>  struct visible _notmuch_thread {
>      notmuch_database_t *notmuch;
>      char *thread_id;
> @@ -44,6 +52,18 @@ struct visible _notmuch_thread {
>  };
>  
>  static int
> +_notmuch_thread_addresses_destructor (notmuch_thread_addresses_t *addresses)
> +{
> +    g_hash_table_unref (addresses->matched_hash);
> +    g_hash_table_unref (addresses->unmatched_hash);
> +    g_ptr_array_free (addresses->matched_array, TRUE);
> +    g_ptr_array_free (addresses->unmatched_array, TRUE);
> +    addresses->matched_array = NULL;
> +    addresses->unmatched_array = NULL;
> +    return 0;
> +}
> +
> +static int
>  _notmuch_thread_destructor (notmuch_thread_t *thread)
>  {
>      g_hash_table_unref (thread->authors_hash);
> @@ -64,6 +84,81 @@ _notmuch_thread_destructor (notmuch_thread_t *thread)
>      return 0;
>  }
>  
> +/* Add address to a thread addresses struct.  If matched is TRUE, then
> + * the address will be added to the matched list.*/
> +static void
> +_thread_add_address (notmuch_thread_addresses_t *addresses,
> +		     const char *address,
> +		     notmuch_bool_t matched)
> +{
> +    char *address_copy;
> +    GHashTable *hash;
> +    GPtrArray *array;
> +
> +    if (matched) {
> +	hash = addresses->matched_hash;
> +	array = addresses->matched_array;
> +    } else {
> +	hash = addresses->unmatched_hash;
> +	array = addresses->unmatched_array;
> +    }
> +
> +    if (address == NULL)
> +	return;
> +
> +    if (g_hash_table_lookup_extended (hash, address, NULL, NULL))
> +	return;
> +
> +    address_copy = talloc_strdup (addresses, address);
> +
> +    g_hash_table_insert (hash, address_copy, NULL);
> +
> +    g_ptr_array_add (array, address_copy);
> +}
> +
> +/* Construct an addresses string from matched and unmatched addresses
> + * in notmuch_thread_addresses_t. The string contains matched
> + * addresses first, then non-matched addresses (with the two groups
> + * separated by '|'). Within each group, addresses are listed in date
> + * order. */

I'd say the the addresses are listed in the order in which they have
been added, which might or might not be the date order.

> +static void
> +_resolve_thread_addresses_string (notmuch_thread_addresses_t *addresses)
> +{
> +    unsigned int i;
> +    char *address;
> +    int first_non_matched_address = 1;
> +
> +    /* First, list all matched addressses in date order. */
> +    for (i = 0; i < addresses->matched_array->len; i++) {
> +	address = (char *) g_ptr_array_index (addresses->matched_array, i);
> +	if (addresses->string)
> +	    addresses->string = talloc_asprintf (addresses, "%s, %s",
> +						 addresses->string,
> +						 address);
> +	else
> +	    addresses->string = address;
> +    }
> +
> +    /* Next, append any non-matched addresses that haven't already appeared. */
> +    for (i = 0; i < addresses->unmatched_array->len; i++) {
> +	address = (char *) g_ptr_array_index (addresses->unmatched_array, i);
> +	if (g_hash_table_lookup_extended (addresses->matched_hash,
> +					  address, NULL, NULL))
> +	    continue;
> +	if (first_non_matched_address) {
> +	    addresses->string = talloc_asprintf (addresses, "%s| %s",
> +						 addresses->string,
> +						 address);
> +	} else {
> +	    addresses->string = talloc_asprintf (addresses, "%s, %s",
> +						 addresses->string,
> +						 address);
> +	}

Simpler would be:

addresses->string = talloc_asprintf (addresses, "%s%c %s",
                                     addresses->string,
                                     first_non_matched_address ? '|' : ','
                                     address);

Also, you might want to talloc_free the old address->string to not waste
memory in the case of long lived notmuch_thread_addresses_t object. Or
better use talloc_asprintf_append() function, which hopefully implements
freeing internally.

-Michal

> +
> +	first_non_matched_address = 0;
> +    }
> +}
> +
>  /* Add each author of the thread to the thread's authors_hash and to
>   * the thread's authors_array. */
>  static void
> @@ -382,6 +477,27 @@ _resolve_thread_relationships (unused (notmuch_thread_t *thread))
>       */
>  }
>  
> +/* Initialize a thread addresses struct. */
> +notmuch_thread_addresses_t *
> +_thread_addresses_init (const void *ctx)
> +{
> +    notmuch_thread_addresses_t *addresses;
> +
> +    addresses = talloc (ctx, notmuch_thread_addresses_t);
> +    if (unlikely (addresses == NULL))
> +	return NULL;
> +
> +    addresses->matched_hash = g_hash_table_new_full (g_str_hash, g_str_equal,
> +						     NULL, NULL);
> +    addresses->matched_array = g_ptr_array_new ();
> +    addresses->unmatched_hash = g_hash_table_new_full (g_str_hash, g_str_equal,
> +						       NULL, NULL);
> +    addresses->unmatched_array = g_ptr_array_new ();
> +    addresses->string = NULL;
> +
> +    return addresses;
> +}
> +
>  /* Create a new notmuch_thread_t object by finding the thread
>   * containing the message with the given doc ID, treating any messages
>   * contained in match_set as "matched".  Remove all messages in the
> -- 
> 1.7.10.4

Re: [PATCH 01/11] lib: new thread addresses structure

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 2689 bytes --]

Hey, Michal.  Thanks for the review.

On Thu, Aug 30 2012, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
>> +/* Construct an addresses string from matched and unmatched addresses
>> + * in notmuch_thread_addresses_t. The string contains matched
>> + * addresses first, then non-matched addresses (with the two groups
>> + * separated by '|'). Within each group, addresses are listed in date
>> + * order. */
>
> I'd say the the addresses are listed in the order in which they have
> been added, which might or might not be the date order.

I actually did not write the _resolve_thread_addresses_string function.
I left the logic and behavior unchanged, and just modified to use the
new addresses structure.  If we think the logic should be changed, maybe
we should do that in a separate patch.

>> +static void
>> +_resolve_thread_addresses_string (notmuch_thread_addresses_t *addresses)
>> +{
>> +    unsigned int i;
>> +    char *address;
>> +    int first_non_matched_address = 1;
>> +
>> +    /* First, list all matched addressses in date order. */
>> +    for (i = 0; i < addresses->matched_array->len; i++) {
>> +	address = (char *) g_ptr_array_index (addresses->matched_array, i);
>> +	if (addresses->string)
>> +	    addresses->string = talloc_asprintf (addresses, "%s, %s",
>> +						 addresses->string,
>> +						 address);
>> +	else
>> +	    addresses->string = address;
>> +    }
>> +
>> +    /* Next, append any non-matched addresses that haven't already appeared. */
>> +    for (i = 0; i < addresses->unmatched_array->len; i++) {
>> +	address = (char *) g_ptr_array_index (addresses->unmatched_array, i);
>> +	if (g_hash_table_lookup_extended (addresses->matched_hash,
>> +					  address, NULL, NULL))
>> +	    continue;
>> +	if (first_non_matched_address) {
>> +	    addresses->string = talloc_asprintf (addresses, "%s| %s",
>> +						 addresses->string,
>> +						 address);
>> +	} else {
>> +	    addresses->string = talloc_asprintf (addresses, "%s, %s",
>> +						 addresses->string,
>> +						 address);
>> +	}
>
> Simpler would be:
>
> addresses->string = talloc_asprintf (addresses, "%s%c %s",
>                                      addresses->string,
>                                      first_non_matched_address ? '|' : ','
>                                      address);
>
> Also, you might want to talloc_free the old address->string to not waste
> memory in the case of long lived notmuch_thread_addresses_t object. Or
> better use talloc_asprintf_append() function, which hopefully implements
> freeing internally.

This looks like a nice simplification, but see comment above.

I'll look into including these changes in the next version of the
series.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH 05/11] lib: add ability to store recipients in message structure

[Michal Sojka]

Hi Jamie,

continuing in the review from yesterday...

On Mon, Aug 20 2012, Jameson Graef Rollins wrote:

[...]

> diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
> index bfb4111..27a41b6 100644
> --- a/lib/notmuch-private.h
> +++ b/lib/notmuch-private.h
> @@ -325,6 +325,15 @@ notmuch_message_set_author (notmuch_message_t *message, const char *author);
>  const char *
>  notmuch_message_get_author (notmuch_message_t *message);
>  
> +/* Set the recipients of 'message' - this is the representation used
> + * when displaying the message */
> +void
> +notmuch_message_set_recipients (notmuch_message_t *message, const char *recipients);
> +
> +/* Get the authors of 'message' */

s/authors/recipients

> +const char *
> +notmuch_message_get_recipients (notmuch_message_t *message);
> +
>  
>  /* index.cc */
>  
> -- 
> 1.7.10.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 06/11] lib: store thread recipients in thread structure

[Michal Sojka]

On Mon, Aug 20 2012, Jameson Graef Rollins wrote:
> This utilizes the new thread addresses struct to store thread
> recipients, again in parallel to authors.
>
> Since message recipients are not stored in the database, including
> recipients in the thread structure exacts a significant overhead as
> the recipients are retrieved from the original message files.  Because
> of this, a new boolean argument, include_recipients, is added to the
> necessary functions (_notmuch_thread_create, _thread_add_message and
> _thread_add_matched_message) that controls whether the recipients are
> fetched and included.  If message recipients are ever stored in the
> database this new argument could probably be removed.
> ---
>  lib/notmuch-private.h |    3 +-
>  lib/notmuch.h         |   14 +++++++++
>  lib/query.cc          |    3 +-
>  lib/thread.cc         |   77 +++++++++++++++++++++++++++++++++++++------------
>  4 files changed, 76 insertions(+), 21 deletions(-)
>
> diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
> index 27a41b6..32d1523 100644
> --- a/lib/notmuch-private.h
> +++ b/lib/notmuch-private.h
> @@ -232,7 +232,8 @@ _notmuch_thread_create (void *ctx,
>  			unsigned int seed_doc_id,
>  			notmuch_doc_id_set_t *match_set,
>  			notmuch_string_list_t *excluded_terms,
> -			notmuch_sort_t sort);
> +			notmuch_sort_t sort,
> +			notmuch_bool_t include_recipients);
>  
>  /* message.cc */
>  
> diff --git a/lib/notmuch.h b/lib/notmuch.h
> index 6acd38d..f9e71c1 100644
> --- a/lib/notmuch.h
> +++ b/lib/notmuch.h
> @@ -759,6 +759,20 @@ notmuch_thread_get_matched_messages (notmuch_thread_t *thread);
>  const char *
>  notmuch_thread_get_authors (notmuch_thread_t *thread);
>  
> +/* Get the recipients of 'thread'
> + *
> + * The returned string is a comma-separated list of the names of the
> + * recipients of mail messages in the query results that belong to this
> + * thread.
> + *
> + * The returned string belongs to 'thread' and as such, should not be
> + * modified by the caller and will only be valid for as long as the
> + * thread is valid, (which is until notmuch_thread_destroy or until
> + * the query from which it derived is destroyed).
> + */
> +const char *
> +notmuch_thread_get_recipients (notmuch_thread_t *thread);
> +
>  /* Get the subject of 'thread'
>   *
>   * The subject is taken from the first message (according to the query
> diff --git a/lib/query.cc b/lib/query.cc
> index e9c1a2d..54833a7 100644
> --- a/lib/query.cc
> +++ b/lib/query.cc
> @@ -486,7 +486,8 @@ notmuch_threads_get (notmuch_threads_t *threads)
>  				   doc_id,
>  				   &threads->match_set,
>  				   threads->query->exclude_terms,
> -				   threads->query->sort);
> +				   threads->query->sort,
> +				   FALSE);
>  }
>  
>  void
> diff --git a/lib/thread.cc b/lib/thread.cc
> index 757e143..baf07c2 100644
> --- a/lib/thread.cc
> +++ b/lib/thread.cc
> @@ -37,6 +37,7 @@ struct visible _notmuch_thread {
>      char *thread_id;
>      char *subject;
>      notmuch_thread_addresses_t *authors;
> +    notmuch_thread_addresses_t *recipients;
>      GHashTable *tags;
>  
>      notmuch_message_list_t *message_list;
> @@ -63,6 +64,7 @@ static int
>  _notmuch_thread_destructor (notmuch_thread_t *thread)
>  {
>      _notmuch_thread_addresses_destructor (thread->authors);
> +    _notmuch_thread_addresses_destructor (thread->recipients);
>      g_hash_table_unref (thread->tags);
>      g_hash_table_unref (thread->message_hash);
>      return 0;
> @@ -204,14 +206,17 @@ _thread_cleanup_address (notmuch_thread_t *thread,
>  static void
>  _thread_add_message (notmuch_thread_t *thread,
>  		     notmuch_message_t *message,
> -		     notmuch_string_list_t *exclude_terms)
> +		     notmuch_string_list_t *exclude_terms,
> +		     notmuch_bool_t include_recipients)
>  {
>      notmuch_tags_t *tags;
>      const char *tag;
> -    InternetAddressList *list = NULL;
> +    InternetAddressList *from_list = NULL;
> +    InternetAddressList *to_list = NULL;
>      InternetAddress *address;
>      const char *from, *author;
> -    char *clean_author;
> +    const char *to, *recipient;
> +    char *clean_address;
>  
>      _notmuch_message_list_add_message (thread->message_list,
>  				       talloc_steal (thread, message));
> @@ -223,10 +228,9 @@ _thread_add_message (notmuch_thread_t *thread,
>  
>      from = notmuch_message_get_header (message, "from");
>      if (from)
> -	list = internet_address_list_parse_string (from);
> -
> -    if (list) {
> -	address = internet_address_list_get_address (list, 0);
> +	from_list = internet_address_list_parse_string (from);
> +    if (from_list) {
> +	address = internet_address_list_get_address (from_list, 0);
>  	if (address) {
>  	    author = internet_address_get_name (address);
>  	    if (author == NULL) {
> @@ -234,11 +238,32 @@ _thread_add_message (notmuch_thread_t *thread,
>  		mailbox = INTERNET_ADDRESS_MAILBOX (address);
>  		author = internet_address_mailbox_get_addr (mailbox);
>  	    }
> -	    clean_author = _thread_cleanup_author (thread, author, from);
> -	    _thread_add_address (thread->authors, clean_author, FALSE);
> -	    notmuch_message_set_author (message, clean_author);
> +	    clean_address = _thread_cleanup_address (thread, author, from);
> +	    _thread_add_address (thread->authors, clean_address, FALSE);
> +	    notmuch_message_set_author (message, clean_address);
> +	}
> +	g_object_unref (G_OBJECT (from_list));
> +    }
> +
> +    if (include_recipients) {
> +    to = notmuch_message_get_header (message, "to");

I think that recipients should be composed from all to, cc and bcc
headers. I often write emails with only Bcc header. If such an email is
saved as draft, I might be surprised by not seeing the "recipients".

> +    if (to)
> +	to_list = internet_address_list_parse_string (to);
> +    if (to_list) {
> +	address = internet_address_list_get_address (to_list, 0);
> +	if (address) {
> +	    recipient = internet_address_get_name (address);
> +	    if (recipient == NULL) {
> +		InternetAddressMailbox *mailbox;
> +		mailbox = INTERNET_ADDRESS_MAILBOX (address);
> +		recipient = internet_address_mailbox_get_addr (mailbox);
> +	    }
> +	    clean_address = _thread_cleanup_address (thread, recipient, to);
> +	    _thread_add_address (thread->recipients, clean_address, FALSE);
> +	    notmuch_message_set_recipients (message, clean_address);
>  	}
> -	g_object_unref (G_OBJECT (list));
> +	g_object_unref (G_OBJECT (to_list));
> +    }
>      }
>  
>      if (! thread->subject) {
> @@ -301,7 +326,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 include_recipients)
>  {
>      time_t date;
>      notmuch_message_t *hashed_message;
> @@ -331,6 +357,8 @@ _thread_add_matched_message (notmuch_thread_t *thread,
>      }
>  
>      _thread_add_address (thread->authors, notmuch_message_get_author (hashed_message), TRUE);
> +    if (include_recipients)
> +    _thread_add_address (thread->recipients, notmuch_message_get_recipients (hashed_message), TRUE);
>  }
>  
>  static void
> @@ -399,10 +427,10 @@ _thread_addresses_init (const void *ctx)
>   *
>   * Creating the thread will perform a database search to get all
>   * messages belonging to the thread and will get the first subject
> - * line, the total count of messages, and all authors in the thread.
> - * Each message in the thread is checked against match_set to allow
> - * for a separate count of matched messages, and to allow a viewer to
> - * display these messages differently.
> + * line, the total count of messages, and all authors and recipients

... all authors (and optionally recipients).

> + * of the thread.  Each message in the thread is checked against
> + * match_set to allow for a separate count of matched messages, and to
> + * allow a viewer to display these messages differently.
>   *
>   * Here, 'ctx' is talloc context for the resulting thread object.
>   *
> @@ -414,7 +442,8 @@ _notmuch_thread_create (void *ctx,
>  			unsigned int seed_doc_id,
>  			notmuch_doc_id_set_t *match_set,
>  			notmuch_string_list_t *exclude_terms,
> -			notmuch_sort_t sort)
> +			notmuch_sort_t sort,
> +			notmuch_bool_t include_recipients)
>  {
>      notmuch_thread_t *thread;
>      notmuch_message_t *seed_message;
> @@ -453,6 +482,9 @@ _notmuch_thread_create (void *ctx,
>      thread->authors = _thread_addresses_init (thread);
>      if (unlikely (thread->authors == NULL))
>  	return NULL;
> +    thread->recipients = _thread_addresses_init (thread);
> +    if (unlikely (thread->recipients == NULL))
> +	return NULL;
>  
>      thread->tags = g_hash_table_new_full (g_str_hash, g_str_equal,
>  					  free, NULL);
> @@ -486,11 +518,11 @@ _notmuch_thread_create (void *ctx,
>  	if (doc_id == seed_doc_id)
>  	    message = seed_message;
>  
> -	_thread_add_message (thread, message, exclude_terms);
> +	_thread_add_message (thread, message, exclude_terms, include_recipients);
>  
>  	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, include_recipients);
>  	}
>  
>  	_notmuch_message_close (message);
> @@ -499,6 +531,7 @@ _notmuch_thread_create (void *ctx,
>      notmuch_query_destroy (thread_id_query);
>  
>      _resolve_thread_addresses_string (thread->authors);
> +    _resolve_thread_addresses_string (thread->recipients);
>  
>      _resolve_thread_relationships (thread);
>  
> @@ -536,6 +569,12 @@ notmuch_thread_get_authors (notmuch_thread_t *thread)
>  }
>  
>  const char *
> +notmuch_thread_get_recipients (notmuch_thread_t *thread)
> +{
> +    return thread->recipients->string;
> +}
> +
> +const char *
>  notmuch_thread_get_subject (notmuch_thread_t *thread)
>  {
>      return thread->subject;
> -- 
> 1.7.10.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 08/11] cli: add thread recipients to search output

[Michal Sojka]

On Mon, Aug 20 2012, Jameson Graef Rollins wrote:
> This adds a "--include-recipients" option to notmuch search.  With
> structured output formats (e.g. json), a new recipients field will be
> included that holds recipients of the thread.  Matched and non-matched
> recipients are delineated as with authors.
>
> As mentioned in the previous patch for the underlying lib functions,
> the need for the option is because message recipients are not stored
> in the database and therefore retrieving them adds a significant
> overhead.  If they were included, this option would not be necessary.
> ---
>  lib/notmuch.h    |    6 +++++-
>  lib/query.cc     |    5 +++--
>  notmuch-search.c |   14 +++++++++++---
>  notmuch-show.c   |    2 +-
>  test/json        |    1 -
>  5 files changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/lib/notmuch.h b/lib/notmuch.h
> index f9e71c1..8eb455e 100644
> --- a/lib/notmuch.h
> +++ b/lib/notmuch.h
> @@ -642,6 +642,9 @@ notmuch_threads_valid (notmuch_threads_t *threads);
>  
>  /* Get the current thread from 'threads' as a notmuch_thread_t.
>   *
> + * If the include_recipients flag is TRUE, thread recipients will be
> + * included in the returned thread object.
> + *
>   * Note: The returned thread belongs to 'threads' and has a lifetime
>   * identical to it (and the query to which it belongs).
>   *
> @@ -652,7 +655,8 @@ notmuch_threads_valid (notmuch_threads_t *threads);
>   * NULL.
>   */
>  notmuch_thread_t *
> -notmuch_threads_get (notmuch_threads_t *threads);
> +notmuch_threads_get (notmuch_threads_t *threads,
> +		     notmuch_bool_t include_recipients);

What about adding a new function notmuch_threds_get_with_recipients() to
not break public API? It would also make the sources more readable.

-M.

Re: [PATCH 11/11] lib: add recipients to database

[Michal Sojka]

On Mon, Aug 20 2012, Jameson Graef Rollins wrote:
> This adds just the "to" recipients, but probably "cc"s should be
> included as well.
> ---
>  lib/database.cc       |    2 +-
>  lib/message.cc        |    4 ++++
>  lib/notmuch-private.h |    2 ++
>  3 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/lib/database.cc b/lib/database.cc
> index 761dc1a..4c1d578 100644
> --- a/lib/database.cc
> +++ b/lib/database.cc
> @@ -1814,7 +1814,7 @@ notmuch_database_add_message (notmuch_database_t *notmuch,
>  		goto DONE;
>  
>  	    date = notmuch_message_file_get_header (message_file, "date");
> -	    _notmuch_message_set_header_values (message, date, from, subject);
> +	    _notmuch_message_set_header_values (message, date, from, to, subject);
>  
>  	    _notmuch_message_index_file (message, filename);
>  	} else {
> diff --git a/lib/message.cc b/lib/message.cc
> index fa28073..cc5c8a0 100644
> --- a/lib/message.cc
> +++ b/lib/message.cc
> @@ -420,6 +420,8 @@ notmuch_message_get_header (notmuch_message_t *message, const char *header)
>       * available */
>      if (strcasecmp (header, "from") == 0)
>  	value = message->doc.get_value (NOTMUCH_VALUE_FROM);
> +    if (strcasecmp (header, "to") == 0)
> +	value = message->doc.get_value (NOTMUCH_VALUE_TO);
>      else if (strcasecmp (header, "subject") == 0)
>  	value = message->doc.get_value (NOTMUCH_VALUE_SUBJECT);
>      else if (strcasecmp (header, "message-id") == 0)
> @@ -830,6 +832,7 @@ void
>  _notmuch_message_set_header_values (notmuch_message_t *message,
>  				    const char *date,
>  				    const char *from,
> +				    const char *to,
>  				    const char *subject)
>  {
>      time_t time_value;
> @@ -844,6 +847,7 @@ _notmuch_message_set_header_values (notmuch_message_t *message,
>      message->doc.add_value (NOTMUCH_VALUE_TIMESTAMP,
>  			    Xapian::sortable_serialise (time_value));
>      message->doc.add_value (NOTMUCH_VALUE_FROM, from);
> +    message->doc.add_value (NOTMUCH_VALUE_TO, to);
>      message->doc.add_value (NOTMUCH_VALUE_SUBJECT, subject);
>  }
>  
> diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
> index 32d1523..f56c580 100644
> --- a/lib/notmuch-private.h
> +++ b/lib/notmuch-private.h
> @@ -95,6 +95,7 @@ typedef enum {
>      NOTMUCH_VALUE_TIMESTAMP = 0,
>      NOTMUCH_VALUE_MESSAGE_ID,
>      NOTMUCH_VALUE_FROM,
> +    NOTMUCH_VALUE_TO,

It would be definitely useful to add other headers to the database (and
make them searchable). As far as I remember this is on the todo list for
ages. I'm only not sure that the approach of adding every possible
header manually (like in this patch) is a good approach. Emails can
contain arbitrary headers so there would be always some header missing.
I'm not that much familiar with Xapian to figure out how to implement
this.

Otherwise this series looks quite well. I'm only not sure whether to
merge it now or after changing the storage of headers in the database.

Probably, if emacs interface is also extended to use this feature as
part of saved searches or hello-section in a way that I can see the list
of drafts on one click from hello screen. I'd vote for merging this now.

However, I've just looked how does my drafts folder (messages saved with
C-x C-s) looks like and one problem I see there is that draft messages
do not have message ids. Do you have an idea how to implement drafts in
emacs UI?

Thanks,
-Michal

Re: [PATCH 06/11] lib: store thread recipients in thread structure

[Mark Walters]

On Mon, 20 Aug 2012, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> This utilizes the new thread addresses struct to store thread
> recipients, again in parallel to authors.
>
> Since message recipients are not stored in the database, including
> recipients in the thread structure exacts a significant overhead as
> the recipients are retrieved from the original message files.  Because
> of this, a new boolean argument, include_recipients, is added to the
> necessary functions (_notmuch_thread_create, _thread_add_message and
> _thread_add_matched_message) that controls whether the recipients are
> fetched and included.  If message recipients are ever stored in the
> database this new argument could probably be removed.
> ---

Hi

I have briefly looked through the series and overall it looks good.

>  lib/notmuch-private.h |    3 +-
>  lib/notmuch.h         |   14 +++++++++
>  lib/query.cc          |    3 +-
>  lib/thread.cc         |   77 +++++++++++++++++++++++++++++++++++++------------
>  4 files changed, 76 insertions(+), 21 deletions(-)
>
> diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
> index 27a41b6..32d1523 100644
> --- a/lib/notmuch-private.h
> +++ b/lib/notmuch-private.h
> @@ -232,7 +232,8 @@ _notmuch_thread_create (void *ctx,
>  			unsigned int seed_doc_id,
>  			notmuch_doc_id_set_t *match_set,
>  			notmuch_string_list_t *excluded_terms,
> -			notmuch_sort_t sort);
> +			notmuch_sort_t sort,
> +			notmuch_bool_t include_recipients);
>  
>  /* message.cc */
>  
> diff --git a/lib/notmuch.h b/lib/notmuch.h
> index 6acd38d..f9e71c1 100644
> --- a/lib/notmuch.h
> +++ b/lib/notmuch.h
> @@ -759,6 +759,20 @@ notmuch_thread_get_matched_messages (notmuch_thread_t *thread);
>  const char *
>  notmuch_thread_get_authors (notmuch_thread_t *thread);
>  
> +/* Get the recipients of 'thread'
> + *
> + * The returned string is a comma-separated list of the names of the
> + * recipients of mail messages in the query results that belong to this
> + * thread.
> + *
> + * The returned string belongs to 'thread' and as such, should not be
> + * modified by the caller and will only be valid for as long as the
> + * thread is valid, (which is until notmuch_thread_destroy or until
> + * the query from which it derived is destroyed).
> + */
> +const char *
> +notmuch_thread_get_recipients (notmuch_thread_t *thread);
> +
>  /* Get the subject of 'thread'
>   *
>   * The subject is taken from the first message (according to the query
> diff --git a/lib/query.cc b/lib/query.cc
> index e9c1a2d..54833a7 100644
> --- a/lib/query.cc
> +++ b/lib/query.cc
> @@ -486,7 +486,8 @@ notmuch_threads_get (notmuch_threads_t *threads)
>  				   doc_id,
>  				   &threads->match_set,
>  				   threads->query->exclude_terms,
> -				   threads->query->sort);
> +				   threads->query->sort,
> +				   FALSE);
>  }
>  
>  void
> diff --git a/lib/thread.cc b/lib/thread.cc
> index 757e143..baf07c2 100644
> --- a/lib/thread.cc
> +++ b/lib/thread.cc
> @@ -37,6 +37,7 @@ struct visible _notmuch_thread {
>      char *thread_id;
>      char *subject;
>      notmuch_thread_addresses_t *authors;
> +    notmuch_thread_addresses_t *recipients;
>      GHashTable *tags;
>  
>      notmuch_message_list_t *message_list;
> @@ -63,6 +64,7 @@ static int
>  _notmuch_thread_destructor (notmuch_thread_t *thread)
>  {
>      _notmuch_thread_addresses_destructor (thread->authors);
> +    _notmuch_thread_addresses_destructor (thread->recipients);
>      g_hash_table_unref (thread->tags);
>      g_hash_table_unref (thread->message_hash);
>      return 0;
> @@ -204,14 +206,17 @@ _thread_cleanup_address (notmuch_thread_t *thread,
>  static void
>  _thread_add_message (notmuch_thread_t *thread,
>  		     notmuch_message_t *message,
> -		     notmuch_string_list_t *exclude_terms)
> +		     notmuch_string_list_t *exclude_terms,
> +		     notmuch_bool_t include_recipients)
>  {
>      notmuch_tags_t *tags;
>      const char *tag;
> -    InternetAddressList *list = NULL;
> +    InternetAddressList *from_list = NULL;
> +    InternetAddressList *to_list = NULL;
>      InternetAddress *address;
>      const char *from, *author;
> -    char *clean_author;
> +    const char *to, *recipient;
> +    char *clean_address;
>  
>      _notmuch_message_list_add_message (thread->message_list,
>  				       talloc_steal (thread, message));
> @@ -223,10 +228,9 @@ _thread_add_message (notmuch_thread_t *thread,
>  
>      from = notmuch_message_get_header (message, "from");
>      if (from)
> -	list = internet_address_list_parse_string (from);
> -
> -    if (list) {
> -	address = internet_address_list_get_address (list, 0);
> +	from_list = internet_address_list_parse_string (from);
> +    if (from_list) {
> +	address = internet_address_list_get_address (from_list, 0);
>  	if (address) {
>  	    author = internet_address_get_name (address);
>  	    if (author == NULL) {
> @@ -234,11 +238,32 @@ _thread_add_message (notmuch_thread_t *thread,
>  		mailbox = INTERNET_ADDRESS_MAILBOX (address);
>  		author = internet_address_mailbox_get_addr (mailbox);
>  	    }
> -	    clean_author = _thread_cleanup_author (thread, author, from);
> -	    _thread_add_address (thread->authors, clean_author, FALSE);
> -	    notmuch_message_set_author (message, clean_author);
> +	    clean_address = _thread_cleanup_address (thread, author, from);
> +	    _thread_add_address (thread->authors, clean_address, FALSE);
> +	    notmuch_message_set_author (message, clean_address);
> +	}
> +	g_object_unref (G_OBJECT (from_list));
> +    }
> +
> +    if (include_recipients) {
> +    to = notmuch_message_get_header (message, "to");
> +    if (to)
> +	to_list = internet_address_list_parse_string (to);
> +    if (to_list) {
> +	address = internet_address_list_get_address (to_list, 0);

I think you are just adding the first address on the to: line. Is that
deliberate? The comment before notmuch_thread_get_recipients (above)
suggests all recipients are being added, but doing so may make the
string too large to be useful.

Best wishes

Mark



> +	if (address) {
> +	    recipient = internet_address_get_name (address);
> +	    if (recipient == NULL) {
> +		InternetAddressMailbox *mailbox;
> +		mailbox = INTERNET_ADDRESS_MAILBOX (address);
> +		recipient = internet_address_mailbox_get_addr (mailbox);
> +	    }
> +	    clean_address = _thread_cleanup_address (thread, recipient, to);
> +	    _thread_add_address (thread->recipients, clean_address, FALSE);
> +	    notmuch_message_set_recipients (message, clean_address);
>  	}
> -	g_object_unref (G_OBJECT (list));
> +	g_object_unref (G_OBJECT (to_list));
> +    }
>      }
>  
>      if (! thread->subject) {
> @@ -301,7 +326,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 include_recipients)
>  {
>      time_t date;
>      notmuch_message_t *hashed_message;
> @@ -331,6 +357,8 @@ _thread_add_matched_message (notmuch_thread_t *thread,
>      }
>  
>      _thread_add_address (thread->authors, notmuch_message_get_author (hashed_message), TRUE);
> +    if (include_recipients)
> +    _thread_add_address (thread->recipients, notmuch_message_get_recipients (hashed_message), TRUE);
>  }
>  
>  static void
> @@ -399,10 +427,10 @@ _thread_addresses_init (const void *ctx)
>   *
>   * Creating the thread will perform a database search to get all
>   * messages belonging to the thread and will get the first subject
> - * line, the total count of messages, and all authors in the thread.
> - * Each message in the thread is checked against match_set to allow
> - * for a separate count of matched messages, and to allow a viewer to
> - * display these messages differently.
> + * line, the total count of messages, and all authors and recipients
> + * of the thread.  Each message in the thread is checked against
> + * match_set to allow for a separate count of matched messages, and to
> + * allow a viewer to display these messages differently.
>   *
>   * Here, 'ctx' is talloc context for the resulting thread object.
>   *
> @@ -414,7 +442,8 @@ _notmuch_thread_create (void *ctx,
>  			unsigned int seed_doc_id,
>  			notmuch_doc_id_set_t *match_set,
>  			notmuch_string_list_t *exclude_terms,
> -			notmuch_sort_t sort)
> +			notmuch_sort_t sort,
> +			notmuch_bool_t include_recipients)
>  {
>      notmuch_thread_t *thread;
>      notmuch_message_t *seed_message;
> @@ -453,6 +482,9 @@ _notmuch_thread_create (void *ctx,
>      thread->authors = _thread_addresses_init (thread);
>      if (unlikely (thread->authors == NULL))
>  	return NULL;
> +    thread->recipients = _thread_addresses_init (thread);
> +    if (unlikely (thread->recipients == NULL))
> +	return NULL;
>  
>      thread->tags = g_hash_table_new_full (g_str_hash, g_str_equal,
>  					  free, NULL);
> @@ -486,11 +518,11 @@ _notmuch_thread_create (void *ctx,
>  	if (doc_id == seed_doc_id)
>  	    message = seed_message;
>  
> -	_thread_add_message (thread, message, exclude_terms);
> +	_thread_add_message (thread, message, exclude_terms, include_recipients);
>  
>  	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, include_recipients);
>  	}
>  
>  	_notmuch_message_close (message);
> @@ -499,6 +531,7 @@ _notmuch_thread_create (void *ctx,
>      notmuch_query_destroy (thread_id_query);
>  
>      _resolve_thread_addresses_string (thread->authors);
> +    _resolve_thread_addresses_string (thread->recipients);
>  
>      _resolve_thread_relationships (thread);
>  
> @@ -536,6 +569,12 @@ notmuch_thread_get_authors (notmuch_thread_t *thread)
>  }
>  
>  const char *
> +notmuch_thread_get_recipients (notmuch_thread_t *thread)
> +{
> +    return thread->recipients->string;
> +}
> +
> +const char *
>  notmuch_thread_get_subject (notmuch_thread_t *thread)
>  {
>      return thread->subject;
> -- 
> 1.7.10.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 00/11] add recipients to search output

[Austin Clements]

Quoth Jameson Graef Rollins on Aug 19 at  6:52 pm:
> This series is an attempt to add thread recipients to the search
> output.
> 
> My personal overall goal of this series is to support the handling of
> drafts in the emacs ui.  For drafts we want to see recipients, instead
> of authors, in the search output.  I can imagine other uses for this
> series as well, though.
> 
> The first four patches generalize the author list handling in thread
> objects to handle any address list.  These patches could be applied
> regardless of if the rest of the series is accepted.
> 
> After that we modify the thread constructor such that it can hold
> thread recipients as well.  Since there is overhead in retrieving
> thread recipients from the message files (recipients are not stored in
> the database) this is handled with a switch.
> 
> Further patches add the new switch to the search CLI that adds thread
> recipients to the structured output formats.  I didn't modify the text
> output format, since there is no way to extend it.  I can imagine
> tweaking the text output such that the author field is instead
> replaced by the recipients (as is done for the emacs UI at the end of
> the series), but that's not done here.

I've gotten up through patch 8.  Overall I really like this series and
the abstractions you're introducing.  However, I don't think you
should replicate the way the authors list is handled in the CLI.  The
authors list is a kludge inherited from the text format, which had to
invent a syntax for lists, that somehow got baked into the library.
JSON, on the other hand, is very good at lists, and should use them
for the recipients (I would love if it used them for the authors list,
but that'll require an incompatible change, so I'd like to implement
my/some JSON versioning scheme first).  After all, "The string is a
stark data structure and everywhere it is passed there is much
duplication of process. It is a perfect vehicle for hiding
information."

I think treating recipients as a first-class list would lead to a much
cleaner and more general API; it would hard-code less in the library,
though it would put more responsibility on the CLI.  What I'm
imagining is just a single new public API function:
notmuch_thread_get_messages, which returns a notmuch_messages_t of the
thread's messages, in the original query's sort order.  It's
remarkable that we *don't* have this API already.  In this case, it
would give the library user (the CLI) the ability to easily construct
the recipients list however it wants.  JSON list or text string?  No
problem.  Just to or to/cc/bcc?  No problem.  Separating matched and
unmatched or merging them together?  No problem.  Also, since this
would naturally construct the recipients list only when needed, we
wouldn't have to worry about telling the library whether or not to pay
the performance cost, and we could trivially add the headers we end up
using to the database later and get an automatic speed boost.

Relative to your current code, this would require either duplicating a
bit of the matched/unmatched hash table code in the CLI (not perfect,
but you wouldn't need the stringifying function, and there isn't much
other code to that) or moving the thread_addresses abstraction into
util.  _thread_cleanup_author would probably also want to move into
util (which is also completely reasonable since it's a pure leaf
function that depends on nothing).

> In the emacs UI, I add a new toggle function that will toggle display
> of thread authors or recipients in the 'authors' field of the search
> output.  It's unfortunate that this ambiguity in that field name
> remains, but I didn't know how to change that cleanly.  I'm working on
> some tests for the new emacs functionality that I'll include in the
> inevitable v2 of this series.
> 
> The last patch is mostly just a tickle to suggest adding the
> recipients to the database.  It would make the --include-recipient
> searches much faster of course, but it might be overhead in the
> database that folks aren't interested in.
> 
> As always, feedback, review, and comments are much appreciated.
> 
> jamie.

Re: [PATCH 01/11] lib: new thread addresses structure

[Austin Clements]

Quoth Jameson Graef Rollins on Aug 19 at  6:52 pm:
> This new structure holds addresses associated with a thread, both
> matched and unmatched.  Initially this will be used to replace the
> existing infrastructure for storing the addresses of thread authors.
> Further patches will use it to store the addresses of threads
> recipients.
> 
> Init and destructor functions are included, as well as a function to
> add addresses to a struct, either "matched" or not.
> ---
>  lib/notmuch.h |    1 +
>  lib/thread.cc |  116 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 117 insertions(+)
> 
> diff --git a/lib/notmuch.h b/lib/notmuch.h
> index 3633bed..6acd38d 100644
> --- a/lib/notmuch.h
> +++ b/lib/notmuch.h
> @@ -118,6 +118,7 @@ typedef struct _notmuch_database notmuch_database_t;
>  typedef struct _notmuch_query notmuch_query_t;
>  typedef struct _notmuch_threads notmuch_threads_t;
>  typedef struct _notmuch_thread notmuch_thread_t;
> +typedef struct _notmuch_thread_addresses notmuch_thread_addresses_t;
>  typedef struct _notmuch_messages notmuch_messages_t;
>  typedef struct _notmuch_message notmuch_message_t;
>  typedef struct _notmuch_tags notmuch_tags_t;
> diff --git a/lib/thread.cc b/lib/thread.cc
> index e976d64..7af9eeb 100644
> --- a/lib/thread.cc
> +++ b/lib/thread.cc
> @@ -24,6 +24,14 @@
>  #include <gmime/gmime.h>
>  #include <glib.h> /* GHashTable */
>  
> +struct visible _notmuch_thread_addresses {
> +    GHashTable *matched_hash;
> +    GPtrArray *matched_array;
> +    GHashTable *unmatched_hash;
> +    GPtrArray *unmatched_array;
> +    char *string;
> +};
> +
>  struct visible _notmuch_thread {
>      notmuch_database_t *notmuch;
>      char *thread_id;
> @@ -44,6 +52,18 @@ struct visible _notmuch_thread {
>  };
>  
>  static int
> +_notmuch_thread_addresses_destructor (notmuch_thread_addresses_t *addresses)
> +{
> +    g_hash_table_unref (addresses->matched_hash);
> +    g_hash_table_unref (addresses->unmatched_hash);
> +    g_ptr_array_free (addresses->matched_array, TRUE);
> +    g_ptr_array_free (addresses->unmatched_array, TRUE);

The second argument should be FALSE for both of these, since the
pointers contained in these pointer arrays are talloc-managed.

> +    addresses->matched_array = NULL;
> +    addresses->unmatched_array = NULL;

Is this necessary?  (Obviously there's no harm.)

> +    return 0;
> +}
> +
> +static int
>  _notmuch_thread_destructor (notmuch_thread_t *thread)
>  {
>      g_hash_table_unref (thread->authors_hash);
> @@ -64,6 +84,81 @@ _notmuch_thread_destructor (notmuch_thread_t *thread)
>      return 0;
>  }
>  
> +/* Add address to a thread addresses struct.  If matched is TRUE, then
> + * the address will be added to the matched list.*/
> +static void
> +_thread_add_address (notmuch_thread_addresses_t *addresses,

_thread_addresses_add?

> +		     const char *address,
> +		     notmuch_bool_t matched)
> +{
> +    char *address_copy;
> +    GHashTable *hash;
> +    GPtrArray *array;
> +
> +    if (matched) {
> +	hash = addresses->matched_hash;
> +	array = addresses->matched_array;
> +    } else {
> +	hash = addresses->unmatched_hash;
> +	array = addresses->unmatched_array;
> +    }
> +
> +    if (address == NULL)
> +	return;
> +
> +    if (g_hash_table_lookup_extended (hash, address, NULL, NULL))
> +	return;
> +
> +    address_copy = talloc_strdup (addresses, address);
> +
> +    g_hash_table_insert (hash, address_copy, NULL);
> +
> +    g_ptr_array_add (array, address_copy);
> +}
> +
> +/* Construct an addresses string from matched and unmatched addresses
> + * in notmuch_thread_addresses_t. The string contains matched
> + * addresses first, then non-matched addresses (with the two groups
> + * separated by '|'). Within each group, addresses are listed in date
> + * order. */
> +static void
> +_resolve_thread_addresses_string (notmuch_thread_addresses_t *addresses)

A better API for this would be to return a const char*.  On the first
call (when addresses->string is NULL), construct the string and return
it.  On subsequent calls, just immediately return addresses->string.
The approach you're using here makes sense from an incremental
perspective, but since you're introducing an abstraction for thread
address lists, I think it makes more sense to look at it from an API
perspective.

Also, the name should probably start with thread_addresses, since
that's the abstraction it's part of.
_thread_addresses_resolve_string?  _thread_addresses_to_string?

> +{
> +    unsigned int i;
> +    char *address;
> +    int first_non_matched_address = 1;
> +
> +    /* First, list all matched addressses in date order. */

Michal's comment about this not necessarily being date order applies
here, too.

> +    for (i = 0; i < addresses->matched_array->len; i++) {
> +	address = (char *) g_ptr_array_index (addresses->matched_array, i);
> +	if (addresses->string)
> +	    addresses->string = talloc_asprintf (addresses, "%s, %s",
> +						 addresses->string,
> +						 address);
> +	else
> +	    addresses->string = address;
> +    }
> +
> +    /* Next, append any non-matched addresses that haven't already appeared. */
> +    for (i = 0; i < addresses->unmatched_array->len; i++) {
> +	address = (char *) g_ptr_array_index (addresses->unmatched_array, i);
> +	if (g_hash_table_lookup_extended (addresses->matched_hash,
> +					  address, NULL, NULL))
> +	    continue;
> +	if (first_non_matched_address) {
> +	    addresses->string = talloc_asprintf (addresses, "%s| %s",
> +						 addresses->string,
> +						 address);
> +	} else {
> +	    addresses->string = talloc_asprintf (addresses, "%s, %s",
> +						 addresses->string,
> +						 address);
> +	}

I second Michal's comments on this code; especially the one about
using talloc_asprintf_append, even if you don't combine the two
branches.  Currently this code leaks O(n^2) memory (in a talloc
context, so it's not permanent, but it's still unfortunate).

> +
> +	first_non_matched_address = 0;
> +    }
> +}
> +
>  /* Add each author of the thread to the thread's authors_hash and to
>   * the thread's authors_array. */
>  static void
> @@ -382,6 +477,27 @@ _resolve_thread_relationships (unused (notmuch_thread_t *thread))
>       */
>  }
>  
> +/* Initialize a thread addresses struct. */
> +notmuch_thread_addresses_t *
> +_thread_addresses_init (const void *ctx)

This should be "create" instead of "init".  Also, this should probably
be static, given that everything else is.  If there's reason to make
it non-static, then it should also start with _notmuch and have a
prototype in notmuch-private.h.

> +{
> +    notmuch_thread_addresses_t *addresses;
> +
> +    addresses = talloc (ctx, notmuch_thread_addresses_t);
> +    if (unlikely (addresses == NULL))
> +	return NULL;

You should set _notmuch_thread_addresses_destructor as the talloc
destructor here, rather than calling it by hand from
_notmuch_thread_destructor (in patch 2).

> +
> +    addresses->matched_hash = g_hash_table_new_full (g_str_hash, g_str_equal,
> +						     NULL, NULL);
> +    addresses->matched_array = g_ptr_array_new ();
> +    addresses->unmatched_hash = g_hash_table_new_full (g_str_hash, g_str_equal,
> +						       NULL, NULL);
> +    addresses->unmatched_array = g_ptr_array_new ();
> +    addresses->string = NULL;
> +
> +    return addresses;
> +}
> +
>  /* Create a new notmuch_thread_t object by finding the thread
>   * containing the message with the given doc ID, treating any messages
>   * contained in match_set as "matched".  Remove all messages in the

Re: [PATCH 02/11] lib: use new addresses structure for thread authors

[Austin Clements]

Quoth Jameson Graef Rollins on Aug 19 at  6:52 pm:
> Now that we have the infrastructure in place, we modify the thread
> object and associated functions to use the new addresses structure for
> storing thread authors.
> ---
>  lib/thread.cc |   41 +++++++++++------------------------------
>  1 file changed, 11 insertions(+), 30 deletions(-)
> 
> diff --git a/lib/thread.cc b/lib/thread.cc
> index 7af9eeb..9e0e5cb 100644
> --- a/lib/thread.cc
> +++ b/lib/thread.cc
> @@ -36,11 +36,7 @@ struct visible _notmuch_thread {
>      notmuch_database_t *notmuch;
>      char *thread_id;
>      char *subject;
> -    GHashTable *authors_hash;
> -    GPtrArray *authors_array;
> -    GHashTable *matched_authors_hash;
> -    GPtrArray *matched_authors_array;
> -    char *authors;
> +    notmuch_thread_addresses_t *authors;
>      GHashTable *tags;
>  
>      notmuch_message_list_t *message_list;
> @@ -66,21 +62,9 @@ _notmuch_thread_addresses_destructor (notmuch_thread_addresses_t *addresses)
>  static int
>  _notmuch_thread_destructor (notmuch_thread_t *thread)
>  {
> -    g_hash_table_unref (thread->authors_hash);
> -    g_hash_table_unref (thread->matched_authors_hash);
> +    _notmuch_thread_addresses_destructor (thread->authors);

This is the explicit call I mentioned in my talloc destructor comment
on patch 1.

>      g_hash_table_unref (thread->tags);
>      g_hash_table_unref (thread->message_hash);
> -
> -    if (thread->authors_array) {
> -	g_ptr_array_free (thread->authors_array, TRUE);
> -	thread->authors_array = NULL;
> -    }
> -
> -    if (thread->matched_authors_array) {
> -	g_ptr_array_free (thread->matched_authors_array, TRUE);
> -	thread->matched_authors_array = NULL;
> -    }
> -
>      return 0;
>  }
>  
> @@ -341,7 +325,7 @@ _thread_add_message (notmuch_thread_t *thread,
>  		author = internet_address_mailbox_get_addr (mailbox);
>  	    }
>  	    clean_author = _thread_cleanup_author (thread, author, from);
> -	    _thread_add_author (thread, clean_author);
> +	    _thread_add_address (thread->authors, clean_author, FALSE);
>  	    notmuch_message_set_author (message, clean_author);
>  	}
>  	g_object_unref (G_OBJECT (list));
> @@ -436,7 +420,7 @@ _thread_add_matched_message (notmuch_thread_t *thread,
>  				  NOTMUCH_MESSAGE_FLAG_MATCH, 1);
>      }
>  
> -    _thread_add_matched_author (thread, notmuch_message_get_author (hashed_message));
> +    _thread_add_address (thread->authors, notmuch_message_get_author (hashed_message), TRUE);
>  }
>  
>  static void
> @@ -555,14 +539,11 @@ _notmuch_thread_create (void *ctx,
>      thread->notmuch = notmuch;
>      thread->thread_id = talloc_strdup (thread, thread_id);
>      thread->subject = NULL;
> -    thread->authors_hash = g_hash_table_new_full (g_str_hash, g_str_equal,
> -						  NULL, NULL);
> -    thread->authors_array = g_ptr_array_new ();
> -    thread->matched_authors_hash = g_hash_table_new_full (g_str_hash,
> -							  g_str_equal,
> -							  NULL, NULL);
> -    thread->matched_authors_array = g_ptr_array_new ();
> -    thread->authors = NULL;
> +
> +    thread->authors = _thread_addresses_init (thread);
> +    if (unlikely (thread->authors == NULL))
> +	return NULL;
> +
>      thread->tags = g_hash_table_new_full (g_str_hash, g_str_equal,
>  					  free, NULL);
>  
> @@ -607,7 +588,7 @@ _notmuch_thread_create (void *ctx,
>  
>      notmuch_query_destroy (thread_id_query);
>  
> -    _resolve_thread_authors_string (thread);
> +    _resolve_thread_addresses_string (thread->authors);

If you make my suggested change to the
_resolve_thread_addresses_string API, this call would simply go
away...

>  
>      _resolve_thread_relationships (thread);
>  
> @@ -641,7 +622,7 @@ notmuch_thread_get_matched_messages (notmuch_thread_t *thread)
>  const char *
>  notmuch_thread_get_authors (notmuch_thread_t *thread)
>  {
> -    return thread->authors;
> +    return thread->authors->string;

... and this would be return _thread_addresses_to_string (or
whatever).

>  }
>  
>  const char *

-- 
Austin Clements                                      MIT/'06/PhD/CSAIL
amdragon@mit.edu                           http://web.mit.edu/amdragon
       Somewhere in the dream we call reality you will find me,
              searching for the reality we call dreams.

Re: [PATCH 05/11] lib: add ability to store recipients in message structure

[Austin Clements]

Quoth Jameson Graef Rollins on Aug 19 at  6:52 pm:
> Added as a string, in parallel to the authors element.
> ---
>  lib/message.cc        |   18 ++++++++++++++++++
>  lib/notmuch-private.h |    9 +++++++++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/lib/message.cc b/lib/message.cc
> index 978de06..fa28073 100644
> --- a/lib/message.cc
> +++ b/lib/message.cc
> @@ -36,6 +36,7 @@ struct visible _notmuch_message {
>      notmuch_string_list_t *filename_term_list;
>      notmuch_string_list_t *filename_list;
>      char *author;
> +    char *recipients;
>      notmuch_message_file_t *message_file;
>      notmuch_message_list_t *replies;
>      unsigned long flags;
> @@ -109,6 +110,7 @@ _notmuch_message_create_for_document (const void *talloc_owner,
>      message->filename_list = NULL;
>      message->message_file = NULL;
>      message->author = NULL;
> +    message->recipients = NULL;
>  
>      message->replies = _notmuch_message_list_create (message);
>      if (unlikely (message->replies == NULL)) {
> @@ -808,6 +810,22 @@ notmuch_message_set_author (notmuch_message_t *message,
>      return;
>  }
>  
> +const char *
> +notmuch_message_get_recipients (notmuch_message_t *message)
> +{
> +    return message->recipients;
> +}
> +
> +void
> +notmuch_message_set_recipients (notmuch_message_t *message,
> +				const char *recipients)
> +{
> +    if (message->recipients)
> +	talloc_free(message->recipients);
> +    message->recipients = talloc_strdup(message, recipients);

Missing spaces before parameter lists.

> +    return;
> +}
> +
>  void
>  _notmuch_message_set_header_values (notmuch_message_t *message,
>  				    const char *date,
> diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
> index bfb4111..27a41b6 100644
> --- a/lib/notmuch-private.h
> +++ b/lib/notmuch-private.h
> @@ -325,6 +325,15 @@ notmuch_message_set_author (notmuch_message_t *message, const char *author);
>  const char *
>  notmuch_message_get_author (notmuch_message_t *message);
>  
> +/* Set the recipients of 'message' - this is the representation used
> + * when displaying the message */
> +void
> +notmuch_message_set_recipients (notmuch_message_t *message, const char *recipients);
> +
> +/* Get the authors of 'message' */
> +const char *
> +notmuch_message_get_recipients (notmuch_message_t *message);
> +
>  
>  /* index.cc */
>  

-- 
Austin Clements                                      MIT/'06/PhD/CSAIL
amdragon@mit.edu                           http://web.mit.edu/amdragon
       Somewhere in the dream we call reality you will find me,
              searching for the reality we call dreams.

Re: [PATCH 06/11] lib: store thread recipients in thread structure

[Austin Clements]

Quoth Jameson Graef Rollins on Aug 19 at  6:52 pm:
> This utilizes the new thread addresses struct to store thread
> recipients, again in parallel to authors.
> 
> Since message recipients are not stored in the database, including
> recipients in the thread structure exacts a significant overhead as
> the recipients are retrieved from the original message files.  Because
> of this, a new boolean argument, include_recipients, is added to the
> necessary functions (_notmuch_thread_create, _thread_add_message and
> _thread_add_matched_message) that controls whether the recipients are
> fetched and included.  If message recipients are ever stored in the
> database this new argument could probably be removed.
> ---
>  lib/notmuch-private.h |    3 +-
>  lib/notmuch.h         |   14 +++++++++
>  lib/query.cc          |    3 +-
>  lib/thread.cc         |   77 +++++++++++++++++++++++++++++++++++++------------
>  4 files changed, 76 insertions(+), 21 deletions(-)
> 
> diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
> index 27a41b6..32d1523 100644
> --- a/lib/notmuch-private.h
> +++ b/lib/notmuch-private.h
> @@ -232,7 +232,8 @@ _notmuch_thread_create (void *ctx,
>  			unsigned int seed_doc_id,
>  			notmuch_doc_id_set_t *match_set,
>  			notmuch_string_list_t *excluded_terms,
> -			notmuch_sort_t sort);
> +			notmuch_sort_t sort,
> +			notmuch_bool_t include_recipients);
>  
>  /* message.cc */
>  
> diff --git a/lib/notmuch.h b/lib/notmuch.h
> index 6acd38d..f9e71c1 100644
> --- a/lib/notmuch.h
> +++ b/lib/notmuch.h
> @@ -759,6 +759,20 @@ notmuch_thread_get_matched_messages (notmuch_thread_t *thread);
>  const char *
>  notmuch_thread_get_authors (notmuch_thread_t *thread);
>  
> +/* Get the recipients of 'thread'
> + *
> + * The returned string is a comma-separated list of the names of the
> + * recipients of mail messages in the query results that belong to this
> + * thread.
> + *
> + * The returned string belongs to 'thread' and as such, should not be
> + * modified by the caller and will only be valid for as long as the
> + * thread is valid, (which is until notmuch_thread_destroy or until
> + * the query from which it derived is destroyed).
> + */
> +const char *
> +notmuch_thread_get_recipients (notmuch_thread_t *thread);
> +
>  /* Get the subject of 'thread'
>   *
>   * The subject is taken from the first message (according to the query
> diff --git a/lib/query.cc b/lib/query.cc
> index e9c1a2d..54833a7 100644
> --- a/lib/query.cc
> +++ b/lib/query.cc
> @@ -486,7 +486,8 @@ notmuch_threads_get (notmuch_threads_t *threads)
>  				   doc_id,
>  				   &threads->match_set,
>  				   threads->query->exclude_terms,
> -				   threads->query->sort);
> +				   threads->query->sort,
> +				   FALSE);
>  }
>  
>  void
> diff --git a/lib/thread.cc b/lib/thread.cc
> index 757e143..baf07c2 100644
> --- a/lib/thread.cc
> +++ b/lib/thread.cc
> @@ -37,6 +37,7 @@ struct visible _notmuch_thread {
>      char *thread_id;
>      char *subject;
>      notmuch_thread_addresses_t *authors;
> +    notmuch_thread_addresses_t *recipients;
>      GHashTable *tags;
>  
>      notmuch_message_list_t *message_list;
> @@ -63,6 +64,7 @@ static int
>  _notmuch_thread_destructor (notmuch_thread_t *thread)
>  {
>      _notmuch_thread_addresses_destructor (thread->authors);
> +    _notmuch_thread_addresses_destructor (thread->recipients);

Same thing about not calling this explicitly.

>      g_hash_table_unref (thread->tags);
>      g_hash_table_unref (thread->message_hash);
>      return 0;
> @@ -204,14 +206,17 @@ _thread_cleanup_address (notmuch_thread_t *thread,
>  static void
>  _thread_add_message (notmuch_thread_t *thread,
>  		     notmuch_message_t *message,
> -		     notmuch_string_list_t *exclude_terms)
> +		     notmuch_string_list_t *exclude_terms,
> +		     notmuch_bool_t include_recipients)

Could we instead omit this argument and construct the recipients list
when notmuch_thread_get_recipients is called for the first time?  That
makes things a little harder here, because you'll have to keep a list
of messages in the order they were added and scan it on demand (and
use the matched flag), but it will avoid the public API major version
bump as well as generally minimizing interface complexity.  Also, I
think that list will prove valuable for other things in the future,
since currently there's no way to iterate over the messages in a
thread in date order; the best you can do is recursive walk the thread
structure.  Conveniently, we already have a message list abstraction.

>  {
>      notmuch_tags_t *tags;
>      const char *tag;
> -    InternetAddressList *list = NULL;
> +    InternetAddressList *from_list = NULL;
> +    InternetAddressList *to_list = NULL;
>      InternetAddress *address;
>      const char *from, *author;
> -    char *clean_author;
> +    const char *to, *recipient;
> +    char *clean_address;
>  
>      _notmuch_message_list_add_message (thread->message_list,
>  				       talloc_steal (thread, message));
> @@ -223,10 +228,9 @@ _thread_add_message (notmuch_thread_t *thread,
>  
>      from = notmuch_message_get_header (message, "from");
>      if (from)
> -	list = internet_address_list_parse_string (from);
> -
> -    if (list) {
> -	address = internet_address_list_get_address (list, 0);
> +	from_list = internet_address_list_parse_string (from);
> +    if (from_list) {
> +	address = internet_address_list_get_address (from_list, 0);
>  	if (address) {
>  	    author = internet_address_get_name (address);
>  	    if (author == NULL) {
> @@ -234,11 +238,32 @@ _thread_add_message (notmuch_thread_t *thread,
>  		mailbox = INTERNET_ADDRESS_MAILBOX (address);
>  		author = internet_address_mailbox_get_addr (mailbox);
>  	    }
> -	    clean_author = _thread_cleanup_author (thread, author, from);
> -	    _thread_add_address (thread->authors, clean_author, FALSE);
> -	    notmuch_message_set_author (message, clean_author);
> +	    clean_address = _thread_cleanup_address (thread, author, from);
> +	    _thread_add_address (thread->authors, clean_address, FALSE);
> +	    notmuch_message_set_author (message, clean_address);
> +	}
> +	g_object_unref (G_OBJECT (from_list));
> +    }
> +
> +    if (include_recipients) {
> +    to = notmuch_message_get_header (message, "to");
> +    if (to)
> +	to_list = internet_address_list_parse_string (to);
> +    if (to_list) {
> +	address = internet_address_list_get_address (to_list, 0);
> +	if (address) {
> +	    recipient = internet_address_get_name (address);
> +	    if (recipient == NULL) {
> +		InternetAddressMailbox *mailbox;
> +		mailbox = INTERNET_ADDRESS_MAILBOX (address);
> +		recipient = internet_address_mailbox_get_addr (mailbox);
> +	    }
> +	    clean_address = _thread_cleanup_address (thread, recipient, to);
> +	    _thread_add_address (thread->recipients, clean_address, FALSE);
> +	    notmuch_message_set_recipients (message, clean_address);
>  	}
> -	g_object_unref (G_OBJECT (list));
> +	g_object_unref (G_OBJECT (to_list));
> +    }
>      }
>  
>      if (! thread->subject) {
> @@ -301,7 +326,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 include_recipients)
>  {
>      time_t date;
>      notmuch_message_t *hashed_message;
> @@ -331,6 +357,8 @@ _thread_add_matched_message (notmuch_thread_t *thread,
>      }
>  
>      _thread_add_address (thread->authors, notmuch_message_get_author (hashed_message), TRUE);
> +    if (include_recipients)
> +    _thread_add_address (thread->recipients, notmuch_message_get_recipients (hashed_message), TRUE);
>  }
>  
>  static void
> @@ -399,10 +427,10 @@ _thread_addresses_init (const void *ctx)
>   *
>   * Creating the thread will perform a database search to get all
>   * messages belonging to the thread and will get the first subject
> - * line, the total count of messages, and all authors in the thread.
> - * Each message in the thread is checked against match_set to allow
> - * for a separate count of matched messages, and to allow a viewer to
> - * display these messages differently.
> + * line, the total count of messages, and all authors and recipients
> + * of the thread.  Each message in the thread is checked against
> + * match_set to allow for a separate count of matched messages, and to
> + * allow a viewer to display these messages differently.
>   *
>   * Here, 'ctx' is talloc context for the resulting thread object.
>   *
> @@ -414,7 +442,8 @@ _notmuch_thread_create (void *ctx,
>  			unsigned int seed_doc_id,
>  			notmuch_doc_id_set_t *match_set,
>  			notmuch_string_list_t *exclude_terms,
> -			notmuch_sort_t sort)
> +			notmuch_sort_t sort,
> +			notmuch_bool_t include_recipients)
>  {
>      notmuch_thread_t *thread;
>      notmuch_message_t *seed_message;
> @@ -453,6 +482,9 @@ _notmuch_thread_create (void *ctx,
>      thread->authors = _thread_addresses_init (thread);
>      if (unlikely (thread->authors == NULL))
>  	return NULL;
> +    thread->recipients = _thread_addresses_init (thread);
> +    if (unlikely (thread->recipients == NULL))
> +	return NULL;
>  
>      thread->tags = g_hash_table_new_full (g_str_hash, g_str_equal,
>  					  free, NULL);
> @@ -486,11 +518,11 @@ _notmuch_thread_create (void *ctx,
>  	if (doc_id == seed_doc_id)
>  	    message = seed_message;
>  
> -	_thread_add_message (thread, message, exclude_terms);
> +	_thread_add_message (thread, message, exclude_terms, include_recipients);
>  
>  	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, include_recipients);
>  	}
>  
>  	_notmuch_message_close (message);
> @@ -499,6 +531,7 @@ _notmuch_thread_create (void *ctx,
>      notmuch_query_destroy (thread_id_query);
>  
>      _resolve_thread_addresses_string (thread->authors);
> +    _resolve_thread_addresses_string (thread->recipients);
>  
>      _resolve_thread_relationships (thread);
>  
> @@ -536,6 +569,12 @@ notmuch_thread_get_authors (notmuch_thread_t *thread)
>  }
>  
>  const char *
> +notmuch_thread_get_recipients (notmuch_thread_t *thread)
> +{
> +    return thread->recipients->string;
> +}
> +
> +const char *
>  notmuch_thread_get_subject (notmuch_thread_t *thread)
>  {
>      return thread->subject;

-- 
Austin Clements                                      MIT/'06/PhD/CSAIL
amdragon@mit.edu                           http://web.mit.edu/amdragon
       Somewhere in the dream we call reality you will find me,
              searching for the reality we call dreams.

Re: [PATCH 03/11] lib: give _thread_cleanup_author a more generic name

[Austin Clements]

Should this patch also rename calls to this function?

Quoth Jameson Graef Rollins on Aug 19 at  6:52 pm:
> We will use this for cleaning non-author address fields, so we give it
> the more generic name _thread_cleanup_address.
> ---
>  lib/thread.cc |   43 ++++++++++++++++++++++---------------------
>  1 file changed, 22 insertions(+), 21 deletions(-)
> 
> diff --git a/lib/thread.cc b/lib/thread.cc
> index 9e0e5cb..b53ccb0 100644
> --- a/lib/thread.cc
> +++ b/lib/thread.cc
> @@ -242,48 +242,49 @@ _resolve_thread_authors_string (notmuch_thread_t *thread)
>   * "Last, First MI" <first.mi.last@company.com>
>   */
>  static char *
> -_thread_cleanup_author (notmuch_thread_t *thread,
> -			const char *author, const char *from)
> +_thread_cleanup_address (notmuch_thread_t *thread,
> +			 const char *address,
> +			 const char *original)
>  {
> -    char *clean_author,*test_author;
> +    char *clean_address,*test_address;
>      const char *comma;
>      char *blank;
>      int fname,lname;
>  
> -    if (author == NULL)
> +    if (address == NULL)
>  	return NULL;
> -    clean_author = talloc_strdup(thread, author);
> -    if (clean_author == NULL)
> +    clean_address = talloc_strdup(thread, address);
> +    if (clean_address == NULL)
>  	return NULL;
>      /* check if there's a comma in the name and that there's a
>       * component of the name behind it (so the name doesn't end with
>       * the comma - in which case the string that strchr finds is just
>       * one character long ",\0").
> -     * Otherwise just return the copy of the original author name that
> +     * Otherwise just return the copy of the original address name that
>       * we just made*/
> -    comma = strchr(author,',');
> +    comma = strchr(address,',');
>      if (comma && strlen(comma) > 1) {
>  	/* let's assemble what we think is the correct name */
> -	lname = comma - author;
> -	fname = strlen(author) - lname - 2;
> -	strncpy(clean_author, comma + 2, fname);
> -	*(clean_author+fname) = ' ';
> -	strncpy(clean_author + fname + 1, author, lname);
> -	*(clean_author+fname+1+lname) = '\0';
> +	lname = comma - address;
> +	fname = strlen(address) - lname - 2;
> +	strncpy(clean_address, comma + 2, fname);
> +	*(clean_address+fname) = ' ';
> +	strncpy(clean_address + fname + 1, address, lname);
> +	*(clean_address+fname+1+lname) = '\0';
>  	/* make a temporary copy and see if it matches the email */
> -	test_author = talloc_strdup(thread,clean_author);
> +	test_address = talloc_strdup(thread,clean_address);
>  
> -	blank=strchr(test_author,' ');
> +	blank=strchr(test_address,' ');
>  	while (blank != NULL) {
>  	    *blank = '.';
> -	    blank=strchr(test_author,' ');
> +	    blank=strchr(test_address,' ');
>  	}
> -	if (strcasestr(from, test_author) == NULL)
> +	if (strcasestr(original, test_address) == NULL)
>  	    /* we didn't identify this as part of the email address
> -	    * so let's punt and return the original author */
> -	    strcpy (clean_author, author);
> +	    * so let's punt and return the original address */
> +	    strcpy (clean_address, address);
>      }
> -    return clean_author;
> +    return clean_address;
>  }
>  
>  /* Add 'message' as a message that belongs to 'thread'.

-- 
Austin Clements                                      MIT/'06/PhD/CSAIL
amdragon@mit.edu                           http://web.mit.edu/amdragon
       Somewhere in the dream we call reality you will find me,
              searching for the reality we call dreams.