refactor: clear the quality-scan findings

`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.
This commit is contained in:
Michel de Graaf
2026-07-25 02:32:11 +02:00
parent 14b0321dbc
commit 9d58bb9b64
3 changed files with 28 additions and 20 deletions
+19 -10
View File
@@ -185,7 +185,7 @@ mod tests {
use super::*; use super::*;
fn store() -> (tempfile::TempDir, StateStore) { 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")); let store = StateStore::new(dir.path().join("state"));
(dir, store) (dir, store)
} }
@@ -254,25 +254,32 @@ mod tests {
fn corrupt_file_loads_as_empty_and_is_kept_as_bak() { fn corrupt_file_loads_as_empty_and_is_kept_as_bak() {
let (tmp, store) = store(); let (tmp, store) = store();
let path = tmp.path().join("state").join("downloads.json"); let path = tmp.path().join("state").join("downloads.json");
std::fs::create_dir_all(path.parent().unwrap()).unwrap(); std::fs::create_dir_all(path.parent().expect("a parent"))
std::fs::write(&path, "{ not json").unwrap(); .expect("the state directory");
std::fs::write(&path, "{ not json").expect("the corrupt file");
assert_eq!(store.load_downloads(), vec![]); assert_eq!(store.load_downloads(), vec![]);
// The bad file is set aside, not left to be overwritten. // The bad file is set aside, not left to be overwritten.
assert!(!path.exists()); assert!(!path.exists());
let bak = path.with_extension("json.bak"); 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] #[test]
fn a_second_corrupt_file_replaces_the_previous_bak() { fn a_second_corrupt_file_replaces_the_previous_bak() {
let (tmp, store) = store(); let (tmp, store) = store();
let path = tmp.path().join("state").join("downloads.json"); let path = tmp.path().join("state").join("downloads.json");
std::fs::create_dir_all(path.parent().unwrap()).unwrap(); std::fs::create_dir_all(path.parent().expect("a parent"))
std::fs::write(path.with_extension("json.bak"), "old bak").unwrap(); .expect("the state directory");
std::fs::write(&path, "{ newer corruption").unwrap(); 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!(store.load_downloads(), vec![]);
assert_eq!( 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" "{ newer corruption"
); );
} }
@@ -281,8 +288,10 @@ mod tests {
fn file_from_a_newer_build_loads_as_empty_and_is_kept_as_bak() { fn file_from_a_newer_build_loads_as_empty_and_is_kept_as_bak() {
let (tmp, store) = store(); let (tmp, store) = store();
let path = tmp.path().join("state").join("rooms.json"); let path = tmp.path().join("state").join("rooms.json");
std::fs::create_dir_all(path.parent().unwrap()).unwrap(); std::fs::create_dir_all(path.parent().expect("a parent"))
std::fs::write(&path, r#"{"version": 99, "data": ["x"]}"#).unwrap(); .expect("the state directory");
std::fs::write(&path, r#"{"version": 99, "data": ["x"]}"#)
.expect("the newer-version file");
assert_eq!(store.load_rooms(), Vec::<String>::new()); assert_eq!(store.load_rooms(), Vec::<String>::new());
assert!(!path.exists()); assert!(!path.exists());
assert!(path.with_extension("json.bak").exists()); assert!(path.with_extension("json.bak").exists());
+1 -6
View File
@@ -16,17 +16,13 @@ impl MainTui {
return self.handle_chat_input(key); 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 { if self.state.show_browse {
return self.handle_browse_input(key); return self.handle_browse_input(key);
} }
// Rooms popup takes over navigation while open.
if self.state.show_rooms { if self.state.show_rooms {
return self.handle_rooms_input(key); return self.handle_rooms_input(key);
} }
// Settings popup takes over navigation while open.
if self.state.settings.is_some() { if self.state.settings.is_some() {
return self.handle_settings_input(key); return self.handle_settings_input(key);
} }
@@ -149,7 +145,6 @@ impl MainTui {
KeyCode::BackTab | KeyCode::Up | KeyCode::Char('k') => { KeyCode::BackTab | KeyCode::Up | KeyCode::Char('k') => {
self.state.cycle_chat_peer(false); self.state.cycle_chat_peer(false);
} }
// Start a conversation with someone not in the list yet.
KeyCode::Char('m') => { KeyCode::Char('m') => {
self.state.show_messages = false; self.state.show_messages = false;
self.state.command_bar_active = true; self.state.command_bar_active = true;
+8 -4
View File
@@ -298,8 +298,10 @@ impl MainTui {
} }
} }
fn render_shortcuts(&self, frame: &mut Frame, area: Rect) { /// Which shortcuts the bar offers, which is purely a question of what is
let shortcuts = if self.state.settings.is_some() { /// 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![ vec![
("↑↓", "move"), ("↑↓", "move"),
("Enter/e", "edit download dir"), ("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. // Surface our own sharing status in the block title.
let shared = self.client.shared_directories(); let shared = self.client.shared_directories();
let sharing = match shared.as_slice() { let sharing = match shared.as_slice() {