From 45c0160cb60fcabd55cd1dc1277bdcb7ddc9f691 Mon Sep 17 00:00:00 2001 From: Manos Pitsidianakis Date: Sun, 23 Feb 2020 15:01:37 +0200 Subject: [PATCH] Fix ThreadListing ThreadListing was broken after the ThreadGroup introduction --- melib/src/thread.rs | 41 +++----- melib/src/thread/iterators.rs | 76 +------------- src/components/mail/listing/thread.rs | 146 ++++++++++++++------------ 3 files changed, 97 insertions(+), 166 deletions(-) diff --git a/melib/src/thread.rs b/melib/src/thread.rs index 1a7eb0ac6..9eeb86bd3 100644 --- a/melib/src/thread.rs +++ b/melib/src/thread.rs @@ -272,13 +272,13 @@ impl FromStr for SortOrder { #[derive(Default, Clone, Debug, Deserialize, Serialize)] pub struct Thread { - root: ThreadNodeHash, - date: UnixTimestamp, - len: usize, - unseen: usize, - attachments: usize, + pub root: ThreadNodeHash, + pub date: UnixTimestamp, + pub len: usize, + pub unseen: usize, + pub attachments: usize, - snoozed: bool, + pub snoozed: bool, } #[derive(Clone, Debug, Deserialize, Serialize)] @@ -294,7 +294,7 @@ impl Default for ThreadGroup { } impl ThreadGroup { - fn root(&self) -> Option<&Thread> { + pub fn root(&self) -> Option<&Thread> { if let ThreadGroup::Root(ref root) = self { Some(root) } else { @@ -498,21 +498,14 @@ impl Threads { } } - pub fn threads_iter(&self) -> ThreadsIterator { - ThreadsIterator { + pub fn threads_group_iter( + &self, + root_tree: SmallVec<[ThreadNodeHash; 1024]>, + ) -> ThreadsGroupIterator { + ThreadsGroupIterator { + root_tree, pos: 0, stack: SmallVec::new(), - root_tree: self.tree_index.borrow(), - thread_nodes: &self.thread_nodes, - } - } - - pub fn thread_iter(&self, index: usize) -> ThreadIterator { - ThreadIterator { - init_pos: index, - pos: index, - stack: SmallVec::new(), - root_tree: self.tree_index.borrow(), thread_nodes: &self.thread_nodes, } } @@ -1191,14 +1184,6 @@ impl Threads { .filter_map(|(h, g)| g.root().map(|_| *h)) .collect::>() } - - pub fn root_iter(&self) -> RootIterator { - RootIterator { - pos: 0, - root_tree: self.tree_index.borrow(), - thread_nodes: &self.thread_nodes, - } - } } impl Index<&ThreadNodeHash> for Threads { diff --git a/melib/src/thread/iterators.rs b/melib/src/thread/iterators.rs index ad7a50cea..835dad6a3 100644 --- a/melib/src/thread/iterators.rs +++ b/melib/src/thread/iterators.rs @@ -22,7 +22,6 @@ use super::{ThreadNode, ThreadNodeHash}; use fnv::FnvHashMap; use smallvec::SmallVec; -use std::cell::Ref; /* `ThreadsIterator` returns messages according to the sorted order. For example, for the following * threads: @@ -39,13 +38,13 @@ use std::cell::Ref; * 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) root_tree: Ref<'a, Vec>, pub(super) thread_nodes: &'a FnvHashMap, } -impl<'a> Iterator for ThreadsIterator<'a> { +impl<'a> Iterator for ThreadsGroupIterator<'a> { type Item = (usize, ThreadNodeHash, bool); fn next(&mut self) -> Option<(usize, ThreadNodeHash, bool)> { { @@ -97,75 +96,6 @@ impl<'a> Iterator for ThreadsIterator<'a> { * the iterator returns them as `A, B, C, D` */ -pub struct ThreadIterator<'a> { - pub(super) init_pos: usize, - pub(super) pos: usize, - pub(super) stack: SmallVec<[usize; 16]>, - pub(super) root_tree: Ref<'a, Vec>, - pub(super) thread_nodes: &'a FnvHashMap, -} -impl<'a> Iterator for ThreadIterator<'a> { - type Item = (usize, ThreadNodeHash); - fn next(&mut self) -> Option<(usize, ThreadNodeHash)> { - { - let mut tree = &(*self.root_tree); - for i in self.stack.iter() { - tree = &self.thread_nodes[&tree[*i]].children; - } - if self.pos == tree.len() || (self.stack.is_empty() && self.pos > self.init_pos) { - if self.stack.is_empty() { - return None; - } - self.pos = self.stack.pop().unwrap() + 1; - } else { - debug_assert!(self.pos < tree.len()); - let ret = (self.stack.len(), tree[self.pos]); - 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 { - return self.next(); - } - } - self.pos += 1; - if self.thread_nodes[&ret.1].message.is_some() { - return Some(ret); - } - } - } - self.next() - } -} - -pub struct RootIterator<'a> { - pub pos: usize, - pub root_tree: Ref<'a, Vec>, - pub thread_nodes: &'a FnvHashMap, -} - -impl<'a> Iterator for RootIterator<'a> { - type Item = ThreadNodeHash; - fn next(&mut self) -> Option { - { - if self.pos == self.root_tree.len() { - return None; - } - let mut ret = self.root_tree[self.pos]; - self.pos += 1; - let thread_node = &self.thread_nodes[&ret]; - if thread_node.message().is_none() { - ret = thread_node.children()[0]; - while self.thread_nodes[&ret].message().is_none() { - ret = self.thread_nodes[&ret].children()[0]; - } - } - Some(ret) - } - } -} - pub struct ThreadGroupIterator<'a> { pub(super) group: ThreadNodeHash, pub(super) pos: usize, diff --git a/src/components/mail/listing/thread.rs b/src/components/mail/listing/thread.rs index 27ca503d0..52e8bcf4f 100644 --- a/src/components/mail/listing/thread.rs +++ b/src/components/mail/listing/thread.rs @@ -39,7 +39,7 @@ pub struct ThreadListing { color_cache: ColorCache, row_updates: SmallVec<[ThreadHash; 8]>, - locations: Vec, + order: FnvHashMap, /// If we must redraw on next redraw event dirty: bool, /// If `self.view` is focused or not. @@ -117,8 +117,8 @@ impl MailListingTrait for ThreadListing { } let account = &context.accounts[self.cursor_pos.0]; let threads = &account.collection.threads[&self.cursor_pos.1]; - self.length = threads.len(); - self.locations.clear(); + self.length = 0; + self.order.clear(); let default_cell = { let mut ret = Cell::with_char(' '); ret.set_fg(self.color_cache.theme_default.fg) @@ -126,7 +126,7 @@ impl MailListingTrait for ThreadListing { .set_attrs(self.color_cache.theme_default.attrs); ret }; - if self.length == 0 { + if threads.len() == 0 { let message = format!("Folder `{}` is empty.", account[&self.cursor_pos.1].name()); self.content = CellBuffer::new_with_context(message.len(), 1, default_cell, context); write_string_to_grid( @@ -141,14 +141,19 @@ impl MailListingTrait for ThreadListing { return; } self.content = - CellBuffer::new_with_context(MAX_COLS, self.length + 1, default_cell, context); + CellBuffer::new_with_context(MAX_COLS, threads.len() + 1, default_cell, context); let mut indentations: Vec = Vec::with_capacity(6); let mut thread_idx = 0; // needed for alternate thread colors /* Draw threaded view. */ - threads.sort_by(self.sort, self.subsort, &account.collection.envelopes); + let mut roots = threads.roots(); + threads.group_inner_sort_by(&mut roots, self.sort, &account.collection.envelopes); + let roots = roots + .into_iter() + .filter_map(|r| threads.groups[&r].root().map(|r| r.root)) + .collect::<_>(); + let mut iter = threads.threads_group_iter(roots).peekable(); let thread_nodes: &FnvHashMap = &threads.thread_nodes(); - let mut iter = threads.threads_iter().peekable(); /* This is just a desugared for loop so that we can use .peek() */ let mut idx = 0; while let Some((indentation, thread_node_hash, has_sibling)) = iter.next() { @@ -160,7 +165,7 @@ impl MailListingTrait for ThreadListing { if thread_node.has_message() { let envelope: EnvelopeRef = account.collection.get_env(thread_node.message().unwrap()); - self.locations.push(envelope.hash()); + self.order.insert(envelope.hash(), idx); let fg_color = if !envelope.is_seen() { Color::Byte(0) } else { @@ -216,6 +221,7 @@ impl MailListingTrait for ThreadListing { _ => {} } } + self.length = self.order.len(); } } @@ -227,7 +233,7 @@ impl ListingTrait for ThreadListing { self.new_cursor_pos = (coordinates.0, coordinates.1, 0); self.unfocused = false; self.view = None; - self.locations.clear(); + self.order.clear(); self.row_updates.clear(); self.initialised = false; } @@ -240,7 +246,12 @@ impl ListingTrait for ThreadListing { let bottom_right = bottom_right!(area); if self.length == 0 { clear_area(grid, area, self.color_cache.theme_default); - copy_area(grid, &self.content, area, ((0, 0), (MAX_COLS - 1, 0))); + copy_area( + grid, + &self.content, + area, + ((0, 0), pos_dec(self.content.size(), (1, 1))), + ); context.dirty_areas.push_back(area); return; } @@ -346,31 +357,30 @@ impl ListingTrait for ThreadListing { } fn highlight_line(&mut self, grid: &mut CellBuffer, area: Area, idx: usize, context: &Context) { - if context.accounts[self.cursor_pos.0].collection[&self.cursor_pos.1].is_empty() { + if self.length == 0 { return; } - if self.locations[idx] != 0 { - let envelope: EnvelopeRef = context.accounts[self.cursor_pos.0] - .collection - .get_env(self.locations[idx]); + let env_hash = self.get_env_under_cursor(idx, context); + let envelope: EnvelopeRef = context.accounts[self.cursor_pos.0] + .collection + .get_env(env_hash); - let fg_color = if !envelope.is_seen() { - Color::Byte(0) - } else { - Color::Default - }; - let bg_color = if self.cursor_pos.2 == idx { - Color::Byte(246) - } else if !envelope.is_seen() { - Color::Byte(251) - } else if idx % 2 == 0 { - Color::Byte(236) - } else { - Color::Default - }; - change_colors(grid, area, fg_color, bg_color); - } + let fg_color = if !envelope.is_seen() { + Color::Byte(0) + } else { + Color::Default + }; + let bg_color = if self.cursor_pos.2 == idx { + Color::Byte(246) + } else if !envelope.is_seen() { + Color::Byte(251) + } else if idx % 2 == 0 { + Color::Byte(236) + } else { + Color::Default + }; + change_colors(grid, area, fg_color, bg_color); } fn set_movement(&mut self, mvm: PageMovement) { @@ -396,7 +406,7 @@ impl ThreadListing { content: CellBuffer::new(0, 0, Cell::with_char(' ')), color_cache: ColorCache::default(), row_updates: SmallVec::new(), - locations: Vec::new(), + order: FnvHashMap::default(), dirty: true, unfocused: false, view: None, @@ -407,33 +417,33 @@ impl ThreadListing { } fn highlight_line_self(&mut self, idx: usize, context: &Context) { - if context.accounts[self.cursor_pos.0].collection[&self.cursor_pos.1].is_empty() { + if self.length == 0 { return; } - if self.locations[idx] != 0 { - let envelope: EnvelopeRef = context.accounts[self.cursor_pos.0] - .collection - .get_env(self.locations[idx]); - let fg_color = if !envelope.is_seen() { - Color::Byte(0) - } else { - Color::Default - }; - let bg_color = if !envelope.is_seen() { - Color::Byte(251) - } else if idx % 2 == 0 { - Color::Byte(236) - } else { - Color::Default - }; - change_colors( - &mut self.content, - ((0, idx), (MAX_COLS - 1, idx)), - fg_color, - bg_color, - ); - } + let env_hash = self.get_env_under_cursor(idx, context); + let envelope: EnvelopeRef = context.accounts[self.cursor_pos.0] + .collection + .get_env(env_hash); + + let fg_color = if !envelope.is_seen() { + Color::Byte(0) + } else { + Color::Default + }; + let bg_color = if !envelope.is_seen() { + Color::Byte(251) + } else if idx % 2 == 0 { + Color::Byte(236) + } else { + Color::Default + }; + change_colors( + &mut self.content, + ((0, idx), (MAX_COLS - 1, idx)), + fg_color, + bg_color, + ); } fn make_thread_entry( @@ -483,6 +493,19 @@ impl ThreadListing { } s } + + fn get_env_under_cursor(&self, cursor: usize, _context: &Context) -> EnvelopeHash { + *self + .order + .iter() + .find(|(_, &r)| r == cursor) + .unwrap_or_else(|| { + debug!("self.order empty ? cursor={} {:#?}", cursor, &self.order); + panic!(); + }) + .0 + } + fn format_date(envelope: &Envelope) -> String { let d = std::time::UNIX_EPOCH + std::time::Duration::from_secs(envelope.date()); let now: std::time::Duration = std::time::SystemTime::now() @@ -531,13 +554,6 @@ impl Component for ThreadListing { let idx = self.cursor_pos.2; - let has_message: bool = self.locations[self.new_cursor_pos.2] > 0; - if !has_message { - self.dirty = false; - /* Draw the entire list */ - return self.draw_list(grid, area, context); - } - /* Mark message as read */ let must_highlight = { if self.length == 0 { @@ -546,7 +562,7 @@ impl Component for ThreadListing { let account = &context.accounts[self.cursor_pos.0]; let envelope: EnvelopeRef = account .collection - .get_env(self.locations[self.cursor_pos.2]); + .get_env(self.get_env_under_cursor(idx, context)); envelope.is_seen() } }; @@ -594,7 +610,7 @@ impl Component for ThreadListing { let coordinates = ( self.cursor_pos.0, self.cursor_pos.1, - self.locations[self.cursor_pos.2], + self.get_env_under_cursor(self.cursor_pos.2, context), ); if let Some(ref mut v) = self.view {