From 17a0f31b3e2b88e15aba1662fd1ee5bd828b91fd Mon Sep 17 00:00:00 2001 From: Manos Pitsidianakis Date: Sat, 14 Dec 2019 19:56:43 +0200 Subject: [PATCH] ui/accounts: split StartupCheck event semantics UIEvent::StartupCheck was meant to notify the UI that a folder had made progress and polling its async worker would return a Result>. However the StartupCheck was received by MailListing components which called account.status() which did the polling. That means that if the polling got back results, the listing would have to call account.status() again to show them. This is a problem in configurations with only one account because there aren't any other sources of event to force the listing to recheck account.status() A new event UIEvent::WorkerProgress will do the job of notifying an Account to poll its workers and the account will send a startupcheck if it has made progress. That way the refresh progress is as follows: Worker thread sends WorkerProgress event -> State calls appropriate account's account.status() method -> account polls workers, and if there are new results send StartupCheck events -> State passes StartupCheck events to components -> Listings update themselves when they receive the event --- src/bin.rs | 2 +- ui/src/conf/accounts.rs | 99 ++++++++++++++++++++++------------------- ui/src/state.rs | 8 +++- ui/src/types.rs | 1 + 4 files changed, 63 insertions(+), 47 deletions(-) diff --git a/src/bin.rs b/src/bin.rs index 8f0f1899..187d1d28 100644 --- a/src/bin.rs +++ b/src/bin.rs @@ -367,7 +367,7 @@ fn run_app() -> Result<()> { } ThreadEvent::UIEvent(e) => { state.rcv_event(e); - state.render(); + state.redraw(); }, ThreadEvent::Pulse => { state.check_accounts(); diff --git a/ui/src/conf/accounts.rs b/ui/src/conf/accounts.rs index b19dd6e6..198c5c1a 100644 --- a/ui/src/conf/accounts.rs +++ b/ui/src/conf/accounts.rs @@ -809,6 +809,9 @@ impl Account { if payload.is_err() { self.folders .insert(folder_hash, MailboxEntry::Failed(payload.unwrap_err())); + self.sender + .send(ThreadEvent::UIEvent(UIEvent::StartupCheck(folder_hash))) + .unwrap(); return; } let envelopes = payload @@ -825,60 +828,66 @@ impl Account { .merge(envelopes, folder_hash, self.sent_folder) { for f in updated_folders { - self.notify_fn.notify(f); + self.sender + .send(ThreadEvent::UIEvent(UIEvent::StartupCheck(f))) + .unwrap(); } } } } - self.notify_fn.notify(folder_hash); + self.sender + .send(ThreadEvent::UIEvent(UIEvent::StartupCheck(folder_hash))) + .unwrap(); } pub fn status(&mut self, folder_hash: FolderHash) -> result::Result<(), usize> { - match self.workers.get_mut(&folder_hash).unwrap() { - None => { - return if self.folders[&folder_hash].is_available() - || (self.folders[&folder_hash].is_parsing() - && self.collection.threads.contains_key(&folder_hash)) - { - Ok(()) - } else { - Err(0) - }; - } - Some(ref mut w) => match w.poll() { - Ok(AsyncStatus::NoUpdate) => { - //return Err(0); + loop { + match self.workers.get_mut(&folder_hash).unwrap() { + None => { + return if self.folders[&folder_hash].is_available() + || (self.folders[&folder_hash].is_parsing() + && self.collection.threads.contains_key(&folder_hash)) + { + Ok(()) + } else { + Err(0) + }; } - Ok(AsyncStatus::Payload(envs)) => { - debug!("got payload in status for {}", folder_hash); - self.load_mailbox(folder_hash, envs); - } - Ok(AsyncStatus::Finished) => { - debug!("got finished in status for {}", folder_hash); - self.folders.entry(folder_hash).and_modify(|f| { - let m = if let MailboxEntry::Parsing(m, _, _) = f { - std::mem::replace(m, Mailbox::default()) - } else { - return; - }; - *f = MailboxEntry::Available(m); - }); + Some(ref mut w) => match debug!(w.poll()) { + Ok(AsyncStatus::NoUpdate) => { + break; + } + Ok(AsyncStatus::Payload(envs)) => { + debug!("got payload in status for {}", folder_hash); + self.load_mailbox(folder_hash, envs); + } + Ok(AsyncStatus::Finished) => { + debug!("got finished in status for {}", folder_hash); + self.folders.entry(folder_hash).and_modify(|f| { + let m = if let MailboxEntry::Parsing(m, _, _) = f { + std::mem::replace(m, Mailbox::default()) + } else { + return; + }; + *f = MailboxEntry::Available(m); + }); - self.workers.insert(folder_hash, None); - } - Ok(AsyncStatus::ProgressReport(n)) => { - self.folders.entry(folder_hash).and_modify(|f| { - if let MailboxEntry::Parsing(_, ref mut d, _) = f { - *d += n; - } - }); - //return Err(n); - } - _ => { - //return Err(0); - } - }, - }; + self.workers.insert(folder_hash, None); + } + Ok(AsyncStatus::ProgressReport(n)) => { + self.folders.entry(folder_hash).and_modify(|f| { + if let MailboxEntry::Parsing(_, ref mut d, _) = f { + *d += n; + } + }); + //return Err(n); + } + _ => { + break; + } + }, + }; + } if self.folders[&folder_hash].is_available() || (self.folders[&folder_hash].is_parsing() && self.collection.threads.contains_key(&folder_hash)) diff --git a/ui/src/state.rs b/ui/src/state.rs index 79c75679..ab5f84dd 100644 --- a/ui/src/state.rs +++ b/ui/src/state.rs @@ -230,7 +230,7 @@ impl State { sender.clone(), NotifyFn::new(Box::new(move |f: FolderHash| { sender - .send(ThreadEvent::UIEvent(UIEvent::StartupCheck(f))) + .send(ThreadEvent::UIEvent(UIEvent::WorkerProgress(f))) .unwrap(); })), ) @@ -639,6 +639,12 @@ impl State { self.child = Some(child); return; } + UIEvent::WorkerProgress(folder_hash) => { + if let Some(&account_idx) = self.context.mailbox_hashes.get(&folder_hash) { + let _ = self.context.accounts[account_idx].status(folder_hash); + } + return; + } UIEvent::ChangeMode(m) => { if self.mode == UIMode::Embed { self.context.input_from_raw(); diff --git a/ui/src/types.rs b/ui/src/types.rs index f5d9344d..8b782ef8 100644 --- a/ui/src/types.rs +++ b/ui/src/types.rs @@ -101,6 +101,7 @@ pub enum UIEvent { StatusEvent(StatusEvent), MailboxUpdate((usize, FolderHash)), // (account_idx, mailbox_idx) ComponentKill(Uuid), + WorkerProgress(FolderHash), StartupCheck(FolderHash), RefreshEvent(Box), EnvelopeUpdate(EnvelopeHash),