From ba7f5dce1c37c04768aa060b35f3803e6db3840e Mon Sep 17 00:00:00 2001 From: Manos Pitsidianakis Date: Sun, 2 Jul 2023 00:32:58 +0300 Subject: [PATCH] listing/thread: fix display of threaded conversations tree structure When missing intermediate and/or parent messages in a thread, the printed thread tree branches were completely invalid. This commit makes sure thread node entries that have no corresponding envelopes are accounted for in the tree structure. --- melib/src/thread.rs | 11 ++- melib/src/thread/iterators.rs | 73 +++++++++----------- src/components/mail/listing.rs | 2 +- src/components/mail/listing/compact.rs | 4 +- src/components/mail/listing/conversations.rs | 4 +- src/components/mail/listing/plain.rs | 8 +-- src/components/mail/listing/thread.rs | 54 ++++++++------- src/components/mail/view/thread.rs | 14 +--- 8 files changed, 76 insertions(+), 94 deletions(-) diff --git a/melib/src/thread.rs b/melib/src/thread.rs index b04243cf..838d31d6 100644 --- a/melib/src/thread.rs +++ b/melib/src/thread.rs @@ -744,11 +744,8 @@ impl Threads { } } - pub fn threads_group_iter( - &self, - root_tree: SmallVec<[ThreadNodeHash; 1024]>, - ) -> ThreadsGroupIterator { - ThreadsGroupIterator { + pub fn threads_iter(&self, root_tree: SmallVec<[ThreadNodeHash; 1024]>) -> ThreadsIterator { + ThreadsIterator { root_tree, pos: 0, stack: SmallVec::new(), @@ -756,8 +753,8 @@ impl Threads { } } - pub fn thread_group_iter(&self, index: ThreadHash) -> ThreadGroupIterator { - ThreadGroupIterator { + pub fn thread_iter(&self, index: ThreadHash) -> ThreadIterator { + ThreadIterator { group: self.thread_ref(index).root(), pos: 0, stack: SmallVec::new(), diff --git a/melib/src/thread/iterators.rs b/melib/src/thread/iterators.rs index 99c9d89c..b9039c7e 100644 --- a/melib/src/thread/iterators.rs +++ b/melib/src/thread/iterators.rs @@ -25,28 +25,28 @@ use smallvec::SmallVec; use super::{ThreadNode, ThreadNodeHash}; -/* `ThreadsIterator` returns messages according to the sorted order. For - * example, for the following threads: - * - * ``` - * A_ - * |_ B - * |_C - * D - * E_ - * |_F - * ``` - * - * the iterator returns them as `A, B, C, D, E, F` - */ - -pub struct ThreadsGroupIterator<'a> { +/// [`ThreadsIterator`] returns messages according to the sorted order. +/// +/// For example, for the following threads: +/// +/// ```text +/// A_ +/// |_ B +/// |_C +/// D +/// E_ +/// |_F +/// ``` +/// +/// the iterator returns them as `A, B, C, D, E, F`. +pub struct ThreadsIterator<'a> { pub(super) root_tree: SmallVec<[ThreadNodeHash; 1024]>, pub(super) pos: usize, pub(super) stack: SmallVec<[usize; 16]>, pub(super) thread_nodes: &'a HashMap, } -impl<'a> Iterator for ThreadsGroupIterator<'a> { + +impl<'a> Iterator for ThreadsIterator<'a> { type Item = (usize, ThreadNodeHash, bool); fn next(&mut self) -> Option { loop { @@ -70,41 +70,36 @@ impl<'a> Iterator for ThreadsGroupIterator<'a> { if !self.thread_nodes[&tree[self.pos]].children.is_empty() { self.stack.push(self.pos); self.pos = 0; - if self.thread_nodes[&ret.1].message.is_some() { - return Some(ret); - } else { - continue; - } - } - self.pos += 1; - if self.thread_nodes[&ret.1].message.is_some() { return Some(ret); } + self.pos += 1; + return Some(ret); } } } } -/* `ThreadIterator` returns messages of a specific thread according to the - * sorted order. For example, for the following thread: - * - * ``` - * A_ - * |_ B - * |_C - * |_D - * ``` - * - * the iterator returns them as `A, B, C, D` - */ -pub struct ThreadGroupIterator<'a> { +/// [`ThreadIterator`] returns messages of a specific thread according to the +/// sorted order. +/// +/// For example, for the following thread: +/// +/// ```text +/// A_ +/// |_ B +/// |_C +/// |_D +/// ``` +/// +/// the iterator returns them as `A, B, C, D`. +pub struct ThreadIterator<'a> { pub(super) group: ThreadNodeHash, pub(super) pos: usize, pub(super) stack: SmallVec<[usize; 16]>, pub(super) thread_nodes: &'a HashMap, } -impl<'a> Iterator for ThreadGroupIterator<'a> { +impl<'a> Iterator for ThreadIterator<'a> { type Item = (usize, ThreadNodeHash); fn next(&mut self) -> Option<(usize, ThreadNodeHash)> { loop { diff --git a/src/components/mail/listing.rs b/src/components/mail/listing.rs index 4565d61f..9a5fc1e1 100644 --- a/src/components/mail/listing.rs +++ b/src/components/mail/listing.rs @@ -450,7 +450,7 @@ pub trait MailListingTrait: ListingTrait { /*{ let threads_lck = account.collection.get_threads(mailbox_hash); for thread_hash in thread_hashes { - for (_, h) in threads_lck.thread_group_iter(thread_hash) { + for (_, h) in threads_lck.thread_iter(thread_hash) { envs_to_set.push(threads_lck.thread_nodes()[&h].message().unwrap()); } self.row_updates().push(thread_hash); diff --git a/src/components/mail/listing/compact.rs b/src/components/mail/listing/compact.rs index 20fc28c9..4df8c1c7 100644 --- a/src/components/mail/listing/compact.rs +++ b/src/components/mail/listing/compact.rs @@ -401,7 +401,7 @@ impl MailListingTrait for CompactListing { from_address_list.clear(); from_address_set.clear(); for (envelope, show_subject) in threads - .thread_group_iter(thread) + .thread_iter(thread) .filter_map(|(_, h)| { Some(( threads.thread_nodes()[&h].message()?, @@ -1165,7 +1165,7 @@ impl CompactListing { let mut from_address_set: std::collections::HashSet> = std::collections::HashSet::new(); for (envelope, show_subject) in threads - .thread_group_iter(thread_hash) + .thread_iter(thread_hash) .filter_map(|(_, h)| { threads.thread_nodes()[&h] .message() diff --git a/src/components/mail/listing/conversations.rs b/src/components/mail/listing/conversations.rs index f6fcab2e..2d45d3a7 100644 --- a/src/components/mail/listing/conversations.rs +++ b/src/components/mail/listing/conversations.rs @@ -306,7 +306,7 @@ impl MailListingTrait for ConversationsListing { from_address_list.clear(); from_address_set.clear(); for (envelope, show_subject) in threads - .thread_group_iter(thread) + .thread_iter(thread) .filter_map(|(_, h)| { Some(( threads.thread_nodes()[&h].message()?, @@ -856,7 +856,7 @@ impl ConversationsListing { let mut from_address_set: std::collections::HashSet> = std::collections::HashSet::new(); for (envelope, show_subject) in threads - .thread_group_iter(thread_hash) + .thread_iter(thread_hash) .filter_map(|(_, h)| { threads.thread_nodes()[&h] .message() diff --git a/src/components/mail/listing/plain.rs b/src/components/mail/listing/plain.rs index 75401419..783d3606 100644 --- a/src/components/mail/listing/plain.rs +++ b/src/components/mail/listing/plain.rs @@ -294,12 +294,8 @@ impl MailListingTrait for PlainListing { let thread_nodes: &HashMap = threads.thread_nodes(); let env_hash_iter = Box::new( threads - .threads_group_iter(roots) - .filter_map(|(_, thread_node_hash, _)| { - let thread_node = &thread_nodes[&thread_node_hash]; - - thread_node.message() - }) + .threads_iter(roots) + .filter_map(|(_, thread_node_hash, _)| thread_nodes[&thread_node_hash].message()) .collect::>() .into_iter(), ) as Box>; diff --git a/src/components/mail/listing/thread.rs b/src/components/mail/listing/thread.rs index ab5c1827..7fee8513 100644 --- a/src/components/mail/listing/thread.rs +++ b/src/components/mail/listing/thread.rs @@ -19,7 +19,7 @@ * along with meli. If not, see . */ -use std::{cmp, convert::TryInto, fmt::Write, iter::FromIterator}; +use std::{cmp, convert::TryInto, iter::FromIterator}; use melib::{ThreadNode, Threads}; @@ -262,11 +262,11 @@ impl MailListingTrait for ThreadListing { SmallVec::new(), ); - let mut indentations: Vec = Vec::with_capacity(6); let roots = items .filter_map(|r| threads.groups[&r].root().map(|r| r.root)) - .collect::<_>(); - let mut iter = threads.threads_group_iter(roots).peekable(); + .collect::>(); + let mut indentations: Vec = Vec::with_capacity(6); + let mut iter = threads.threads_iter(roots).peekable(); let thread_nodes: &HashMap = threads.thread_nodes(); /* This is just a desugared for loop so that we can use .peek() */ let mut idx: usize = 0; @@ -364,8 +364,6 @@ impl MailListingTrait for ThreadListing { ); self.rows.row_attr_cache.insert(idx, row_attr); idx += 1; - } else { - continue; } match iter.peek() { @@ -782,25 +780,36 @@ impl ThreadListing { is_root: bool, ) -> String { let thread_node = &threads[&node_idx]; - let has_parent = thread_node.has_parent() && !is_root; + let has_parent = thread_node.has_parent(); + let has_visible_parent = has_parent && !is_root; let show_subject = thread_node.show_subject(); - let mut s = String::new(); //format!("{}{}{} ", idx, " ", ThreadListing::format_date(&envelope)); - for i in 0..indent { - if indentations.len() > i && indentations[i] { - s.push('│'); - } else if indentations.len() > i { - s.push(' '); - } - if i > 0 { - s.push(' '); + let mut s = String::new(); + + // Do not print any indentation if entry is a root but it has a parent that is + // missing AND it has no siblings; therefore there's no point in + // printing anything before the root's level in the thread tree. It + // would just be empty space. + if !(is_root && has_parent && !has_sibling) { + for i in 0..indent { + if indentations.len() > i && indentations[i] { + s.push('│'); + } else if indentations.len() > i { + s.push(' '); + } + if i > 0 { + s.push(' '); + } } } - if indent > 0 && (has_sibling || has_parent) { - if has_sibling && has_parent { + + if indent > 0 && ((has_sibling || has_visible_parent) || is_root) { + if has_sibling && has_visible_parent { s.push('├'); } else if has_sibling { s.push('┬'); + } else if has_parent && is_root { + s.push('─'); } else { s.push('└'); } @@ -808,15 +817,8 @@ impl ThreadListing { s.push('>'); } - /* - s.push_str(if envelope.has_attachments() { - "📎" - } else { - "" - }); - */ if show_subject { - let _ = write!(s, "{:.85}", envelope.subject()); + s.push_str(&envelope.subject()); } s } diff --git a/src/components/mail/view/thread.rs b/src/components/mail/view/thread.rs index 0104ecc6..fe987186 100644 --- a/src/components/mail/view/thread.rs +++ b/src/components/mail/view/thread.rs @@ -208,7 +208,7 @@ impl ThreadView { } let (account_hash, mailbox_hash, _) = self.coordinates; - let thread_iter = threads.thread_group_iter(self.thread_group); + let thread_iter = threads.thread_iter(self.thread_group); self.entries.clear(); for (line, (ind, thread_node_hash)) in thread_iter.enumerate() { let entry = if let Some(msg_hash) = threads.thread_nodes()[&thread_node_hash].message() @@ -677,11 +677,7 @@ impl ThreadView { clear_area(grid, area, theme_default); let account = &context.accounts[&self.coordinates.0]; let threads = account.collection.get_threads(self.coordinates.1); - let thread_root = threads - .thread_group_iter(self.thread_group) - .next() - .unwrap() - .1; + let thread_root = threads.thread_iter(self.thread_group).next().unwrap().1; let thread_node = &threads.thread_nodes()[&thread_root]; let i = thread_node.message().unwrap_or_else(|| { let mut iter_ptr = thread_node.children()[0]; @@ -791,11 +787,7 @@ impl ThreadView { clear_area(grid, area, theme_default); let account = &context.accounts[&self.coordinates.0]; let threads = account.collection.get_threads(self.coordinates.1); - let thread_root = threads - .thread_group_iter(self.thread_group) - .next() - .unwrap() - .1; + let thread_root = threads.thread_iter(self.thread_group).next().unwrap().1; let thread_node = &threads.thread_nodes()[&thread_root]; let i = thread_node.message().unwrap_or_else(|| { let mut iter_ptr = thread_node.children()[0];