diff --git a/lib/mu-query-threads.cc b/lib/mu-query-threads.cc index f6663cce..fb0a3804 100644 --- a/lib/mu-query-threads.cc +++ b/lib/mu-query-threads.cc @@ -62,7 +62,7 @@ struct Container { return up && up == other->ur_parent(); } - template void for_each_child(Func&& func) { + template void for_each_child(const Func& func) { auto it{children.rbegin()}; while (it != children.rend()) { auto next = std::next(it); @@ -80,13 +80,6 @@ struct Container { std::string thread_date_key; - // The container's own date-key, set once from its query-match and never - // overwritten by the bubble-up above. Siblings must sort by this, not by - // thread_date_key -- otherwise a subthread gaining a new, later message - // (e.g. a reply to one message in a patch series) reorders it among its - // siblings, even though its own date didn't change. - std::string own_date_key; - Option query_match; bool is_nuked{}; Container* parent{}; @@ -195,12 +188,14 @@ determine_id_table(QueryResultsType& qres) }(); // We sort by date (ascending), *except* for the root; we don't - // know what query_matchs will be at the root level yet, so remember - // both. Moreover, even when sorting the top-level in descending + // know what query_matchs will be at the root level yet, so + // remember both. + // + // Moreover, even when sorting the top-level in descending // order, still sort the thread levels below that in ascending // order, keeping sibling order stable. - container.own_date_key = container.thread_date_key = - container.query_match->date_key = mi.date_str().value_or(""); + container.thread_date_key = container.query_match->date_key = + mi.date_str().value_or(""); // initial guess for the thread-date; might be updated // later. @@ -494,17 +489,24 @@ sort_container(Container& container) for (auto& child : container.children) sort_container(*child); - // now sort this level, by each child's own date, falling back to - // thread_date_key if the container has no message of its own; use a - // stable sort so messages with equal dates keep their original (mset) - // order. - std::ranges::stable_sort(container.children, {}, [](const Container* c) { - return c->own_date_key.empty() ? c->thread_date_key : c->own_date_key; + // now sort this level, by each child's _own_ date, not the (bubbled-up) + // thread-date -- otherwise a subthread gaining a new message could + // reorder wrongly. + // + // Fall back to the thread-date for containers without a message of + // their own; use a stable sort so messages with equal dates keep their + // original (mset) order. + std::ranges::stable_sort(container.children, {}, + [](const Container* c) -> const std::string& { + return c->query_match ? c->query_match->date_key : c->thread_date_key; }); // and 'bubble up' the date of the *newest* message with a date. We - // reasonably assume that it's later than its parent. - const auto& newest_date = container.children.back()->thread_date_key; + // reasonably assume that it's later than its parent. Note that since + // children are not sorted by thread-date, the newest is not necessarily + // the last child. + const auto& newest_date = std::ranges::max( + container.children, {}, &Container::thread_date_key)->thread_date_key; if (!newest_date.empty()) container.thread_date_key = newest_date; } @@ -735,6 +737,32 @@ test_sibling_order_stable() }); } +static void +test_thread_date_bubble_up() +{ + // thread A's newest message (date 9) hangs under a middle sibling; + // thread B (date 5) is older than that, so ascending order must put A + // *after* B. + auto results = MockQueryResults{ + MockQueryResult{"a", "1", {}}, + MockQueryResult{"a1", "2", {"a"}}, + MockQueryResult{"a2", "3", {"a"}}, + MockQueryResult{"reply-a1", "9", {"a", "a1"}}, + MockQueryResult{"b", "5", {}}, + }; + + calculate_threads(results, false); + + assert_thread_paths(results, + { + {"b", "0"}, + {"a", "1"}, + {"a1", "1:0"}, + {"reply-a1", "1:0:0"}, + {"a2", "1:1"}, + }); +} + static void test_id_table_inconsistent() { @@ -966,6 +994,7 @@ try { g_test_add_func("/threader/sort/ascending", test_sort_ascending); g_test_add_func("/threader/sort/decending", test_sort_descending); g_test_add_func("/threader/sort/sibling-order-stable", test_sibling_order_stable); + g_test_add_func("/threader/sort/thread-date-bubble-up", test_thread_date_bubble_up); g_test_add_func("/threader/id-table-inconsistent", test_id_table_inconsistent); g_test_add_func("/threader/dups/dup-last", test_dups_dup_last); diff --git a/lib/mu-query-threads.hh b/lib/mu-query-threads.hh index 5aab888d..1743bf29 100644 --- a/lib/mu-query-threads.hh +++ b/lib/mu-query-threads.hh @@ -1,5 +1,5 @@ /* -** Copyright (C) 2021 Dirk-Jan C. Binnema +** Copyright (C) 2021-2026 Dirk-Jan C. Binnema ** ** This program is free software; you can redistribute it and/or modify it ** under the terms of the GNU General Public License as published by the @@ -28,6 +28,9 @@ namespace Mu { * thread-paths for each message, so we can let Xapian order them in the correct * order. * + * This is strongly inspired by the JWZ message threading algorithm + * https://www.jwz.org/doc/threading.html + * * Note - threads are sorted chronologically, and the messages below the top * level are always sorted in ascending orde *