feat(tui): dismiss a finished download with d

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().
This commit is contained in:
Michel de Graaf
2026-08-27 13:22:39 +02:00
parent eac340cdb8
commit 0f91ffdcf3
4 changed files with 93 additions and 27 deletions
+31 -22
View File
@@ -3,6 +3,16 @@ use crate::models::{DownloadEntry, FileDisplayData};
use soulseek_rs::{DownloadStatus, types::Download}; use soulseek_rs::{DownloadStatus, types::Download};
use std::{sync::mpsc, thread}; 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 { impl MainTui {
/// Channel for queued downloads, created on first use. /// Channel for queued downloads, created on first use.
pub(super) fn downloads_sender( pub(super) fn downloads_sender(
@@ -63,26 +73,16 @@ impl MainTui {
// session holds, so dropping them only here would last until the next // session holds, so dropping them only here would last until the next
// poll put them straight back. // poll put them straight back.
for entry in &self.state.downloads { for entry in &self.state.downloads {
if matches!( if is_finished(&entry.download) {
entry.download.status,
DownloadStatus::Completed
| DownloadStatus::Failed(_)
| DownloadStatus::TimedOut
) {
self.client.remove_download( self.client.remove_download(
&entry.download.username, &entry.download.username,
&entry.download.filename, &entry.download.filename,
); );
} }
} }
self.state.downloads.retain(|entry| { self.state
!matches!( .downloads
entry.download.status, .retain(|entry| !is_finished(&entry.download));
DownloadStatus::Completed
| DownloadStatus::Failed(_)
| DownloadStatus::TimedOut
)
});
let len = self.state.downloads.len(); let len = self.state.downloads.len();
if len == 0 { if len == 0 {
self.state.downloads_table_state.select(None); 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 { let Some(index) = self.state.downloads_table_state.selected() else {
return; return;
}; };
@@ -130,14 +132,21 @@ impl MainTui {
}; };
let download = &download_entry.download; let download = &download_entry.download;
if !matches!(download.status, DownloadStatus::Queued) { let dismissed = match download.status {
return; DownloadStatus::Queued => self
}
if !self
.client .client
.remove_queued_download(&download.username, &download.filename) .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; return;
} }
+1 -1
View File
@@ -428,7 +428,7 @@ impl MainTui {
self.toggle_selected_download_pause(); self.toggle_selected_download_pause();
} }
KeyCode::Char('d') => { KeyCode::Char('d') => {
self.remove_selected_queued_download(); self.remove_selected_download();
} }
KeyCode::Char('r') => { KeyCode::Char('r') => {
self.retry_selected_download(); self.retry_selected_download();
+59 -2
View File
@@ -265,7 +265,8 @@ pub fn launch_main_tui(
mod tests { mod tests {
use super::*; use super::*;
use crate::daemon::proto::ChatMessageDto; 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 /// A stand-in for a daemon: it already holds a conversation, a transfer
/// queue, and a search cache, none of which this window created. /// 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 { 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(); let (sender, _) = std::sync::mpsc::channel();
soulseek_rs::types::Download { soulseek_rs::types::Download {
username: username.to_string(), username: username.to_string(),
@@ -298,13 +314,17 @@ mod tests {
token: 1, token: 1,
size: 4096, size: 4096,
download_directory: "/tmp".to_string(), download_directory: "/tmp".to_string(),
status: soulseek_rs::DownloadStatus::Queued, status,
sender, sender,
queue_position: None, queue_position: None,
metadata: soulseek_rs::types::DownloadMetadata::default(), metadata: soulseek_rs::types::DownloadMetadata::default(),
} }
} }
fn key(ch: char) -> KeyEvent {
KeyEvent::new(KeyCode::Char(ch), KeyModifiers::NONE)
}
impl SessionApi for TalkativeSession { impl SessionApi for TalkativeSession {
fn message_history(&self) -> Vec<ChatMessageDto> { fn message_history(&self) -> Vec<ChatMessageDto> {
self.history.clone() 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] #[test]
fn a_window_shows_searches_run_in_another() { fn a_window_shows_searches_run_in_another() {
let mut tui = let mut tui =
+1 -1
View File
@@ -393,7 +393,7 @@ impl MainTui {
vec![ vec![
("p", "pause/resume"), ("p", "pause/resume"),
("r", "retry failed"), ("r", "retry failed"),
("d", "delete queued"), ("d", "delete queued/done"),
("c", "clear finished"), ("c", "clear finished"),
("b", "browse user"), ("b", "browse user"),
("1-3", "focus pane"), ("1-3", "focus pane"),