From 933caf814fcbbb7420d03ef42bb37bea6dd90449 Mon Sep 17 00:00:00 2001 From: Carl Worth Date: Sun, 15 Nov 2009 20:39:25 -0800 Subject: [PATCH] notmuch show: Implement proper thread ordering/nesting of messages. 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 | 22 ++++++++++ lib/notmuch-private.h | 6 +++ lib/notmuch.h | 35 ++++++++++++++++ lib/thread.cc | 75 ++++++++++++++++++++++++++++++++- notmuch-show.c | 98 ++++++++++++++++++++++++++++--------------- 5 files changed, 200 insertions(+), 36 deletions(-) diff --git a/lib/message.cc b/lib/message.cc index b9f998c5..72c350f3 100644 --- a/lib/message.cc +++ b/lib/message.cc @@ -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 diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h index c4b66395..6036ce4a 100644 --- a/lib/notmuch-private.h +++ b/lib/notmuch-private.h @@ -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. diff --git a/lib/notmuch.h b/lib/notmuch.h index d0b0d9e4..32b53328 100644 --- a/lib/notmuch.h +++ b/lib/notmuch.h @@ -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 diff --git a/lib/thread.cc b/lib/thread.cc index cbce9c21..4411d64d 100644 --- a/lib/thread.cc +++ b/lib/thread.cc @@ -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) { diff --git a/notmuch-show.c b/notmuch-show.c index 03553d88..7749dbc4 100644 --- a/notmuch-show.c +++ b/notmuch-show.c @@ -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); -- 2.26.2