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 <noreply@anthropic.com>
This commit is contained in:
parent
12eb683007
commit
b6ef2b4508
2 changed files with 38 additions and 23 deletions
17
src/inbox.rs
17
src/inbox.rs
|
|
@ -7,13 +7,6 @@ const BATCH_SIZE: u32 = 50;
|
||||||
|
|
||||||
pub(crate) struct Inbox {
|
pub(crate) struct Inbox {
|
||||||
pub emails: Vec<Email>,
|
pub emails: Vec<Email>,
|
||||||
pub oldest_seq: Option<u32>,
|
|
||||||
}
|
|
||||||
|
|
||||||
impl Inbox {
|
|
||||||
pub fn has_older(&self) -> bool {
|
|
||||||
self.oldest_seq.map_or(false, |s| s > 1)
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Refresh inbox (full reload). Reconnects on error.
|
/// Refresh inbox (full reload). Reconnects on error.
|
||||||
|
|
@ -59,19 +52,13 @@ pub(crate) fn fetch_older_batch(
|
||||||
fn fetch_latest(session: &mut ImapSession) -> Result<Inbox, String> {
|
fn fetch_latest(session: &mut ImapSession) -> Result<Inbox, String> {
|
||||||
let exists = select_inbox(session)?;
|
let exists = select_inbox(session)?;
|
||||||
if exists == 0 {
|
if exists == 0 {
|
||||||
return Ok(Inbox {
|
return Ok(Inbox { emails: Vec::new() });
|
||||||
emails: Vec::new(),
|
|
||||||
oldest_seq: None,
|
|
||||||
});
|
|
||||||
}
|
}
|
||||||
let start = exists.saturating_sub(BATCH_SIZE - 1).max(1);
|
let start = exists.saturating_sub(BATCH_SIZE - 1).max(1);
|
||||||
let range = format!("{}:{}", start, exists);
|
let range = format!("{}:{}", start, exists);
|
||||||
let mut emails = fetch_range_emails(session, &range)?;
|
let mut emails = fetch_range_emails(session, &range)?;
|
||||||
emails.reverse();
|
emails.reverse();
|
||||||
Ok(Inbox {
|
Ok(Inbox { emails })
|
||||||
emails,
|
|
||||||
oldest_seq: Some(start),
|
|
||||||
})
|
|
||||||
}
|
}
|
||||||
|
|
||||||
fn select_inbox(session: &mut ImapSession) -> Result<u32, String> {
|
fn select_inbox(session: &mut ImapSession) -> Result<u32, String> {
|
||||||
|
|
|
||||||
44
src/lib.rs
44
src/lib.rs
|
|
@ -122,12 +122,25 @@ pub fn main(config: &Config, terminal: &mut Terminal<CrosstermBackend<Stdout>>)
|
||||||
while let Ok(result) = result_rx.try_recv() {
|
while let Ok(result) = result_rx.try_recv() {
|
||||||
match result {
|
match result {
|
||||||
WorkerResult::Refreshed(Ok(inbox)) => {
|
WorkerResult::Refreshed(Ok(inbox)) => {
|
||||||
has_older = inbox.has_older();
|
|
||||||
oldest_seq = inbox.oldest_seq;
|
|
||||||
let prev_selected_seq = list_state.selected()
|
let prev_selected_seq = list_state.selected()
|
||||||
.and_then(|i| emails.get(i))
|
.and_then(|i| emails.get(i))
|
||||||
.map(|e| e.seq);
|
.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;
|
error = None;
|
||||||
loading = false;
|
loading = false;
|
||||||
if !emails.is_empty() {
|
if !emails.is_empty() {
|
||||||
|
|
@ -136,11 +149,15 @@ pub fn main(config: &Config, terminal: &mut Terminal<CrosstermBackend<Stdout>>)
|
||||||
.unwrap_or(0);
|
.unwrap_or(0);
|
||||||
list_state.select(Some(new_idx));
|
list_state.select(Some(new_idx));
|
||||||
let seq = emails[new_idx].seq;
|
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_seq = Some(seq);
|
||||||
message_body.clear();
|
message_body.clear();
|
||||||
|
message_rendered = None;
|
||||||
message_scroll = 0;
|
message_scroll = 0;
|
||||||
body_loading = true;
|
body_loading = true;
|
||||||
|
pending_fetch_seq = None;
|
||||||
wanted_body_seq.store(seq, Ordering::Relaxed);
|
wanted_body_seq.store(seq, Ordering::Relaxed);
|
||||||
let _ = cmd_tx.send(WorkerCmd::FetchBody { seq });
|
let _ = cmd_tx.send(WorkerCmd::FetchBody { seq });
|
||||||
}
|
}
|
||||||
|
|
@ -148,6 +165,8 @@ pub fn main(config: &Config, terminal: &mut Terminal<CrosstermBackend<Stdout>>)
|
||||||
list_state.select(None);
|
list_state.select(None);
|
||||||
message_seq = None;
|
message_seq = None;
|
||||||
message_body.clear();
|
message_body.clear();
|
||||||
|
message_rendered = None;
|
||||||
|
pending_fetch_seq = None;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
WorkerResult::Refreshed(Err(e)) => {
|
WorkerResult::Refreshed(Err(e)) => {
|
||||||
|
|
@ -369,11 +388,9 @@ pub fn main(config: &Config, terminal: &mut Terminal<CrosstermBackend<Stdout>>)
|
||||||
let seq = emails[i].seq;
|
let seq = emails[i].seq;
|
||||||
if message_seq != Some(seq) {
|
if message_seq != Some(seq) {
|
||||||
message_seq = Some(seq);
|
message_seq = Some(seq);
|
||||||
message_body.clear();
|
|
||||||
message_scroll = 0;
|
message_scroll = 0;
|
||||||
body_loading = true;
|
pending_fetch_seq = Some(seq);
|
||||||
wanted_body_seq.store(seq, Ordering::Relaxed);
|
last_nav = Instant::now();
|
||||||
let _ = cmd_tx.send(WorkerCmd::FetchBody { seq });
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -387,10 +404,19 @@ pub fn main(config: &Config, terminal: &mut Terminal<CrosstermBackend<Stdout>>)
|
||||||
let seq = emails[idx].seq;
|
let seq = emails[idx].seq;
|
||||||
// Remove from UI immediately
|
// Remove from UI immediately
|
||||||
emails.remove(idx);
|
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() {
|
if emails.is_empty() {
|
||||||
list_state.select(None);
|
list_state.select(None);
|
||||||
message_seq = None;
|
message_seq = None;
|
||||||
message_body.clear();
|
message_body.clear();
|
||||||
|
message_rendered = None;
|
||||||
|
pending_fetch_seq = None;
|
||||||
} else {
|
} else {
|
||||||
let new_idx = idx.min(emails.len().saturating_sub(1));
|
let new_idx = idx.min(emails.len().saturating_sub(1));
|
||||||
list_state.select(Some(new_idx));
|
list_state.select(Some(new_idx));
|
||||||
|
|
@ -398,8 +424,10 @@ pub fn main(config: &Config, terminal: &mut Terminal<CrosstermBackend<Stdout>>)
|
||||||
if message_seq != Some(new_seq) {
|
if message_seq != Some(new_seq) {
|
||||||
message_seq = Some(new_seq);
|
message_seq = Some(new_seq);
|
||||||
message_body.clear();
|
message_body.clear();
|
||||||
|
message_rendered = None;
|
||||||
message_scroll = 0;
|
message_scroll = 0;
|
||||||
body_loading = true;
|
body_loading = true;
|
||||||
|
pending_fetch_seq = None;
|
||||||
wanted_body_seq.store(new_seq, Ordering::Relaxed);
|
wanted_body_seq.store(new_seq, Ordering::Relaxed);
|
||||||
let _ = cmd_tx.send(WorkerCmd::FetchBody { seq: new_seq });
|
let _ = cmd_tx.send(WorkerCmd::FetchBody { seq: new_seq });
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue