memoization works

src/tui/app/app.rs

@@ -139,6 +139,9 @@ pub struct App<MH: IMusicHoard> {
     music_hoard: MH,
     selection: Selection,
     state: AppState<(), (), (), String, String, String>,
+    // FIXME: is it possible to use a wrapper struct? - when state() is called return a wrapper
+    // around App<MH> which will contain App.
+    memo: Vec<Option<usize>>,
 }
 
 impl<MH: IMusicHoard> App<MH> {
@@ -153,6 +156,7 @@ impl<MH: IMusicHoard> App<MH> {
             music_hoard,
             selection,
             state,
+            memo: vec![],
         }
     }
 
@@ -242,6 +246,7 @@ impl<MH: IMusicHoard> IAppInteractBrowse for App<MH> {
     fn begin_search(&mut self) {
         assert!(self.state.is_browse());
         self.state = AppState::Search(String::new());
+        self.memo = vec![self.selection.selected_artist()];
         self.selection
             .reset_artist(self.music_hoard.get_collection());
     }
@@ -291,25 +296,27 @@ impl<MH: IMusicHoard> IAppInteractReloadPrivate for App<MH> {
     }
 }
 
+// FIXME: add `search_next` to find next match
+// FIXME: once `search_next` is added, backspace should step back. If the previous action was
+//        `search_next` then no character should be removed. If the previous action was to append
+//        a character, a character should be removed. When in doubt see how Emacs's isearch works.
+// FIXME: Also add a `cancel_search` which returns to the previous selection.
 impl<MH: IMusicHoard> IAppInteractSearch for App<MH> {
     fn append_character(&mut self, ch: char) {
         let collection = self.music_hoard.get_collection();
         let search = self.state.as_mut().unwrap_search();
         search.push(ch);
-        self.selection.incremental_artist_search(collection, search);
+        let prev = self.selection.incremental_artist_search(collection, search);
+        self.memo.push(prev);
     }
 
-    // Removing a character restarts the search from scratch. For now, the performance impact of
-    // this is not noticeable. If it does become noticeable, some form of memoization should be used
-    // as an optimisation. The most intuitive behaviour when removing a character is to return to
-    // the previous search position. However, it is difficult to construct a search predicate in
-    // selection.rs that will always correctly reverse to the previous position.
     fn remove_character(&mut self) {
         let collection = self.music_hoard.get_collection();
         let search = self.state.as_mut().unwrap_search();
-        search.pop();
-        self.selection.reset_artist(&collection);
-        self.selection.incremental_artist_search(collection, search);
+        if search.pop().is_some() {
+            let prev = self.memo.pop().unwrap();
+            self.selection.select_artist(collection, prev);
+        }
     }
 
     fn finish_search(&mut self) {

src/tui/app/selection.rs

@@ -5,6 +5,7 @@ use musichoard::collection::{
     Collection,
 };
 use ratatui::widgets::ListState;
+use std::cmp;
 
 #[derive(Copy, Clone, Debug, PartialEq, Eq)]
 pub enum Category {
@@ -69,10 +70,19 @@ impl Selection {
         }
     }
 
+    // FIXME: the name is not suitable in the presence of other `select` methods.
     pub fn select(&mut self, artists: &[Artist], selected: ActiveSelection) {
         self.artist.reinitialise(artists, selected.artist);
     }
 
+    pub fn selected_artist(&mut self) -> Option<usize> {
+        self.artist.state.list.selected()
+    }
+
+    pub fn select_artist(&mut self, artists: &[Artist], index: Option<usize>) {
+        self.artist.select(artists, index);
+    }
+
     pub fn reset_artist(&mut self, artists: &[Artist]) {
         if self.artist.state.list.selected() != Some(0) {
             self.select(artists, ActiveSelection { artist: None });
@@ -95,8 +105,12 @@ impl Selection {
         };
     }
 
-    pub fn incremental_artist_search(&mut self, collection: &Collection, artist_name: &str) {
-        self.artist.incremental_search(collection, artist_name);
+    pub fn incremental_artist_search(
+        &mut self,
+        collection: &Collection,
+        artist_name: &str,
+    ) -> Option<usize> {
+        self.artist.incremental_search(collection, artist_name)
     }
 
     pub fn increment_selection(&mut self, collection: &Collection, delta: Delta) {
@@ -182,10 +196,13 @@ impl ArtistSelection {
         }
     }
 
-    fn select_to(&mut self, artists: &[Artist], mut to: usize) {
-        if to >= artists.len() {
-            to = artists.len() - 1;
+    fn select(&mut self, artists: &[Artist], mut to: Option<usize>) {
+        to = to.map(|i| cmp::min(i, artists.len() - 1));
+        self.state.list.select(to);
     }
+
+    fn select_to(&mut self, artists: &[Artist], mut to: usize) {
+        to = cmp::min(to, artists.len() - 1);
         if self.state.list.selected() != Some(to) {
             self.state.list.select(Some(to));
             self.album = AlbumSelection::initialise(&artists[to].albums);
@@ -249,8 +266,9 @@ impl ArtistSelection {
         result
     }
 
-    // FIXME: Use memoization - the only way to have correct behaviour
-    fn incremental_search(&mut self, artists: &[Artist], artist_name: &str) {
+    fn incremental_search(&mut self, artists: &[Artist], artist_name: &str) -> Option<usize> {
+        let previous = self.state.list.selected();
+
         if let Some(index) = self.state.list.selected() {
             let case_sensitive = Self::is_case_sensitive(artist_name);
             let char_sensitive = Self::is_char_sensitive(artist_name);
@@ -266,6 +284,8 @@ impl ArtistSelection {
                 self.select_to(artists, index + slice_index);
             }
         }
+
+        previous
     }
 
     fn increment_by(&mut self, artists: &[Artist], by: usize) {