This is mainly coding style issues pointed out by Tomi, along with moving some of the low level list access into two new functions. The interdiff is a bit noisy since I dropped the creation of debug_print.{c,h}. I'm going to optimistically claim this version is ready to go, but feel free to point out any issues. diff --git a/command-line-arguments.c b/command-line-arguments.c index 90d69453..d64aa85b 100644 --- a/command-line-arguments.c +++ b/command-line-arguments.c @@ -1,7 +1,7 @@ #include <assert.h> #include <string.h> #include <stdio.h> -#include "debug_print.h" +#include "error_util.h" #include "command-line-arguments.h" typedef enum { diff --git a/lib/message-id.c b/lib/message-id.c index a1dce9c8..e71ce9f4 100644 --- a/lib/message-id.c +++ b/lib/message-id.c @@ -100,7 +100,6 @@ char * _notmuch_message_id_parse_strict (void *ctx, const char *message_id) { const char *s, *end; - char *result; if (message_id == NULL || *message_id == '\0') return NULL; @@ -118,11 +117,10 @@ _notmuch_message_id_parse_strict (void *ctx, const char *message_id) if (*end != '>') return NULL; else { - const char *last = skip_space (end+1); + const char *last = skip_space (end + 1); if (*last != '\0') return NULL; } - result = talloc_strndup (ctx, s, end - s); - return result; + return talloc_strndup (ctx, s, end - s); } diff --git a/lib/message.cc b/lib/message.cc index 5e17029a..6f2f6345 100644 --- a/lib/message.cc +++ b/lib/message.cc @@ -637,7 +637,12 @@ _cmpmsg (const void *pa, const void *pb) time_t time_a = notmuch_message_get_date (*a); time_t time_b = notmuch_message_get_date (*b); - return (int) difftime (time_a, time_b); + if (time_a == time_b) + return 0; + else if (time_a < time_b) + return -1; + else + return 1; } notmuch_message_list_t * diff --git a/lib/messages.c b/lib/messages.c index a88f974f..04fa19f8 100644 --- a/lib/messages.c +++ b/lib/messages.c @@ -56,6 +56,15 @@ _notmuch_message_list_add_message (notmuch_message_list_t *list, list->tail = &node->next; } +bool +_notmuch_message_list_empty (notmuch_message_list_t *list) +{ + if (list == NULL) + return TRUE; + + return (list->head == NULL); +} + notmuch_messages_t * _notmuch_messages_create (notmuch_message_list_t *list) { @@ -101,6 +110,18 @@ notmuch_messages_valid (notmuch_messages_t *messages) return (messages->iterator != NULL); } +bool +_notmuch_messages_has_next (notmuch_messages_t *messages) +{ + if (! notmuch_messages_valid (messages)) + return false; + + if (! messages->is_of_list_type) + INTERNAL_ERROR("_notmuch_messages_has_next not implimented for msets"); + + return (messages->iterator->next != NULL); +} + notmuch_message_t * notmuch_messages_get (notmuch_messages_t *messages) { diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h index 5bbaa292..df32d39c 100644 --- a/lib/notmuch-private.h +++ b/lib/notmuch-private.h @@ -50,12 +50,13 @@ NOTMUCH_BEGIN_DECLS #include "gmime-extra.h" #include "xutil.h" -#include "debug_print.h" +#include "error_util.h" #include "string-util.h" #include "crypto.h" #ifdef DEBUG # define DEBUG_DATABASE_SANITY 1 +# define DEBUG_THREADING 1 # define DEBUG_QUERY 1 #endif @@ -476,6 +477,9 @@ struct _notmuch_messages { notmuch_message_list_t * _notmuch_message_list_create (const void *ctx); +bool +_notmuch_message_list_empty (notmuch_message_list_t *list); + void _notmuch_message_list_add_message (notmuch_message_list_t *list, notmuch_message_t *message); @@ -483,6 +487,9 @@ _notmuch_message_list_add_message (notmuch_message_list_t *list, notmuch_messages_t * _notmuch_messages_create (notmuch_message_list_t *list); +bool +_notmuch_messages_has_next (notmuch_messages_t *messages); + /* query.cc */ bool diff --git a/lib/thread.cc b/lib/thread.cc index e4ab4319..47c90664 100644 --- a/lib/thread.cc +++ b/lib/thread.cc @@ -24,6 +24,12 @@ #include <gmime/gmime.h> #include <glib.h> /* GHashTable */ +#ifdef DEBUG_THREADING +#define THREAD_DEBUG(format, ...) fprintf(stderr, format " (%s).\n", ##__VA_ARGS__, __location__) +#else +#define THREAD_DEBUG(format, ...) do {} while (0) /* ignored */ +#endif + #define EMPTY_STRING(s) ((s)[0] == '\0') struct _notmuch_thread { @@ -393,10 +399,10 @@ _parent_via_in_reply_to (notmuch_thread_t *thread, notmuch_message_t *message) { const char *in_reply_to; in_reply_to = _notmuch_message_get_in_reply_to (message); - DEBUG_PRINTF("checking message = %s in_reply_to=%s\n", + THREAD_DEBUG("checking message = %s in_reply_to=%s\n", notmuch_message_get_message_id (message), in_reply_to); - if (in_reply_to && strlen (in_reply_to) && + if (in_reply_to && (! EMPTY_STRING(in_reply_to)) && g_hash_table_lookup_extended (thread->message_hash, in_reply_to, NULL, (void **) &parent)) { @@ -416,31 +422,31 @@ _parent_or_toplevel (notmuch_thread_t *thread, notmuch_message_t *message) const notmuch_string_list_t *references = _notmuch_message_get_references (message); - DEBUG_PRINTF("trying to reparent via references: %s\n", + THREAD_DEBUG("trying to reparent via references: %s\n", notmuch_message_get_message_id (message)); for (notmuch_string_node_t *ref_node = references->head; ref_node; ref_node = ref_node->next) { - DEBUG_PRINTF("checking reference=%s\n", ref_node->string); + THREAD_DEBUG("checking reference=%s\n", ref_node->string); if ((g_hash_table_lookup_extended (thread->message_hash, ref_node->string, NULL, (void **) &new_parent))) { size_t new_depth = _notmuch_message_get_thread_depth (new_parent); - DEBUG_PRINTF("got depth %lu\n", new_depth); + THREAD_DEBUG("got depth %lu\n", new_depth); if (new_depth > max_depth || !parent) { - DEBUG_PRINTF("adding at depth %lu parent=%s\n", new_depth, ref_node->string); + THREAD_DEBUG("adding at depth %lu parent=%s\n", new_depth, ref_node->string); max_depth = new_depth; parent = new_parent; } } } if (parent) { - DEBUG_PRINTF("adding reply %s to parent=%s\n", + THREAD_DEBUG("adding reply %s to parent=%s\n", notmuch_message_get_message_id (message), notmuch_message_get_message_id (parent)); _notmuch_message_add_reply (parent, message); } else { - DEBUG_PRINTF("adding as toplevel %s\n", + THREAD_DEBUG("adding as toplevel %s\n", notmuch_message_get_message_id (message)); _notmuch_message_list_add_message (thread->toplevel_list, message); } @@ -475,13 +481,14 @@ _resolve_thread_relationships (notmuch_thread_t *thread) */ if (first_node) { message = first_node->message; - DEBUG_PRINTF("checking first message %s\n", + THREAD_DEBUG("checking first message %s\n", notmuch_message_get_message_id (message)); - if (! maybe_toplevel_list->head || + if (_notmuch_message_list_empty (maybe_toplevel_list) || ! _parent_via_in_reply_to (thread, message)) { - DEBUG_PRINTF("adding first message as toplevel = %s\n", - notmuch_message_get_message_id (message)); + + THREAD_DEBUG("adding first message as toplevel = %s\n", + notmuch_message_get_message_id (message)); _notmuch_message_list_add_message (maybe_toplevel_list, message); } } @@ -498,8 +505,7 @@ _resolve_thread_relationships (notmuch_thread_t *thread) notmuch_messages_valid (roots); notmuch_messages_move_to_next (roots)) { notmuch_message_t *message = notmuch_messages_get (roots); - /* XXX TODO: cleaner way to test iterator for last, list for emptiness */ - if (roots->iterator->next || thread->toplevel_list->head) + if (_notmuch_messages_has_next (roots) || ! _notmuch_message_list_empty (thread->toplevel_list)) _parent_or_toplevel (thread, message); else _notmuch_message_list_add_message (thread->toplevel_list, message); diff --git a/test/T510-thread-replies.sh b/test/T510-thread-replies.sh index 4688c0ab..5d6bea7e 100755 --- a/test/T510-thread-replies.sh +++ b/test/T510-thread-replies.sh @@ -222,5 +222,4 @@ End of search results. EOF test_expect_equal_file EXPECTED OUTPUT - test_done _______________________________________________ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch