diff --git a/soulseek-rs/src/models/file_display_data.rs b/soulseek-rs/src/models/file_display_data.rs index c3d5d06..008f8d5 100644 --- a/soulseek-rs/src/models/file_display_data.rs +++ b/soulseek-rs/src/models/file_display_data.rs @@ -1,4 +1,4 @@ -#[derive(Clone)] +#[derive(Clone, Default)] pub struct FileDisplayData { pub filename: String, pub size: u64, diff --git a/soulseek-rs/src/ui/download_selector.rs b/soulseek-rs/src/ui/download_selector.rs index e78c468..0c9f144 100644 --- a/soulseek-rs/src/ui/download_selector.rs +++ b/soulseek-rs/src/ui/download_selector.rs @@ -254,28 +254,44 @@ impl FileSelector { } } - fn select_previous(&mut self) { - let i = match self.state.selected() { + /// Previous index with wrap-around, or None when the list is empty. + fn wrapping_prev(selected: Option, len: usize) -> Option { + if len == 0 { + return None; + } + Some(match selected { Some(i) if i > 0 => i - 1, - _ => self.items.len() - 1, - }; - self.state.select(Some(i)); + _ => len - 1, + }) + } + + /// Next index with wrap-around, or None when the list is empty. + fn wrapping_next(selected: Option, len: usize) -> Option { + if len == 0 { + return None; + } + Some(match selected { + Some(i) if i < len - 1 => i + 1, + _ => 0, + }) + } + + fn select_previous(&mut self) { + let i = Self::wrapping_prev(self.state.selected(), self.items.len()); + self.state.select(i); } fn select_next(&mut self) { - let i = match self.state.selected() { - Some(i) if i < self.items.len() - 1 => i + 1, - _ => 0, - }; - self.state.select(Some(i)); + let i = Self::wrapping_next(self.state.selected(), self.items.len()); + self.state.select(i); } fn select_first(&mut self) { - self.state.select(Some(0)); + self.state.select((!self.items.is_empty()).then_some(0)); } fn select_last(&mut self) { - self.state.select(Some(self.items.len() - 1)); + self.state.select(self.items.len().checked_sub(1)); } fn apply_filter(&mut self) { @@ -585,3 +601,30 @@ impl FileSelector { frame.render_widget(controls_widget, area); } } + +#[cfg(test)] +mod tests { + use super::FileSelector; + + // Navigating an empty list (e.g. a filter that matched nothing) must not + // underflow `len - 1`; it should yield no selection. + #[test] + fn wrapping_prev_on_empty_list_is_none() { + assert_eq!(FileSelector::wrapping_prev(None, 0), None); + assert_eq!(FileSelector::wrapping_prev(Some(0), 0), None); + } + + #[test] + fn wrapping_next_on_empty_list_is_none() { + assert_eq!(FileSelector::wrapping_next(None, 0), None); + assert_eq!(FileSelector::wrapping_next(Some(0), 0), None); + } + + #[test] + fn wrapping_navigation_wraps_around() { + assert_eq!(FileSelector::wrapping_prev(Some(0), 3), Some(2)); + assert_eq!(FileSelector::wrapping_prev(Some(2), 3), Some(1)); + assert_eq!(FileSelector::wrapping_next(Some(2), 3), Some(0)); + assert_eq!(FileSelector::wrapping_next(Some(1), 3), Some(2)); + } +} diff --git a/soulseek-rs/src/ui/main_tui.rs b/soulseek-rs/src/ui/main_tui.rs index 6127ca0..b3f80ae 100644 --- a/soulseek-rs/src/ui/main_tui.rs +++ b/soulseek-rs/src/ui/main_tui.rs @@ -162,12 +162,18 @@ impl MainTui { self.state.results_pane_area = Some(right_chunks[0]); self.state.downloads_pane_area = Some(downloads_chunks[0]); - // Render Results pane - let results_items = if self.state.results_filter_query.is_empty() { - &self.state.results_items - } else { - &self.state.results_filtered_items - }; + // Render Results pane. When a filter is active the rendered rows are a + // subset, so the pane also needs the mapping back to unfiltered indices + // to render the selection checkboxes correctly. + let (results_items, results_original_indices) = + if self.state.results_filter_query.is_empty() { + (&self.state.results_items, None) + } else { + ( + &self.state.results_filtered_items, + Some(self.state.results_filtered_indices.as_slice()), + ) + }; let active_search_query = self .state @@ -182,6 +188,7 @@ impl MainTui { items: results_items, table_state: &mut self.state.results_table_state, selected_indices: &self.state.results_selected_indices, + original_indices: results_original_indices, filter_query: &self.state.results_filter_query, is_filtering: self.state.results_is_filtering, focused: self.state.focused_pane == FocusedPane::Results, @@ -815,27 +822,17 @@ impl MainTui { self.state.results_is_filtering = false; } + fn recompute_results_filter(&mut self) { + let (items, indices) = filter_results( + &self.state.results_items, + &self.state.results_filter_query, + ); + self.state.results_filtered_items = items; + self.state.results_filtered_indices = indices; + } + fn apply_filter(&mut self) { - let query = self.state.results_filter_query.to_lowercase(); - if query.is_empty() { - self.state.results_filtered_items = - self.state.results_items.clone(); - self.state.results_filtered_indices = - (0..self.state.results_items.len()).collect(); - } else { - self.state.results_filtered_items.clear(); - self.state.results_filtered_indices.clear(); - - for (idx, item) in self.state.results_items.iter().enumerate() { - if item.filename.to_lowercase().contains(&query) - || item.username.to_lowercase().contains(&query) - { - self.state.results_filtered_items.push(item.clone()); - self.state.results_filtered_indices.push(idx); - } - } - } - + self.recompute_results_filter(); if !self.state.results_filtered_items.is_empty() { self.state.results_table_state.select(Some(0)); } @@ -931,15 +928,20 @@ impl MainTui { } } - // Update selected search if this is the active one + // Update selected search if this is the active one. Re-derive + // the filtered view from the current query so an active + // filter is preserved as new results stream in, rather than + // being clobbered by the full unfiltered list. if let Some(selected_idx) = selected_search_index && selected_idx == idx { self.state.results_items = search.results.clone(); - self.state.results_filtered_items = - search.results.clone(); - self.state.results_filtered_indices = - (0..search.results.len()).collect(); + let (items, indices) = filter_results( + &self.state.results_items, + &self.state.results_filter_query, + ); + self.state.results_filtered_items = items; + self.state.results_filtered_indices = indices; } } @@ -1117,3 +1119,74 @@ fn visible_input_at_cursor( (visible_input, cursor_column as u16) } + +/// Filter `items` by a case-insensitive substring match on filename or +/// username. Returns the matching items alongside their indices in the original +/// list, so callers can translate a filtered display index back to the +/// unfiltered results. An empty query returns everything (identity mapping). +fn filter_results( + items: &[FileDisplayData], + query: &str, +) -> (Vec, Vec) { + let query = query.to_lowercase(); + if query.is_empty() { + return (items.to_vec(), (0..items.len()).collect()); + } + + let mut filtered_items = Vec::new(); + let mut filtered_indices = Vec::new(); + for (idx, item) in items.iter().enumerate() { + if item.filename.to_lowercase().contains(&query) + || item.username.to_lowercase().contains(&query) + { + filtered_items.push(item.clone()); + filtered_indices.push(idx); + } + } + (filtered_items, filtered_indices) +} + +#[cfg(test)] +mod tests { + use super::filter_results; + use crate::models::FileDisplayData; + + fn file(filename: &str, username: &str) -> FileDisplayData { + FileDisplayData { + filename: filename.to_string(), + username: username.to_string(), + ..Default::default() + } + } + + #[test] + fn empty_query_returns_identity_mapping() { + let items = vec![file("a.mp3", "bob"), file("b.flac", "amy")]; + let (filtered, indices) = filter_results(&items, ""); + assert_eq!(filtered.len(), 2); + assert_eq!(indices, vec![0, 1]); + } + + #[test] + fn query_matches_filename_and_username_and_maps_indices() { + let items = vec![ + file("track.mp3", "bob"), + file("song.flac", "alice"), + file("alice_demo.mp3", "carol"), + ]; + // "alice" matches item 1 (username) and item 2 (filename). + let (filtered, indices) = filter_results(&items, "alice"); + assert_eq!(filtered.len(), 2); + assert_eq!(indices, vec![1, 2]); + assert_eq!(filtered[0].filename, "song.flac"); + assert_eq!(filtered[1].filename, "alice_demo.mp3"); + } + + #[test] + fn query_is_case_insensitive() { + let items = vec![file("The Weeknd.mp3", "dj")]; + let (filtered, indices) = filter_results(&items, "WEEKND"); + assert_eq!(filtered.len(), 1); + assert_eq!(indices, vec![0]); + } +} diff --git a/soulseek-rs/src/ui/panes/results_pane.rs b/soulseek-rs/src/ui/panes/results_pane.rs index 465ee07..ed80b8e 100644 --- a/soulseek-rs/src/ui/panes/results_pane.rs +++ b/soulseek-rs/src/ui/panes/results_pane.rs @@ -19,12 +19,33 @@ pub struct ResultsPaneParams<'a> { pub items: &'a [FileDisplayData], pub table_state: &'a mut TableState, pub selected_indices: &'a HashSet, + /// Maps a rendered row index to its index in the unfiltered results list. + /// `None` means the rendered rows are the unfiltered list (identity map). + pub original_indices: Option<&'a [usize]>, pub filter_query: &'a str, pub is_filtering: bool, pub focused: bool, pub active_search_query: Option<&'a str>, } +/// Whether the rendered row `display_idx` is selected. `selected_indices` holds +/// indices into the *unfiltered* results, so under an active filter the display +/// index must be translated through `original_indices` first. +fn row_is_selected( + display_idx: usize, + original_indices: Option<&[usize]>, + selected_indices: &HashSet, +) -> bool { + let original = match original_indices { + Some(map) => match map.get(display_idx) { + Some(&o) => o, + None => return false, + }, + None => display_idx, + }; + selected_indices.contains(&original) +} + pub fn render_results_pane( frame: &mut Frame, area: Rect, @@ -34,6 +55,7 @@ pub fn render_results_pane( items, table_state, selected_indices, + original_indices, filter_query, is_filtering, focused, @@ -83,11 +105,12 @@ No results. Select a search from the Searches pane [1]. Or start new search [s .iter() .enumerate() .map(|(idx, file)| { - let checkbox = if selected_indices.contains(&idx) { - "[✓]" - } else { - "[ ]" - }; + let checkbox = + if row_is_selected(idx, original_indices, selected_indices) { + "[✓]" + } else { + "[ ]" + }; let bitrate_str = file .bitrate @@ -147,3 +170,34 @@ No results. Select a search from the Searches pane [1]. Or start new search [s frame.render_stateful_widget(table, area, table_state); } + +#[cfg(test)] +mod tests { + use super::row_is_selected; + use std::collections::HashSet; + + #[test] + fn identity_mapping_used_when_no_filter() { + let selected: HashSet = [1].into_iter().collect(); + assert!(row_is_selected(1, None, &selected)); + assert!(!row_is_selected(0, None, &selected)); + } + + #[test] + fn filtered_rows_resolve_through_original_indices() { + // Filter shows original rows 2, 5, 7 as display rows 0, 1, 2. + // The user selected original row 5. + let original_indices = [2usize, 5, 7]; + let selected: HashSet = [5].into_iter().collect(); + assert!(!row_is_selected(0, Some(&original_indices), &selected)); + assert!(row_is_selected(1, Some(&original_indices), &selected)); + assert!(!row_is_selected(2, Some(&original_indices), &selected)); + } + + #[test] + fn out_of_range_display_index_is_not_selected() { + let original_indices = [2usize, 5]; + let selected: HashSet = [2, 5].into_iter().collect(); + assert!(!row_is_selected(9, Some(&original_indices), &selected)); + } +}