notmuch show: Implement proper thread ordering/nesting of messages.
authorCarl Worth <cworth@cworth.org>
Mon, 16 Nov 2009 04:39:25 +0000 (20:39 -0800)
committerCarl Worth <cworth@cworth.org>
Mon, 16 Nov 2009 04:41:45 +0000 (20:41 -0800)
We now properly analyze the in-reply-to headers to create a proper
tree representing the actual thread and present the messages in this
correct thread order. Also, there's a new "depth:" value added to the
"message{" header so that clients can format the thread as desired,
(such as by indenting replies).

lib/message.cc
lib/notmuch-private.h
lib/notmuch.h
lib/thread.cc
notmuch-show.c

index b9f998c5f7e9f6bd7c75f01e8bf337fdb15add14..72c350f32ac2badaa96c15530c1d5b2071db4074 100644 (file)
@@ -33,6 +33,8 @@ struct _notmuch_message {
     char *thread_id;
     char *filename;
     notmuch_message_file_t *message_file;
+    notmuch_message_list_t *replies;
+
     Xapian::Document doc;
 };
 
@@ -110,6 +112,13 @@ _notmuch_message_create (const void *talloc_owner,
     message->filename = NULL;
     message->message_file = NULL;
 
+    message->replies = _notmuch_message_list_create (message);
+    if (unlikely (message->replies == NULL)) {
+       if (status)
+           *status = NOTMUCH_PRIVATE_STATUS_OUT_OF_MEMORY;
+       return NULL;
+    }
+
     /* This is C++'s creepy "placement new", which is really just an
      * ugly way to call a constructor for a pre-allocated object. So
      * it's really not an error to not be checking for OUT_OF_MEMORY
@@ -305,6 +314,19 @@ notmuch_message_get_thread_id (notmuch_message_t *message)
     return message->thread_id;
 }
 
+void
+_notmuch_message_add_reply (notmuch_message_t *message,
+                           notmuch_message_node_t *reply)
+{
+    _notmuch_message_list_append (message->replies, reply);
+}
+
+notmuch_messages_t *
+notmuch_message_get_replies (notmuch_message_t *message)
+{
+    return _notmuch_messages_create (message->replies);
+}
+
 /* Set the filename for 'message' to 'filename'.
  *
  * XXX: We should still figure out if we think it's important to store
index c4b66395f326a449c0e023a41cc24565f9668017..6036ce4a63a023399340d6d91c355b785744a38c 100644 (file)
@@ -304,6 +304,12 @@ _notmuch_message_list_add_message (notmuch_message_list_t *list,
 notmuch_messages_t *
 _notmuch_messages_create (notmuch_message_list_t *list);
 
+/* message.cc */
+
+void
+_notmuch_message_add_reply (notmuch_message_t *message,
+                           notmuch_message_node_t *reply);
+
 /* date.c */
 
 /* Parse an RFC 8222 date string to a time_t value.
index d0b0d9e43f5bc302e25cb67bccb771772b739df6..32b5332815837f8be6b7d22c2ff53cff114a4491 100644 (file)
@@ -477,6 +477,21 @@ notmuch_thread_get_thread_id (notmuch_thread_t *thread);
 int
 notmuch_thread_get_total_messages (notmuch_thread_t *thread);
 
+/* Get a notmuch_messages_t iterator for the top-level messages in
+ * 'thread'.
+ *
+ * This iterator will not necessarily iterate over all of the messages
+ * in the thread. It will only iterate over the messages in the thread
+ * which are not replies to other messages in the thread.
+ *
+ * To iterate over all messages in the thread, the caller will need to
+ * iterate over the result of notmuch_message_get_replies for each
+ * top-level message (and do that recursively for the resulting
+ * messages, etc.).
+ */
+notmuch_messages_t *
+notmuch_thread_get_toplevel_messages (notmuch_thread_t *thread);
+
 /* Get the number of messages in 'thread' that matched the search.
  *
  * This count includes only the messages in this thread that were
@@ -639,6 +654,26 @@ notmuch_message_get_message_id (notmuch_message_t *message);
 const char *
 notmuch_message_get_thread_id (notmuch_message_t *message);
 
+/* Get a notmuch_messages_t iterator for all of the replies to
+ * 'message'.
+ *
+ * Note: This call only makes sense if 'message' was ultimately
+ * obtained from a notmuch_thread_t object, (such as by coming
+ * directly from the result of calling notmuch_thread_get_
+ * toplevel_messages or by any number of subsequent
+ * calls to notmuch_message_get_replies).
+ *
+ * If 'message' was obtained through some non-thread means, (such as
+ * by a call to notmuch_query_search_messages), then this function
+ * will return NULL.
+ *
+ * If there are no replies to 'message', this function will return
+ * NULL. (Note that notmuch_messages_has_more will accept that NULL
+ * value as legitimate, and simply return FALSE for it.)
+ */
+notmuch_messages_t *
+notmuch_message_get_replies (notmuch_message_t *message);
+
 /* Get the filename for the email corresponding to 'message'.
  *
  * The returned filename is an absolute filename, (the initial
index cbce9c21528db21de4fca38ababfada0d2df79fd..4411d64d98da05964318b23c0d26cc9dba7e77af 100644 (file)
@@ -34,6 +34,8 @@ struct _notmuch_thread {
     char *authors;
     GHashTable *tags;
 
+    notmuch_message_list_t *message_list;
+    GHashTable *message_hash;
     int total_messages;
     int matched_messages;
     time_t oldest;
@@ -45,6 +47,7 @@ _notmuch_thread_destructor (notmuch_thread_t *thread)
 {
     g_hash_table_unref (thread->authors_hash);
     g_hash_table_unref (thread->tags);
+    g_hash_table_unref (thread->message_hash);
 
     return 0;
 }
@@ -70,6 +73,11 @@ _thread_add_author (notmuch_thread_t *thread,
        thread->authors = talloc_strdup (thread, author);
 }
 
+/* Add 'message' as a message that belongs to 'thread'.
+ *
+ * The 'thread' will talloc_steal the 'message' and hold onto a
+ * reference to it.
+ */
 static void
 _thread_add_message (notmuch_thread_t *thread,
                     notmuch_message_t *message)
@@ -80,6 +88,14 @@ _thread_add_message (notmuch_thread_t *thread,
     InternetAddress *address;
     const char *from, *author;
 
+    _notmuch_message_list_add_message (thread->message_list,
+                                      talloc_steal (thread, message));
+    thread->total_messages++;
+
+    g_hash_table_insert (thread->message_hash,
+                        xstrdup (notmuch_message_get_message_id (message)),
+                        message);
+
     from = notmuch_message_get_header (message, "from");
     list = internet_address_list_parse_string (from);
     if (list) {
@@ -109,8 +125,6 @@ _thread_add_message (notmuch_thread_t *thread,
        tag = notmuch_tags_get (tags);
        g_hash_table_insert (thread->tags, xstrdup (tag), NULL);
     }
-
-    thread->total_messages++;
 }
 
 static void
@@ -130,6 +144,44 @@ _thread_add_matched_message (notmuch_thread_t *thread,
     thread->matched_messages++;
 }
 
+static void
+_resolve_thread_relationships (unused (notmuch_thread_t *thread))
+{
+    notmuch_message_node_t **prev, *node;
+    notmuch_message_t *message, *parent;
+    const char *in_reply_to;
+
+    prev = &thread->message_list->head;
+    while ((node = *prev)) {
+       message = node->message;
+       in_reply_to = _notmuch_message_get_in_reply_to (message);
+       if (in_reply_to &&
+           g_hash_table_lookup_extended (thread->message_hash,
+                                         in_reply_to, NULL,
+                                         (void **) &parent))
+       {
+           *prev = node->next;
+           if (thread->message_list->tail == &node->next)
+               thread->message_list->tail = prev;
+           node->next = NULL;
+           _notmuch_message_add_reply (parent, node);
+       } else {
+           prev = &((*prev)->next);
+       }
+    }
+
+    /* XXX: After scanning through the entire list looking for parents
+     * via "In-Reply-To", we should do a second pass that looks at the
+     * list of messages IDs in the "References" header instead. (And
+     * for this the parent would be the "deepest" message of all the
+     * messages found in the "References" list.)
+     *
+     * Doing this will allow messages and sub-threads to be positioned
+     * correctly in the thread even when an intermediate message is
+     * missing from the thread.
+     */
+}
+
 /* Create a new notmuch_thread_t object for the given thread ID,
  * treating any messages matching 'query_string' as "matched".
  *
@@ -160,6 +212,10 @@ _notmuch_thread_create (void *ctx,
     if (unlikely (query_string == NULL))
        return NULL;
 
+    /* XXX: We could be a bit more efficient here if
+     * thread_id_query_string is identical to query_string, (then we
+     * could get by with just one database search instead of two). */
+
     matched_query_string = talloc_asprintf (ctx, "%s AND (%s)",
                                            thread_id_query_string,
                                            query_string);
@@ -189,6 +245,13 @@ _notmuch_thread_create (void *ctx,
     thread->tags = g_hash_table_new_full (g_str_hash, g_str_equal,
                                          free, NULL);
 
+    thread->message_list = _notmuch_message_list_create (thread);
+    if (unlikely (thread->message_list == NULL))
+       return NULL;
+
+    thread->message_hash = g_hash_table_new_full (g_str_hash, g_str_equal,
+                                                 free, NULL);
+
     thread->total_messages = 0;
     thread->matched_messages = 0;
     thread->oldest = 0;
@@ -214,9 +277,17 @@ _notmuch_thread_create (void *ctx,
 
     notmuch_query_destroy (matched_query);
 
+    _resolve_thread_relationships (thread);
+
     return thread;
 }
 
+notmuch_messages_t *
+notmuch_thread_get_toplevel_messages (notmuch_thread_t *thread)
+{
+    return _notmuch_messages_create (thread->message_list);
+}
+
 const char *
 notmuch_thread_get_thread_id (notmuch_thread_t *thread)
 {
index 03553d88b30b5abcde0b5c399480a5cf36e55652..7749dbc4b68ffde96b39fe81c95a993a8e708955 100644 (file)
@@ -117,22 +117,72 @@ show_part (GMimeObject *part, int *part_count)
     printf ("\fpart}\n");
 }
 
+static void
+show_message (void *ctx, notmuch_message_t *message, int indent)
+{
+    const char *headers[] = {
+       "Subject", "From", "To", "Cc", "Bcc", "Date"
+    };
+    const char *name, *value;
+    unsigned int i;
+
+    printf ("\fmessage{ id:%s depth:%d filename:%s\n",
+           notmuch_message_get_message_id (message),
+           indent,
+           notmuch_message_get_filename (message));
+
+    printf ("\fheader{\n");
+
+    printf ("%s\n", _get_one_line_summary (ctx, message));
+
+    for (i = 0; i < ARRAY_SIZE (headers); i++) {
+       name = headers[i];
+       value = notmuch_message_get_header (message, name);
+       if (value)
+           printf ("%s: %s\n", name, value);
+    }
+
+    printf ("\fheader}\n");
+    printf ("\fbody{\n");
+
+    show_message_body (notmuch_message_get_filename (message), show_part);
+
+    printf ("\fbody}\n");
+
+    printf ("\fmessage}\n");
+}
+
+
+static void
+show_messages (void *ctx, notmuch_messages_t *messages, int indent)
+{
+    notmuch_message_t *message;
+
+    for (;
+        notmuch_messages_has_more (messages);
+        notmuch_messages_advance (messages))
+    {
+       message = notmuch_messages_get (messages);
+
+       show_message (ctx, message, indent);
+
+       show_messages (ctx, notmuch_message_get_replies (message), indent + 1);
+
+       notmuch_message_destroy (message);
+    }
+}
+
 int
 notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
 {
     notmuch_config_t *config;
     notmuch_database_t *notmuch;
     notmuch_query_t *query;
+    notmuch_threads_t *threads;
+    notmuch_thread_t *thread;
     notmuch_messages_t *messages;
-    notmuch_message_t *message;
     char *query_string;
 
-    const char *headers[] = {
-       "Subject", "From", "To", "Cc", "Bcc", "Date"
-    };
-    const char *name, *value;
-    unsigned int i;
-
     config = notmuch_config_open (ctx, NULL, NULL);
     if (config == NULL)
        return 1;
@@ -153,37 +203,17 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
        return 1;
     }
 
-    for (messages = notmuch_query_search_messages (query, 0, -1);
-        notmuch_messages_has_more (messages);
-        notmuch_messages_advance (messages))
+    for (threads = notmuch_query_search_threads (query, 0, -1);
+        notmuch_threads_has_more (threads);
+        notmuch_threads_advance (threads))
     {
-       message = notmuch_messages_get (messages);
-
-       printf ("\fmessage{ id:%s filename:%s\n",
-               notmuch_message_get_message_id (message),
-               notmuch_message_get_filename (message));
+       thread = notmuch_threads_get (threads);
 
-       printf ("\fheader{\n");
+       messages = notmuch_thread_get_toplevel_messages (thread);
 
-       printf ("%s\n", _get_one_line_summary (ctx, message));
+       show_messages (ctx, messages, 0);
 
-       for (i = 0; i < ARRAY_SIZE (headers); i++) {
-           name = headers[i];
-           value = notmuch_message_get_header (message, name);
-           if (value)
-               printf ("%s: %s\n", name, value);
-       }
-
-       printf ("\fheader}\n");
-       printf ("\fbody{\n");
-
-       show_message_body (notmuch_message_get_filename (message), show_part);
-
-       printf ("\fbody}\n");
-
-       printf ("\fmessage}\n");
-
-       notmuch_message_destroy (message);
+       notmuch_thread_destroy (thread);
     }
 
     notmuch_query_destroy (query);