From 9d58bb9b64573465ed61a79807901c908aa7989a Mon Sep 17 00:00:00 2001 From: Michel de Graaf Date: Sat, 25 Jul 2026 02:32:11 +0200 Subject: [PATCH] refactor: clear the quality-scan findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `render_shortcuts` was 144 lines of "which shortcuts apply" wrapped around 40 lines of drawing; the decision moves into a `shortcuts` method beside the `rooms_shortcuts` helper that already worked this way. Three identically shaped comments over the popup guards collapse into one that carries the part worth knowing — an open popup owns the keyboard, so the order matters. The state tests trade bare `unwrap()` for `expect()` so a failure names the step. --- soulseek-rs/src/persist/state.rs | 29 ++++++++++++++++++--------- soulseek-rs/src/ui/main_tui/input.rs | 7 +------ soulseek-rs/src/ui/main_tui/render.rs | 12 +++++++---- 3 files changed, 28 insertions(+), 20 deletions(-) diff --git a/soulseek-rs/src/persist/state.rs b/soulseek-rs/src/persist/state.rs index 478443f..ab4d3b1 100644 --- a/soulseek-rs/src/persist/state.rs +++ b/soulseek-rs/src/persist/state.rs @@ -185,7 +185,7 @@ mod tests { use super::*; fn store() -> (tempfile::TempDir, StateStore) { - let dir = tempfile::tempdir().unwrap(); + let dir = tempfile::tempdir().expect("a temporary state directory"); let store = StateStore::new(dir.path().join("state")); (dir, store) } @@ -254,25 +254,32 @@ mod tests { fn corrupt_file_loads_as_empty_and_is_kept_as_bak() { let (tmp, store) = store(); let path = tmp.path().join("state").join("downloads.json"); - std::fs::create_dir_all(path.parent().unwrap()).unwrap(); - std::fs::write(&path, "{ not json").unwrap(); + std::fs::create_dir_all(path.parent().expect("a parent")) + .expect("the state directory"); + std::fs::write(&path, "{ not json").expect("the corrupt file"); assert_eq!(store.load_downloads(), vec![]); // The bad file is set aside, not left to be overwritten. assert!(!path.exists()); let bak = path.with_extension("json.bak"); - assert_eq!(std::fs::read_to_string(bak).unwrap(), "{ not json"); + assert_eq!( + std::fs::read_to_string(bak).expect("the .bak"), + "{ not json" + ); } #[test] fn a_second_corrupt_file_replaces_the_previous_bak() { let (tmp, store) = store(); let path = tmp.path().join("state").join("downloads.json"); - std::fs::create_dir_all(path.parent().unwrap()).unwrap(); - std::fs::write(path.with_extension("json.bak"), "old bak").unwrap(); - std::fs::write(&path, "{ newer corruption").unwrap(); + std::fs::create_dir_all(path.parent().expect("a parent")) + .expect("the state directory"); + std::fs::write(path.with_extension("json.bak"), "old bak") + .expect("the previous .bak"); + std::fs::write(&path, "{ newer corruption").expect("the corrupt file"); assert_eq!(store.load_downloads(), vec![]); assert_eq!( - std::fs::read_to_string(path.with_extension("json.bak")).unwrap(), + std::fs::read_to_string(path.with_extension("json.bak")) + .expect("the .bak"), "{ newer corruption" ); } @@ -281,8 +288,10 @@ mod tests { fn file_from_a_newer_build_loads_as_empty_and_is_kept_as_bak() { let (tmp, store) = store(); let path = tmp.path().join("state").join("rooms.json"); - std::fs::create_dir_all(path.parent().unwrap()).unwrap(); - std::fs::write(&path, r#"{"version": 99, "data": ["x"]}"#).unwrap(); + std::fs::create_dir_all(path.parent().expect("a parent")) + .expect("the state directory"); + std::fs::write(&path, r#"{"version": 99, "data": ["x"]}"#) + .expect("the newer-version file"); assert_eq!(store.load_rooms(), Vec::::new()); assert!(!path.exists()); assert!(path.with_extension("json.bak").exists()); diff --git a/soulseek-rs/src/ui/main_tui/input.rs b/soulseek-rs/src/ui/main_tui/input.rs index 3ac6b13..109a71d 100644 --- a/soulseek-rs/src/ui/main_tui/input.rs +++ b/soulseek-rs/src/ui/main_tui/input.rs @@ -16,17 +16,13 @@ impl MainTui { return self.handle_chat_input(key); } - // Browse popup takes over navigation while open. + // An open popup owns the keyboard, so these come before pane routing. if self.state.show_browse { return self.handle_browse_input(key); } - - // Rooms popup takes over navigation while open. if self.state.show_rooms { return self.handle_rooms_input(key); } - - // Settings popup takes over navigation while open. if self.state.settings.is_some() { return self.handle_settings_input(key); } @@ -149,7 +145,6 @@ impl MainTui { KeyCode::BackTab | KeyCode::Up | KeyCode::Char('k') => { self.state.cycle_chat_peer(false); } - // Start a conversation with someone not in the list yet. KeyCode::Char('m') => { self.state.show_messages = false; self.state.command_bar_active = true; diff --git a/soulseek-rs/src/ui/main_tui/render.rs b/soulseek-rs/src/ui/main_tui/render.rs index 9752c7f..a2ff1fb 100644 --- a/soulseek-rs/src/ui/main_tui/render.rs +++ b/soulseek-rs/src/ui/main_tui/render.rs @@ -298,8 +298,10 @@ impl MainTui { } } - fn render_shortcuts(&self, frame: &mut Frame, area: Rect) { - let shortcuts = if self.state.settings.is_some() { + /// Which shortcuts the bar offers, which is purely a question of what is + /// open and where the focus sits. Sibling of [`Self::rooms_shortcuts`]. + fn shortcuts(&self) -> Vec<(&'static str, &'static str)> { + if self.state.settings.is_some() { vec![ ("↑↓", "move"), ("Enter/e", "edit download dir"), @@ -399,9 +401,11 @@ impl MainTui { ] } } - }; + } + } - let shortcuts_line = format_shortcuts_styled(&shortcuts); + fn render_shortcuts(&self, frame: &mut Frame, area: Rect) { + let shortcuts_line = format_shortcuts_styled(&self.shortcuts()); // Surface our own sharing status in the block title. let shared = self.client.shared_directories(); let sharing = match shared.as_slice() {