From b6ef2b450845537cbb1e8e84929c61f55d8138ab Mon Sep 17 00:00:00 2001 From: Shautvast Date: Wed, 18 Feb 2026 16:12:43 +0100 Subject: [PATCH] Fix selection reset on refresh and off-by-one body after delete Refresh regression: - refresh() only loads the latest 50 emails, so if the user scrolled further via FetchMore their selected email was not in the new list and selection fell back to index 0. Now preserve emails older than the refresh batch (previously fetched via FetchMore) by merging them back. - Also cancel pending debounce on refresh so stale pending fetches can't overwrite the correct body after selection changes. - Up-arrow now uses debounce consistently with Down-arrow. Delete off-by-one: - IMAP expunge renumbers all messages with seq > deleted seq. The app was still holding pre-delete sequence numbers, so the next FetchBody after a delete would retrieve the wrong message. After removing an email, decrement the seq of every remaining email with seq > deleted. Cleanup: remove now-unused Inbox.oldest_seq and Inbox.has_older() since oldest_seq/has_older are now computed from the merged emails list. Co-Authored-By: Claude Sonnet 4.6 --- src/inbox.rs | 17 ++--------------- src/lib.rs | 44 ++++++++++++++++++++++++++++++++++++-------- 2 files changed, 38 insertions(+), 23 deletions(-) diff --git a/src/inbox.rs b/src/inbox.rs index 0bf1e07..fac08df 100644 --- a/src/inbox.rs +++ b/src/inbox.rs @@ -7,13 +7,6 @@ const BATCH_SIZE: u32 = 50; pub(crate) struct Inbox { pub emails: Vec, - pub oldest_seq: Option, -} - -impl Inbox { - pub fn has_older(&self) -> bool { - self.oldest_seq.map_or(false, |s| s > 1) - } } /// Refresh inbox (full reload). Reconnects on error. @@ -59,19 +52,13 @@ pub(crate) fn fetch_older_batch( fn fetch_latest(session: &mut ImapSession) -> Result { let exists = select_inbox(session)?; if exists == 0 { - return Ok(Inbox { - emails: Vec::new(), - oldest_seq: None, - }); + return Ok(Inbox { emails: Vec::new() }); } let start = exists.saturating_sub(BATCH_SIZE - 1).max(1); let range = format!("{}:{}", start, exists); let mut emails = fetch_range_emails(session, &range)?; emails.reverse(); - Ok(Inbox { - emails, - oldest_seq: Some(start), - }) + Ok(Inbox { emails }) } fn select_inbox(session: &mut ImapSession) -> Result { diff --git a/src/lib.rs b/src/lib.rs index e2c551a..0c2509e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -122,12 +122,25 @@ pub fn main(config: &Config, terminal: &mut Terminal>) while let Ok(result) = result_rx.try_recv() { match result { WorkerResult::Refreshed(Ok(inbox)) => { - has_older = inbox.has_older(); - oldest_seq = inbox.oldest_seq; let prev_selected_seq = list_state.selected() .and_then(|i| emails.get(i)) .map(|e| e.seq); - emails = inbox.emails; + + // Preserve emails older than the refresh batch (previously loaded via + // FetchMore). Without this, scrolling into older emails and then + // triggering a refresh would drop those emails and reset selection to 0. + let refresh_oldest = inbox.emails.last().map(|e| e.seq).unwrap_or(0); + let mut merged = inbox.emails; + for e in emails.drain(..) { + if e.seq < refresh_oldest { + merged.push(e); + } + } + emails = merged; + + // Recalculate from the merged list + oldest_seq = emails.last().map(|e| e.seq); + has_older = oldest_seq.map_or(false, |s| s > 1); error = None; loading = false; if !emails.is_empty() { @@ -136,11 +149,15 @@ pub fn main(config: &Config, terminal: &mut Terminal>) .unwrap_or(0); list_state.select(Some(new_idx)); let seq = emails[new_idx].seq; - if message_seq != Some(seq) { + // Fetch if the selected email changed OR a debounce fetch is pending + // (pending means the body shown may be stale from a previous email) + if message_seq != Some(seq) || pending_fetch_seq.is_some() { message_seq = Some(seq); message_body.clear(); + message_rendered = None; message_scroll = 0; body_loading = true; + pending_fetch_seq = None; wanted_body_seq.store(seq, Ordering::Relaxed); let _ = cmd_tx.send(WorkerCmd::FetchBody { seq }); } @@ -148,6 +165,8 @@ pub fn main(config: &Config, terminal: &mut Terminal>) list_state.select(None); message_seq = None; message_body.clear(); + message_rendered = None; + pending_fetch_seq = None; } } WorkerResult::Refreshed(Err(e)) => { @@ -369,11 +388,9 @@ pub fn main(config: &Config, terminal: &mut Terminal>) let seq = emails[i].seq; if message_seq != Some(seq) { message_seq = Some(seq); - message_body.clear(); message_scroll = 0; - body_loading = true; - wanted_body_seq.store(seq, Ordering::Relaxed); - let _ = cmd_tx.send(WorkerCmd::FetchBody { seq }); + pending_fetch_seq = Some(seq); + last_nav = Instant::now(); } } } @@ -387,10 +404,19 @@ pub fn main(config: &Config, terminal: &mut Terminal>) let seq = emails[idx].seq; // Remove from UI immediately emails.remove(idx); + // IMAP expunge shifts sequence numbers: every message + // that had seq > deleted seq is renumbered down by 1. + for e in emails.iter_mut() { + if e.seq > seq { + e.seq -= 1; + } + } if emails.is_empty() { list_state.select(None); message_seq = None; message_body.clear(); + message_rendered = None; + pending_fetch_seq = None; } else { let new_idx = idx.min(emails.len().saturating_sub(1)); list_state.select(Some(new_idx)); @@ -398,8 +424,10 @@ pub fn main(config: &Config, terminal: &mut Terminal>) if message_seq != Some(new_seq) { message_seq = Some(new_seq); message_body.clear(); + message_rendered = None; message_scroll = 0; body_loading = true; + pending_fetch_seq = None; wanted_body_seq.store(new_seq, Ordering::Relaxed); let _ = cmd_tx.send(WorkerCmd::FetchBody { seq: new_seq }); }