From 0f91ffdcf31eab9e4a51edc06841c5cff3461240 Mon Sep 17 00:00:00 2001 From: Michel de Graaf Date: Thu, 27 Aug 2026 13:22:39 +0200 Subject: [PATCH] feat(tui): dismiss a finished download with d MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A completed row used to ignore d: only a queued transfer could be pulled out of the list, so a finished one sat there until the next restart. d now goes through one handler that drops a finished row whatever the session thinks of it — a completed transfer can live only in this window, restored from an earlier run, where there is nothing to forget. The finished-status test was spelled out twice in clear_finished_downloads and had to agree by hand; both callers now share is_finished(). --- soulseek-rs/src/ui/main_tui/downloads.rs | 55 ++++++++++++--------- soulseek-rs/src/ui/main_tui/input.rs | 2 +- soulseek-rs/src/ui/main_tui/mod.rs | 61 +++++++++++++++++++++++- soulseek-rs/src/ui/main_tui/render.rs | 2 +- 4 files changed, 93 insertions(+), 27 deletions(-) diff --git a/soulseek-rs/src/ui/main_tui/downloads.rs b/soulseek-rs/src/ui/main_tui/downloads.rs index 9fb3722..5fefec1 100644 --- a/soulseek-rs/src/ui/main_tui/downloads.rs +++ b/soulseek-rs/src/ui/main_tui/downloads.rs @@ -3,6 +3,16 @@ use crate::models::{DownloadEntry, FileDisplayData}; use soulseek_rs::{DownloadStatus, types::Download}; use std::{sync::mpsc, thread}; +/// Done, given up, or ran out of time: the row is all that is left. +const fn is_finished(download: &Download) -> bool { + matches!( + download.status, + DownloadStatus::Completed + | DownloadStatus::Failed(_) + | DownloadStatus::TimedOut + ) +} + impl MainTui { /// Channel for queued downloads, created on first use. pub(super) fn downloads_sender( @@ -63,26 +73,16 @@ impl MainTui { // session holds, so dropping them only here would last until the next // poll put them straight back. for entry in &self.state.downloads { - if matches!( - entry.download.status, - DownloadStatus::Completed - | DownloadStatus::Failed(_) - | DownloadStatus::TimedOut - ) { + if is_finished(&entry.download) { self.client.remove_download( &entry.download.username, &entry.download.filename, ); } } - self.state.downloads.retain(|entry| { - !matches!( - entry.download.status, - DownloadStatus::Completed - | DownloadStatus::Failed(_) - | DownloadStatus::TimedOut - ) - }); + self.state + .downloads + .retain(|entry| !is_finished(&entry.download)); let len = self.state.downloads.len(); if len == 0 { self.state.downloads_table_state.select(None); @@ -121,7 +121,9 @@ impl MainTui { } } - pub(super) fn remove_selected_queued_download(&mut self) { + /// Drop the selected transfer: a queued one leaves the queue, a finished + /// one leaves the list. + pub(super) fn remove_selected_download(&mut self) { let Some(index) = self.state.downloads_table_state.selected() else { return; }; @@ -130,14 +132,21 @@ impl MainTui { }; let download = &download_entry.download; - if !matches!(download.status, DownloadStatus::Queued) { - return; - } - - if !self - .client - .remove_queued_download(&download.username, &download.filename) - { + let dismissed = match download.status { + DownloadStatus::Queued => self + .client + .remove_queued_download(&download.username, &download.filename), + // A finished transfer may exist only in this window (restored from + // an earlier run), so the session's "nothing to forget" is not a + // reason to keep the row. + DownloadStatus::Completed => { + self.client + .remove_download(&download.username, &download.filename); + true + } + _ => false, + }; + if !dismissed { return; } diff --git a/soulseek-rs/src/ui/main_tui/input.rs b/soulseek-rs/src/ui/main_tui/input.rs index 4be6277..4c320b4 100644 --- a/soulseek-rs/src/ui/main_tui/input.rs +++ b/soulseek-rs/src/ui/main_tui/input.rs @@ -428,7 +428,7 @@ impl MainTui { self.toggle_selected_download_pause(); } KeyCode::Char('d') => { - self.remove_selected_queued_download(); + self.remove_selected_download(); } KeyCode::Char('r') => { self.retry_selected_download(); diff --git a/soulseek-rs/src/ui/main_tui/mod.rs b/soulseek-rs/src/ui/main_tui/mod.rs index aa4fb1a..0b80188 100644 --- a/soulseek-rs/src/ui/main_tui/mod.rs +++ b/soulseek-rs/src/ui/main_tui/mod.rs @@ -265,7 +265,8 @@ pub fn launch_main_tui( mod tests { use super::*; use crate::daemon::proto::ChatMessageDto; - use crate::models::{MessageDirection, SearchStatus}; + use crate::models::{FocusedPane, MessageDirection, SearchStatus}; + use ratatui::crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; /// A stand-in for a daemon: it already holds a conversation, a transfer /// queue, and a search cache, none of which this window created. @@ -291,6 +292,21 @@ mod tests { } fn queued(username: &str, filename: &str) -> soulseek_rs::types::Download { + at_status(username, filename, soulseek_rs::DownloadStatus::Queued) + } + + fn completed( + username: &str, + filename: &str, + ) -> soulseek_rs::types::Download { + at_status(username, filename, soulseek_rs::DownloadStatus::Completed) + } + + fn at_status( + username: &str, + filename: &str, + status: soulseek_rs::DownloadStatus, + ) -> soulseek_rs::types::Download { let (sender, _) = std::sync::mpsc::channel(); soulseek_rs::types::Download { username: username.to_string(), @@ -298,13 +314,17 @@ mod tests { token: 1, size: 4096, download_directory: "/tmp".to_string(), - status: soulseek_rs::DownloadStatus::Queued, + status, sender, queue_position: None, metadata: soulseek_rs::types::DownloadMetadata::default(), } } + fn key(ch: char) -> KeyEvent { + KeyEvent::new(KeyCode::Char(ch), KeyModifiers::NONE) + } + impl SessionApi for TalkativeSession { fn message_history(&self) -> Vec { self.history.clone() @@ -619,6 +639,43 @@ mod tests { ); } + #[test] + fn d_clears_a_finished_row_but_not_a_running_one() { + // Finished transfers are what a list fills up with after a restart, so + // `d` has to be able to dismiss one — restored or not. A running + // transfer is not ours to discard, and stays put. + let mut tui = with_session(TalkativeSession::default()); + tui.state.downloads.push(crate::models::DownloadEntry { + download: completed("bob", "done.mp3"), + receiver: None, + }); + tui.state.downloads_table_state.select(Some(0)); + tui.state.focused_pane = FocusedPane::Downloads; + + tui.handle_key_event(key('d')); + + assert!(tui.state.downloads.is_empty(), "the row goes too"); + assert_eq!(tui.state.downloads_table_state.selected(), None); + + tui.state.downloads.push(crate::models::DownloadEntry { + download: at_status( + "bob", + "still.mp3", + soulseek_rs::DownloadStatus::InProgress { + bytes_downloaded: 1, + total_bytes: 2, + speed_bytes_per_sec: 1.0, + }, + ), + receiver: None, + }); + tui.state.downloads_table_state.select(Some(0)); + + tui.handle_key_event(key('d')); + + assert_eq!(tui.state.downloads.len(), 1, "it is not disposable"); + } + #[test] fn a_window_shows_searches_run_in_another() { let mut tui = diff --git a/soulseek-rs/src/ui/main_tui/render.rs b/soulseek-rs/src/ui/main_tui/render.rs index 027c433..9e7c42e 100644 --- a/soulseek-rs/src/ui/main_tui/render.rs +++ b/soulseek-rs/src/ui/main_tui/render.rs @@ -393,7 +393,7 @@ impl MainTui { vec![ ("p", "pause/resume"), ("r", "retry failed"), - ("d", "delete queued"), + ("d", "delete queued/done"), ("c", "clear finished"), ("b", "browse user"), ("1-3", "focus pane"),