unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
From: David Bremner <david@tethera.net>
To: notmuch@notmuchmail.org
Subject: [PATCH 10/15] lib/thread: change _resolve_thread_relationships to use depths
Date: Thu, 30 Aug 2018 08:29:10 -0300	[thread overview]
Message-ID: <20180830112915.11761-11-david@tethera.net> (raw)
In-Reply-To: <20180830112915.11761-1-david@tethera.net>

We (finally) implement the XXX comment. It requires a bit of care not
to reparent all of the possible toplevel messages. There is some
slightly naughty low level access to the list structures here that can
be cleaned up in a later commit.
---
 lib/thread.cc               | 53 ++++++++++++++++++-------------------
 test/T510-thread-replies.sh |  1 -
 2 files changed, 26 insertions(+), 28 deletions(-)

diff --git a/lib/thread.cc b/lib/thread.cc
index 85ca00ae..e4ab4319 100644
--- a/lib/thread.cc
+++ b/lib/thread.cc
@@ -467,12 +467,6 @@ _resolve_thread_relationships (notmuch_thread_t *thread)
 	    _notmuch_message_list_add_message (maybe_toplevel_list, message);
     }
 
-    for (notmuch_messages_t *roots = _notmuch_messages_create (maybe_toplevel_list);
-	 notmuch_messages_valid (roots);
-	 notmuch_messages_move_to_next (roots)) {
-	notmuch_message_t *message = notmuch_messages_get (roots);
-	_parent_or_toplevel (thread, message);
-    }
     /*
      * if we reach the end of the list without finding a top-level
      * message, that means the thread is a cycle (or set of cycles)
@@ -481,36 +475,41 @@ _resolve_thread_relationships (notmuch_thread_t *thread)
      */
     if (first_node) {
 	message = first_node->message;
-	if (! thread->toplevel_list->head ||
+	DEBUG_PRINTF("checking first message  %s\n",
+		     notmuch_message_get_message_id (message));
+
+	if (! maybe_toplevel_list->head ||
 	    ! _parent_via_in_reply_to (thread, message)) {
-	    /*
-	     * If the oldest message happens to be in-reply-to a
-	     * missing message, we only check for references if there
-	     * is some other candidate for root message.
-	     */
-	    if (thread->toplevel_list->head)
-		_parent_or_toplevel (thread, message);
-	    else
-		_notmuch_message_list_add_message (thread->toplevel_list, message);
+	    DEBUG_PRINTF("adding first message as toplevel = %s\n",
+		     notmuch_message_get_message_id (message));
+	    _notmuch_message_list_add_message (maybe_toplevel_list, message);
 	}
     }
 
+    for (notmuch_messages_t *messages = _notmuch_messages_create (maybe_toplevel_list);
+	 notmuch_messages_valid (messages);
+	 notmuch_messages_move_to_next (messages))
+    {
+	notmuch_message_t *message = notmuch_messages_get (messages);
+	_notmuch_message_label_depths (message, 0);
+    }
+
+    for (notmuch_messages_t *roots = _notmuch_messages_create (maybe_toplevel_list);
+	 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)
+	    _parent_or_toplevel (thread, message);
+	else
+	    _notmuch_message_list_add_message (thread->toplevel_list, message);
+    }
+
     /* XXX this could be made conditional on messages being inserted
      * (out of order) in later passes
      */
     thread->toplevel_list = _notmuch_message_sort_subtrees (thread, thread->toplevel_list);
 
-
-    /* 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.
-     */
     talloc_free (local);
 }
 
diff --git a/test/T510-thread-replies.sh b/test/T510-thread-replies.sh
index c244054a..915e68ef 100755
--- a/test/T510-thread-replies.sh
+++ b/test/T510-thread-replies.sh
@@ -174,7 +174,6 @@ EOF
 test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "reply to ghost (tree view)"
-test_subtest_known_broken
 test_emacs '(notmuch-tree "id:000-real-root@example.org")
 	    (notmuch-test-wait)
 	    (test-output)
-- 
2.18.0

  parent reply	other threads:[~2018-08-30 11:30 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-30 11:29 threading replies fixes v3 David Bremner
2018-08-30 11:29 ` [PATCH 01/15] util: add DEBUG_PRINTF, rename error_util.h -> debug_print.h David Bremner
2018-09-01 17:06   ` Tomi Ollila
2018-09-03 11:54     ` David Bremner
2018-08-30 11:29 ` [PATCH 02/15] test: start threading test corpus David Bremner
2018-08-30 11:29 ` [PATCH 03/15] test: add known broken tests for "ghost roots" David Bremner
2018-08-30 11:29 ` [PATCH 04/15] lib/thread: sort sibling messages by date David Bremner
2018-09-01 17:11   ` Tomi Ollila
2018-09-03 15:18     ` David Bremner
2018-08-30 11:29 ` [PATCH 05/15] lib: read reference terms into message struct David Bremner
2018-08-30 11:29 ` [PATCH 06/15] lib/thread: refactor in_reply_to test David Bremner
2018-09-01 20:02   ` Tomi Ollila
2018-08-30 11:29 ` [PATCH 07/15] lib/thread: initial use of references as for fallback parenting David Bremner
2018-08-30 11:29 ` [PATCH 08/15] lib: calculate message depth in thread David Bremner
2018-08-30 11:29 ` [PATCH 09/15] lib/thread: rewrite _parent_or_toplevel to use depths David Bremner
2018-08-30 11:29 ` David Bremner [this message]
2018-08-30 11:29 ` [PATCH 11/15] test: add known broken test for good In-Reply-To / bad References David Bremner
2018-08-30 11:29 ` [PATCH 12/15] test/thread-replies: mangle In-Reply-To's David Bremner
2018-08-30 11:29 ` [PATCH 13/15] util/string-util: export skip_space David Bremner
2018-08-30 11:29 ` [PATCH 14/15] lib: add _notmuch_message_id_parse_strict David Bremner
2018-09-01 20:27   ` Tomi Ollila
2018-08-30 11:29 ` [PATCH 15/15] lib: change parent strategy to use In-Reply-To if it looks sane David Bremner
2018-09-01 20:29   ` Tomi Ollila

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://notmuchmail.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180830112915.11761-11-david@tethera.net \
    --to=david@tethera.net \
    --cc=notmuch@notmuchmail.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).