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

@@ -744,11 +744,8 @@ impl Threads {
         }
     }
 
-    pub fn threads_group_iter(
+    pub fn threads_iter(&self, root_tree: SmallVec<[ThreadNodeHash; 1024]>) -> ThreadsIterator {
-        &self,
+        ThreadsIterator {
-        root_tree: SmallVec<[ThreadNodeHash; 1024]>,
-    ) -> ThreadsGroupIterator {
-        ThreadsGroupIterator {
             root_tree,
             pos: 0,
             stack: SmallVec::new(),
@@ -756,8 +753,8 @@ impl Threads {
         }
     }
 
-    pub fn thread_group_iter(&self, index: ThreadHash) -> ThreadGroupIterator {
+    pub fn thread_iter(&self, index: ThreadHash) -> ThreadIterator {
-        ThreadGroupIterator {
+        ThreadIterator {
             group: self.thread_ref(index).root(),
             pos: 0,
             stack: SmallVec::new(),

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
+/// [`ThreadsIterator`] returns messages according to the sorted order.
- * example, for the following threads:
+///
- *
+/// For example, for the following threads:
- *  ```
+///
- *  A_
+/// ```text
- *   |_ B
+/// A_
- *   |_C
+///  |_ B
- *  D
+///  |_C
- *  E_
+/// D
- *   |_F
+/// E_
- *   ```
+///  |_F
- *
+/// ```
- *   the iterator returns them as `A, B, C, D, E, F`
+///
- */
+/// the iterator returns them as `A, B, C, D, E, F`.
-
+pub struct ThreadsIterator<'a> {
-pub struct ThreadsGroupIterator<'a> {
     pub(super) root_tree: SmallVec<[ThreadNodeHash; 1024]>,
     pub(super) pos: usize,
     pub(super) stack: SmallVec<[usize; 16]>,
     pub(super) thread_nodes: &'a HashMap<ThreadNodeHash, ThreadNode>,
 }
-impl<'a> Iterator for ThreadsGroupIterator<'a> {
+
+impl<'a> Iterator for ThreadsIterator<'a> {
     type Item = (usize, ThreadNodeHash, bool);
     fn next(&mut self) -> Option<Self::Item> {
         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<ThreadNodeHash, ThreadNode>,
 }
 
-impl<'a> Iterator for ThreadGroupIterator<'a> {
+impl<'a> Iterator for ThreadIterator<'a> {
     type Item = (usize, ThreadNodeHash);
     fn next(&mut self) -> Option<(usize, ThreadNodeHash)> {
         loop {

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);

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<Vec<u8>> =
             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()

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<Vec<u8>> =
             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()

src/components/mail/listing/plain.rs

@@ -294,12 +294,8 @@ impl MailListingTrait for PlainListing {
         let thread_nodes: &HashMap<ThreadNodeHash, ThreadNode> = threads.thread_nodes();
         let env_hash_iter = Box::new(
             threads
-                .threads_group_iter(roots)
+                .threads_iter(roots)
-                .filter_map(|(_, thread_node_hash, _)| {
+                .filter_map(|(_, thread_node_hash, _)| thread_nodes[&thread_node_hash].message())
-                    let thread_node = &thread_nodes[&thread_node_hash];
-
-                    thread_node.message()
-                })
                 .collect::<SmallVec<[EnvelopeHash; 2048]>>()
                 .into_iter(),
         ) as Box<dyn Iterator<Item = EnvelopeHash>>;

src/components/mail/listing/thread.rs

@@ -19,7 +19,7 @@
  * along with meli. If not, see <http://www.gnu.org/licenses/>.
  */
 
-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<bool> = Vec::with_capacity(6);
         let roots = items
             .filter_map(|r| threads.groups[&r].root().map(|r| r.root))
-            .collect::<_>();
+            .collect::<SmallVec<[ThreadNodeHash; 1024]>>();
-        let mut iter = threads.threads_group_iter(roots).peekable();
+        let mut indentations: Vec<bool> = Vec::with_capacity(6);
+        let mut iter = threads.threads_iter(roots).peekable();
         let thread_nodes: &HashMap<ThreadNodeHash, ThreadNode> = 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));
+        let mut s = String::new();
-        for i in 0..indent {
+
-            if indentations.len() > i && indentations[i] {
+        // Do not print any indentation if entry is a root but it has a parent that is
-                s.push('│');
+        // missing AND it has no siblings; therefore there's no point in
-            } else if indentations.len() > i {
+        // printing anything before the root's level in the thread tree. It
-                s.push(' ');
+        // would just be empty space.
-            }
+        if !(is_root && has_parent && !has_sibling) {
-            if i > 0 {
+            for i in 0..indent {
-                s.push(' ');
+                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
     }

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
+            let thread_root = threads.thread_iter(self.thread_group).next().unwrap().1;
-                .thread_group_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
+            let thread_root = threads.thread_iter(self.thread_group).next().unwrap().1;
-                .thread_group_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];