From 4dc56f66c61fe84faaecc3abfdd2057155244fe6 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Fri, 1 Mar 2024 22:31:12 +0100 Subject: [PATCH] Make Selection fields private (#154) Closes #153 Reviewed-on: https://git.thenineworlds.net/wojtek/musichoard/pulls/154 --- src/tui/app/machine/browse.rs | 33 ++++----- src/tui/app/machine/search.rs | 127 ++++++++++++++++---------------- src/tui/app/selection/album.rs | 8 ++ src/tui/app/selection/artist.rs | 12 +++ src/tui/app/selection/mod.rs | 36 ++++++--- src/tui/app/selection/track.rs | 4 + src/tui/ui.rs | 30 +++----- 7 files changed, 140 insertions(+), 110 deletions(-) diff --git a/src/tui/app/machine/browse.rs b/src/tui/app/machine/browse.rs index 686ef23..00e7c18 100644 --- a/src/tui/app/machine/browse.rs +++ b/src/tui/app/machine/browse.rs @@ -113,43 +113,38 @@ mod tests { fn increment_decrement() { let mut browse = AppMachine::browse(inner(music_hoard(COLLECTION.to_owned()))); let sel = &browse.inner.selection; - assert_eq!(sel.active, Category::Artist); - assert_eq!(sel.artist.state.list.selected(), Some(0)); + assert_eq!(sel.category(), Category::Artist); + assert_eq!(sel.selected(), Some(0)); browse = browse.increment_selection(Delta::Line).unwrap_browse(); let sel = &browse.inner.selection; - assert_eq!(sel.active, Category::Artist); - assert_eq!(sel.artist.state.list.selected(), Some(1)); + assert_eq!(sel.category(), Category::Artist); + assert_eq!(sel.selected(), Some(1)); browse = browse.increment_category().unwrap_browse(); let sel = &browse.inner.selection; - assert_eq!(sel.active, Category::Album); - assert_eq!(sel.artist.state.list.selected(), Some(1)); - assert_eq!(sel.artist.album.state.list.selected(), Some(0)); + assert_eq!(sel.category(), Category::Album); + assert_eq!(sel.selected(), Some(0)); browse = browse.increment_selection(Delta::Line).unwrap_browse(); let sel = &browse.inner.selection; - assert_eq!(sel.active, Category::Album); - assert_eq!(sel.artist.state.list.selected(), Some(1)); - assert_eq!(sel.artist.album.state.list.selected(), Some(1)); + assert_eq!(sel.category(), Category::Album); + assert_eq!(sel.selected(), Some(1)); browse = browse.decrement_selection(Delta::Line).unwrap_browse(); let sel = &browse.inner.selection; - assert_eq!(sel.active, Category::Album); - assert_eq!(sel.artist.state.list.selected(), Some(1)); - assert_eq!(sel.artist.album.state.list.selected(), Some(0)); + assert_eq!(sel.category(), Category::Album); + assert_eq!(sel.selected(), Some(0)); browse = browse.decrement_category().unwrap_browse(); let sel = &browse.inner.selection; - assert_eq!(sel.active, Category::Artist); - assert_eq!(sel.artist.state.list.selected(), Some(1)); - assert_eq!(sel.artist.album.state.list.selected(), Some(0)); + assert_eq!(sel.category(), Category::Artist); + assert_eq!(sel.selected(), Some(1)); browse = browse.decrement_selection(Delta::Line).unwrap_browse(); let sel = &browse.inner.selection; - assert_eq!(sel.active, Category::Artist); - assert_eq!(sel.artist.state.list.selected(), Some(0)); - assert_eq!(sel.artist.album.state.list.selected(), Some(0)); + assert_eq!(sel.category(), Category::Artist); + assert_eq!(sel.selected(), Some(0)); } #[test] diff --git a/src/tui/app/machine/search.rs b/src/tui/app/machine/search.rs index 4c857bb..e8561ab 100644 --- a/src/tui/app/machine/search.rs +++ b/src/tui/app/machine/search.rs @@ -133,7 +133,7 @@ impl IAppInteractSearchPrivate for AppMachine { let search = &self.state.string; let sel = &self.inner.selection; - let result = match sel.active { + let result = match sel.category() { Category::Artist => sel .state_artist(collection) .and_then(|state| Self::search_artists(search, next, state)), @@ -262,153 +262,156 @@ mod tests { fn artist_incremental_search() { // Empty collection. let mut search = AppMachine::search(inner(music_hoard(vec![])), orig(None)); - assert_eq!(search.inner.selection.artist.state.list.selected(), None); + assert_eq!(search.inner.selection.selected(), None); search.state.string = String::from("album_artist 'a'"); search.incremental_search(false); - assert_eq!(search.inner.selection.artist.state.list.selected(), None); + assert_eq!(search.inner.selection.selected(), None); // Basic test, first element. let mut search = AppMachine::search(inner(music_hoard(COLLECTION.to_owned())), orig(Some(1))); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(0)); + assert_eq!(search.inner.selection.selected(), Some(0)); search.state.string = String::from(""); search.incremental_search(false); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(0)); + assert_eq!(search.inner.selection.selected(), Some(0)); search.state.string = String::from("album_artist "); search.incremental_search(false); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(0)); + assert_eq!(search.inner.selection.selected(), Some(0)); search.state.string = String::from("album_artist 'a'"); search.incremental_search(false); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(0)); + assert_eq!(search.inner.selection.selected(), Some(0)); // Basic test, non-first element. let mut search = AppMachine::search(inner(music_hoard(COLLECTION.to_owned())), orig(Some(1))); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(0)); + assert_eq!(search.inner.selection.selected(), Some(0)); search.state.string = String::from("album_artist "); search.incremental_search(false); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(0)); + assert_eq!(search.inner.selection.selected(), Some(0)); search.state.string = String::from("album_artist 'c'"); search.incremental_search(false); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(2)); + assert_eq!(search.inner.selection.selected(), Some(2)); // Non-lowercase. let mut search = AppMachine::search(inner(music_hoard(COLLECTION.to_owned())), orig(Some(1))); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(0)); + assert_eq!(search.inner.selection.selected(), Some(0)); search.state.string = String::from("Album_Artist "); search.incremental_search(false); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(0)); + assert_eq!(search.inner.selection.selected(), Some(0)); search.state.string = String::from("Album_Artist 'C'"); search.incremental_search(false); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(2)); + assert_eq!(search.inner.selection.selected(), Some(2)); // Non-ascii. let mut search = AppMachine::search(inner(music_hoard(COLLECTION.to_owned())), orig(Some(1))); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(0)); + assert_eq!(search.inner.selection.selected(), Some(0)); search.state.string = String::from("album_artist "); search.incremental_search(false); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(0)); + assert_eq!(search.inner.selection.selected(), Some(0)); search.state.string = String::from("album_artist ‘c’"); search.incremental_search(false); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(2)); + assert_eq!(search.inner.selection.selected(), Some(2)); // Non-lowercase, non-ascii. let mut search = AppMachine::search(inner(music_hoard(COLLECTION.to_owned())), orig(Some(1))); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(0)); + assert_eq!(search.inner.selection.selected(), Some(0)); search.state.string = String::from("Album_Artist "); search.incremental_search(false); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(0)); + assert_eq!(search.inner.selection.selected(), Some(0)); search.state.string = String::from("Album_Artist ‘C’"); search.incremental_search(false); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(2)); + assert_eq!(search.inner.selection.selected(), Some(2)); // Stop at name, not sort name. let mut search = AppMachine::search(inner(music_hoard(COLLECTION.to_owned())), orig(Some(1))); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(0)); + assert_eq!(search.inner.selection.selected(), Some(0)); search.state.string = String::from("the "); search.incremental_search(false); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(2)); + assert_eq!(search.inner.selection.selected(), Some(2)); search.state.string = String::from("the album_artist 'c'"); search.incremental_search(false); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(2)); + assert_eq!(search.inner.selection.selected(), Some(2)); // Search next with common prefix. let mut search = AppMachine::search(inner(music_hoard(COLLECTION.to_owned())), orig(Some(1))); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(0)); + assert_eq!(search.inner.selection.selected(), Some(0)); search.state.string = String::from("album_artist"); search.incremental_search(false); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(0)); + assert_eq!(search.inner.selection.selected(), Some(0)); search.incremental_search(true); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(1)); + assert_eq!(search.inner.selection.selected(), Some(1)); search.incremental_search(true); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(2)); + assert_eq!(search.inner.selection.selected(), Some(2)); search.incremental_search(true); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(3)); + assert_eq!(search.inner.selection.selected(), Some(3)); search.incremental_search(true); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(3)); + assert_eq!(search.inner.selection.selected(), Some(3)); } #[test] fn album_incremental_search() { let mut search = AppMachine::search(inner(music_hoard(COLLECTION.to_owned())), orig(Some(1))); - search.inner.selection.active = Category::Album; + search.inner.selection.increment_category(); + assert_eq!(search.inner.selection.category(), Category::Album); let sel = &search.inner.selection; - assert_eq!(sel.artist.album.state.list.selected(), Some(0)); + assert_eq!(sel.selected(), Some(0)); search.state.string = String::from("album_title "); search.incremental_search(false); let sel = &search.inner.selection; - assert_eq!(sel.artist.album.state.list.selected(), Some(0)); + assert_eq!(sel.selected(), Some(0)); let search = search.append_character('a').unwrap_search(); let search = search.append_character('.').unwrap_search(); let search = search.append_character('b').unwrap_search(); let sel = &search.inner.selection; - assert_eq!(sel.artist.album.state.list.selected(), Some(1)); + assert_eq!(sel.selected(), Some(1)); } #[test] fn track_incremental_search() { let mut search = AppMachine::search(inner(music_hoard(COLLECTION.to_owned())), orig(Some(1))); - search.inner.selection.active = Category::Track; + search.inner.selection.increment_category(); + search.inner.selection.increment_category(); + assert_eq!(search.inner.selection.category(), Category::Track); let sel = &search.inner.selection; - assert_eq!(sel.artist.album.track.state.list.selected(), Some(0)); + assert_eq!(sel.selected(), Some(0)); search.state.string = String::from("track "); search.incremental_search(false); let sel = &search.inner.selection; - assert_eq!(sel.artist.album.track.state.list.selected(), Some(0)); + assert_eq!(sel.selected(), Some(0)); let search = search.append_character('a').unwrap_search(); let search = search.append_character('.').unwrap_search(); @@ -417,13 +420,13 @@ mod tests { let search = search.append_character('2').unwrap_search(); let sel = &search.inner.selection; - assert_eq!(sel.artist.album.track.state.list.selected(), Some(1)); + assert_eq!(sel.selected(), Some(1)); } #[test] fn search() { let search = AppMachine::search(inner(music_hoard(COLLECTION.to_owned())), orig(Some(2))); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(0)); + assert_eq!(search.inner.selection.selected(), Some(0)); let search = search.append_character('a').unwrap_search(); let search = search.append_character('l').unwrap_search(); @@ -438,69 +441,69 @@ mod tests { let search = search.append_character('s').unwrap_search(); let search = search.append_character('t').unwrap_search(); let search = search.append_character(' ').unwrap_search(); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(0)); + assert_eq!(search.inner.selection.selected(), Some(0)); let search = search.append_character('\'').unwrap_search(); let search = search.append_character('c').unwrap_search(); let search = search.append_character('\'').unwrap_search(); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(2)); + assert_eq!(search.inner.selection.selected(), Some(2)); let search = search.step_back().unwrap_search(); let search = search.step_back().unwrap_search(); let search = search.step_back().unwrap_search(); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(0)); + assert_eq!(search.inner.selection.selected(), Some(0)); let search = search.append_character('\'').unwrap_search(); let search = search.append_character('b').unwrap_search(); let search = search.append_character('\'').unwrap_search(); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(1)); + assert_eq!(search.inner.selection.selected(), Some(1)); let app = search.finish_search(); let browse = app.unwrap_browse(); - assert_eq!(browse.inner.selection.artist.state.list.selected(), Some(1)); + assert_eq!(browse.inner.selection.selected(), Some(1)); } #[test] fn search_next_step_back() { let search = AppMachine::search(inner(music_hoard(COLLECTION.to_owned())), orig(Some(2))); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(0)); + assert_eq!(search.inner.selection.selected(), Some(0)); let search = search.step_back().unwrap_search(); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(0)); + assert_eq!(search.inner.selection.selected(), Some(0)); let search = search.append_character('a').unwrap_search(); let search = search.search_next().unwrap_search(); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(1)); + assert_eq!(search.inner.selection.selected(), Some(1)); let search = search.search_next().unwrap_search(); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(2)); + assert_eq!(search.inner.selection.selected(), Some(2)); let search = search.search_next().unwrap_search(); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(3)); + assert_eq!(search.inner.selection.selected(), Some(3)); let search = search.search_next().unwrap_search(); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(3)); + assert_eq!(search.inner.selection.selected(), Some(3)); let search = search.step_back().unwrap_search(); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(3)); + assert_eq!(search.inner.selection.selected(), Some(3)); let search = search.step_back().unwrap_search(); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(2)); + assert_eq!(search.inner.selection.selected(), Some(2)); let search = search.step_back().unwrap_search(); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(1)); + assert_eq!(search.inner.selection.selected(), Some(1)); let search = search.step_back().unwrap_search(); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(0)); + assert_eq!(search.inner.selection.selected(), Some(0)); let search = search.step_back().unwrap_search(); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(0)); + assert_eq!(search.inner.selection.selected(), Some(0)); } #[test] fn cancel_search() { let search = AppMachine::search(inner(music_hoard(COLLECTION.to_owned())), orig(Some(2))); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(0)); + assert_eq!(search.inner.selection.selected(), Some(0)); let search = search.append_character('a').unwrap_search(); let search = search.append_character('l').unwrap_search(); @@ -518,31 +521,31 @@ mod tests { let search = search.append_character('\'').unwrap_search(); let search = search.append_character('b').unwrap_search(); let search = search.append_character('\'').unwrap_search(); - assert_eq!(search.inner.selection.artist.state.list.selected(), Some(1)); + assert_eq!(search.inner.selection.selected(), Some(1)); let browse = search.cancel_search().unwrap_browse(); - assert_eq!(browse.inner.selection.artist.state.list.selected(), Some(2)); + assert_eq!(browse.inner.selection.selected(), Some(2)); } #[test] fn empty_search() { let search = AppMachine::search(inner(music_hoard(vec![])), orig(None)); - assert_eq!(search.inner.selection.artist.state.list.selected(), None); + assert_eq!(search.inner.selection.selected(), None); let search = search.append_character('a').unwrap_search(); - assert_eq!(search.inner.selection.artist.state.list.selected(), None); + assert_eq!(search.inner.selection.selected(), None); let search = search.search_next().unwrap_search(); - assert_eq!(search.inner.selection.artist.state.list.selected(), None); + assert_eq!(search.inner.selection.selected(), None); let search = search.step_back().unwrap_search(); - assert_eq!(search.inner.selection.artist.state.list.selected(), None); + assert_eq!(search.inner.selection.selected(), None); let search = search.step_back().unwrap_search(); - assert_eq!(search.inner.selection.artist.state.list.selected(), None); + assert_eq!(search.inner.selection.selected(), None); let browse = search.cancel_search().unwrap_browse(); - assert_eq!(browse.inner.selection.artist.state.list.selected(), None); + assert_eq!(browse.inner.selection.selected(), None); } #[test] diff --git a/src/tui/app/selection/album.rs b/src/tui/app/selection/album.rs index 3780948..1329d95 100644 --- a/src/tui/app/selection/album.rs +++ b/src/tui/app/selection/album.rs @@ -96,6 +96,14 @@ impl AlbumSelection { selected.and_then(|index| self.track.selection_state(&albums[index].tracks)) } + pub fn widget_state(&mut self) -> &mut WidgetState { + &mut self.state + } + + pub fn widget_state_track(&mut self) -> &mut WidgetState { + self.track.widget_state() + } + pub fn reset(&mut self, albums: &[Album]) { if self.state.list.selected() != Some(0) { self.reinitialise(albums, None); diff --git a/src/tui/app/selection/artist.rs b/src/tui/app/selection/artist.rs index 4b11287..8b76d5a 100644 --- a/src/tui/app/selection/artist.rs +++ b/src/tui/app/selection/artist.rs @@ -113,6 +113,18 @@ impl ArtistSelection { selected.and_then(|index| self.album.state_tracks(&artists[index].albums)) } + pub fn widget_state(&mut self) -> &mut WidgetState { + &mut self.state + } + + pub fn widget_state_album(&mut self) -> &mut WidgetState { + self.album.widget_state() + } + + pub fn widget_state_track(&mut self) -> &mut WidgetState { + self.album.widget_state_track() + } + pub fn reset(&mut self, artists: &[Artist]) { if self.state.list.selected() != Some(0) { self.reinitialise(artists, None); diff --git a/src/tui/app/selection/mod.rs b/src/tui/app/selection/mod.rs index 494a2e0..8cddcad 100644 --- a/src/tui/app/selection/mod.rs +++ b/src/tui/app/selection/mod.rs @@ -14,6 +14,16 @@ pub enum Category { Track, } +pub struct Selection { + active: Category, + artist: ArtistSelection, +} + +pub struct SelectionState<'a, T> { + pub list: &'a [T], + pub index: usize, +} + #[derive(Clone, Debug, Default)] pub struct WidgetState { pub list: ListState, @@ -26,16 +36,6 @@ impl PartialEq for WidgetState { } } -pub struct Selection { - pub active: Category, - pub artist: ArtistSelection, -} - -pub struct SelectionState<'a, T> { - pub list: &'a [T], - pub index: usize, -} - pub enum Delta { Line, Page, @@ -104,6 +104,10 @@ impl Selection { self.artist.select_track(artists, index); } + pub fn category(&self) -> Category { + self.active + } + pub fn selected(&self) -> Option { match self.active { Category::Artist => self.selected_artist(), @@ -136,6 +140,18 @@ impl Selection { self.artist.state_track(coll) } + pub fn widget_state_artist(&mut self) -> &mut WidgetState { + self.artist.widget_state() + } + + pub fn widget_state_album(&mut self) -> &mut WidgetState { + self.artist.widget_state_album() + } + + pub fn widget_state_track(&mut self) -> &mut WidgetState { + self.artist.widget_state_track() + } + pub fn reset(&mut self, collection: &Collection) { match self.active { Category::Artist => self.reset_artist(collection), diff --git a/src/tui/app/selection/track.rs b/src/tui/app/selection/track.rs index 5620d1f..feedac5 100644 --- a/src/tui/app/selection/track.rs +++ b/src/tui/app/selection/track.rs @@ -60,6 +60,10 @@ impl TrackSelection { selected.map(|index| SelectionState { list, index }) } + pub fn widget_state(&mut self) -> &mut WidgetState { + &mut self.state + } + pub fn reset(&mut self, tracks: &[Track]) { if self.state.list.selected() != Some(0) { self.reinitialise(tracks, None); diff --git a/src/tui/ui.rs b/src/tui/ui.rs index 289035a..5f6b467 100644 --- a/src/tui/ui.rs +++ b/src/tui/ui.rs @@ -590,46 +590,39 @@ impl Ui { state: &AppPublicState, frame: &mut Frame, ) { - let active = selection.active; + let active = selection.category(); let areas = FrameArea::new(frame.size()); - let artist_selection = &mut selection.artist; let artist_state = ArtistState::new( active == Category::Artist, artists, - &mut artist_selection.state, + selection.widget_state_artist(), ); Self::render_artist_column(artist_state, areas.artist, frame); let no_albums: Vec = vec![]; - let albums = artist_selection - .state - .list - .selected() - .map(|i| &artists[i].albums) + let albums = selection + .state_album(artists) + .map(|st| st.list) .unwrap_or_else(|| &no_albums); - let album_selection = &mut artist_selection.album; let album_state = AlbumState::new( active == Category::Album, albums, - &mut album_selection.state, + selection.widget_state_album(), ); Self::render_album_column(album_state, areas.album, frame); let no_tracks: Vec = vec![]; - let tracks = album_selection - .state - .list - .selected() - .map(|i| &albums[i].tracks) + let tracks = selection + .state_track(artists) + .map(|st| st.list) .unwrap_or_else(|| &no_tracks); - let track_selection = &mut album_selection.track; let track_state = TrackState::new( active == Category::Track, tracks, - &mut track_selection.state, + selection.widget_state_track(), ); Self::render_track_column(track_state, areas.track, frame); @@ -640,8 +633,7 @@ impl Ui { fn render_info_overlay(artists: &Collection, selection: &mut Selection, frame: &mut Frame) { let area = OverlayBuilder::default().build(frame.size()); - let artist_selection = &mut selection.artist; - let artist_overlay = ArtistOverlay::new(artists, &artist_selection.state.list); + let artist_overlay = ArtistOverlay::new(artists, &selection.widget_state_artist().list); Self::render_overlay_widget("Artist", artist_overlay.properties, area, false, frame); }