Keep persistent ActivityStates for everything on the stack.

This fixes the cases where temporarily going to another full-screen
activity failed to restore the in-progress state of the previous one.
In particular, nipping out to the utilities menu from composing a
post, and reading the server rules during account registration.

It's a shame; I'd have preferred to be deleting and recreating state
objects all the time, to ensure they had a well-defined amount of
internal state. But this needed less organisation. Perhaps I can come
back and do it better another time.

diff --git a/TODO.md b/TODO.md
index ed3e261dc6b87b337de03b7d71bf96093b656553..eb1393f5490bd14b11b5a1fd01dd6b7ec1edc4db 100644 (file)
--- a/TODO.md
+++ b/TODO.md
@@ -1,17 +1,5 @@
 # Bugs
 
-Some state fails to be retained if you temporarily leave an activity
-via [ESC] and then return to it (say via [RET]). In the post-compose
-menu, the client panics because `TuiLogicalState::new_activity_state`
-was trying to go into the post-compose menu without the previous
-activity having passed it a `Post`, because the activity in question
-was the utilities menu which hasn't got one. And in login / account
-registration, we go right back to the beginning. _Some_ new technique
-is needed for persisting the state of activities on the current stack;
-perhaps we need to start keeping a `Vec<Box<dyn ActivityState>>`? Or
-perhaps just have each activity save its absolutely necessary stuff?
-Not sure which is better.
-
 Server error reporting: HTTP error codes from Mastodon API requests
 are generally accompanied by a JSON body containing a more useful
 message. For example, if you prematurely try to start the full client

diff --git a/src/activity_stack.rs b/src/activity_stack.rs
index 2eaa5a02b02a27802318a03cda86ab6020e725a6..552ed9ed1cb4ef14296a645daac61f4f26dae877 100644 (file)
--- a/src/activity_stack.rs
+++ b/src/activity_stack.rs
@@ -1,7 +1,7 @@
 use super::client::{Boosts, Replies};
 use super::file::SearchDirection;
 
-#[derive(PartialEq, Eq, Debug, Clone)]
+#[derive(PartialEq, Eq, Debug, Clone, Hash)]
 pub enum NonUtilityActivity {
     MainMenu,
     HomeTimelineFile,
@@ -14,7 +14,7 @@ pub enum NonUtilityActivity {
     LoginMenu,
 }
 
-#[derive(PartialEq, Eq, Debug, Clone)]
+#[derive(PartialEq, Eq, Debug, Clone, Hash)]
 pub enum UtilityActivity {
     UtilsMenu,
     ReadMentions,
@@ -35,7 +35,7 @@ pub enum UtilityActivity {
     InstanceRules,
 }
 
-#[derive(PartialEq, Eq, Debug, Clone)]
+#[derive(PartialEq, Eq, Debug, Clone, Hash)]
 pub enum OverlayActivity {
     GetUserToExamine,
     GetPostIdToRead,
@@ -43,7 +43,7 @@ pub enum OverlayActivity {
     GetSearchExpression(SearchDirection),
 }
 
-#[derive(PartialEq, Eq, Debug, Clone)]
+#[derive(PartialEq, Eq, Debug, Clone, Hash)]
 pub enum Activity {
     NonUtil(NonUtilityActivity),
     Util(UtilityActivity),
@@ -181,6 +181,16 @@ impl ActivityStack {
         self.pop();
         self.goto(act);
     }
+
+    pub fn iter(&self) -> impl Iterator<Item = Activity> + '_ {
+        self.nonutil
+            .iter()
+            .cloned()
+            .map(Activity::NonUtil)
+            .chain(self.util.iter().cloned().map(Activity::Util))
+            .chain(self.initial_util.iter().cloned().map(Activity::Util))
+            .chain(self.overlay.iter().cloned().map(Activity::Overlay))
+    }
 }
 
 #[test]

diff --git a/src/file.rs b/src/file.rs
index 1e0994fd973e80a950c750b6a770180af6fd24cc..f294fd7acf6f3c94dde62b7a26f69f8f677fc246 100644 (file)
--- a/src/file.rs
+++ b/src/file.rs
@@ -373,7 +373,7 @@ enum UIMode {
     Select(HighlightType, SelectionPurpose),
 }
 
-#[derive(Debug, PartialEq, Eq, Clone, Copy)]
+#[derive(Debug, PartialEq, Eq, Clone, Copy, Hash)]
 pub enum SearchDirection {
     Up,
     Down,

diff --git a/src/tui.rs b/src/tui.rs
index 6d621e2827fa0fdd2d037d1dcf23da6a44c1001a..39e75c7c4abaf37d13748f8a910ce70637be9c29 100644 (file)
--- a/src/tui.rs
+++ b/src/tui.rs
@@ -303,7 +303,7 @@ impl Tui {
             finish_account_setup(&mut client, cfgloc)?;
         }
 
-        let mut state = TuiLogicalState::new(&client, cfgloc.clone());
+        let mut state = TuiLogicalState::new(&mut client, cfgloc.clone());
         state.load_ldb()?;
 
         stdout().execute(EnterAlternateScreen)?;
@@ -623,8 +623,7 @@ pub trait ActivityState {
 
 struct TuiLogicalState {
     activity_stack: ActivityStack,
-    activity_state: Box<dyn ActivityState>,
-    overlay_activity_state: Option<Box<dyn ActivityState>>,
+    activity_states: HashMap<Activity, Box<dyn ActivityState>>,
     last_area: Option<Rect>,
     file_positions: HashMap<FeedId, SavedFilePos>,
     unfolded_posts: Rc<RefCell<HashSet<String>>>,
@@ -632,23 +631,24 @@ struct TuiLogicalState {
 }
 
 impl TuiLogicalState {
-    fn new(client: &Client, cfgloc: ConfigLocation) -> Self {
-        let (activity, activity_state) = if client.auth.is_logged_in() {
-            (NonUtilityActivity::MainMenu, main_menu(client))
+    fn new(client: &mut Client, cfgloc: ConfigLocation) -> Self {
+        let activity = if client.auth.is_logged_in() {
+            NonUtilityActivity::MainMenu
         } else {
-            (NonUtilityActivity::LoginMenu, login_menu(cfgloc.clone()))
+            NonUtilityActivity::LoginMenu
         };
-        let activity_stack = ActivityStack::new(activity);
 
-        TuiLogicalState {
-            activity_stack,
-            activity_state,
-            overlay_activity_state: None,
+        let mut st = TuiLogicalState {
+            activity_stack: ActivityStack::new(activity),
+            activity_states: HashMap::new(),
             last_area: None,
             file_positions: HashMap::new(),
             cfgloc,
             unfolded_posts: Rc::new(RefCell::new(HashSet::new())),
-        }
+        };
+        st.changed_activity(client, None, false);
+
+        st
     }
 
     fn draw_frame(
@@ -660,13 +660,13 @@ impl TuiLogicalState {
 
         if self.last_area != Some(area) {
             self.last_area = Some(area);
-            self.activity_state.resize(w, h);
-            if let Some(ref mut state) = self.overlay_activity_state {
+            self.activity_state_mut().resize(w, h);
+            if let Some(ref mut state) = self.overlay_activity_state_mut() {
                 state.resize(w, h);
             }
         }
 
-        let (lines, cursorpos) = self.activity_state.draw(w, h);
+        let (lines, cursorpos) = self.activity_state().draw(w, h);
         let mut cursorpos = cursorpos;
         buf.reset();
         let mut last_x = 0;
@@ -686,7 +686,7 @@ impl TuiLogicalState {
             CursorPosition::End => Some((last_x, last_y)),
         };
 
-        if let Some(state) = &self.overlay_activity_state {
+        if let Some(state) = &self.overlay_activity_state() {
             let (lines, overlay_cursorpos) = state.draw(w, h);
             cursorpos = overlay_cursorpos;
             let ytop = h - min(h, lines.len());
@@ -734,10 +734,10 @@ impl TuiLogicalState {
             OurKey::Ctrl('L') => return PhysicalAction::Refresh,
 
             _ => {
-                if let Some(ref mut state) = self.overlay_activity_state {
+                if let Some(state) = self.overlay_activity_state_mut() {
                     state.handle_keypress(key, client)
                 } else {
-                    self.activity_state.handle_keypress(key, client)
+                    self.activity_state_mut().handle_keypress(key, client)
                 }
             }
         };
@@ -773,7 +773,7 @@ impl TuiLogicalState {
                 }
                 LogicalAction::GotSearchExpression(dir, regex) => {
                     self.pop_overlay_activity();
-                    self.activity_state.got_search_expression(dir, regex)
+                    self.activity_state_mut().got_search_expression(dir, regex)
                 }
                 LogicalAction::Error(_) => break PhysicalAction::Beep, // FIXME: Error Log
                 LogicalAction::PostComposed(post) => {
@@ -813,7 +813,7 @@ impl TuiLogicalState {
         feeds_updated: HashSet<FeedId>,
         client: &mut Client,
     ) -> PhysicalAction {
-        self.activity_state
+        self.activity_state_mut()
             .handle_feed_updates(&feeds_updated, client);
 
         if feeds_updated.contains(&FeedId::Mentions) {
@@ -862,18 +862,36 @@ impl TuiLogicalState {
     }
 
     fn checkpoint_ldb(&mut self) {
-        if let Some((feed_id, saved_pos)) =
-            self.activity_state.save_file_position()
-        {
-            let changed =
-                self.file_positions.get(&feed_id) != Some(&saved_pos);
-            self.file_positions.insert(feed_id, saved_pos);
-            if changed {
-                // FIXME: maybe suddenly change our mind and go to the
-                // Error Log
-                self.save_ldb().unwrap();
+        let mut changed = false;
+        for state in self.activity_states.values_mut() {
+            if let Some((feed_id, saved_pos)) = state.save_file_position() {
+                if self.file_positions.get(&feed_id) != Some(&saved_pos) {
+                    self.file_positions.insert(feed_id, saved_pos);
+                    changed = true;
+                }
             }
         }
+
+        if changed {
+            // FIXME: maybe suddenly change our mind and go to the
+            // Error Log
+            self.save_ldb().unwrap();
+        }
+    }
+
+    fn ensure_activity_state(
+        &mut self,
+        act: Activity,
+        client: &mut Client,
+        post: Option<Post>,
+        is_interrupt: bool,
+    ) {
+        if !self.activity_states.contains_key(&act) {
+            self.activity_states.insert(
+                act.clone(),
+                self.new_activity_state(act, client, post, is_interrupt),
+            );
+        }
     }
 
     fn changed_activity(
@@ -883,30 +901,36 @@ impl TuiLogicalState {
         is_interrupt: bool,
     ) {
         self.checkpoint_ldb();
-        self.activity_state = self.new_activity_state(
+
+        self.ensure_activity_state(
             self.activity_stack.top(),
             client,
             post,
             is_interrupt,
         );
-        self.overlay_activity_state = match self.activity_stack.overlay() {
-            Some(activity) => {
-                Some(self.new_activity_state(activity, client, None, false))
-            }
-            None => None,
-        };
+        if let Some(act) = self.activity_stack.overlay() {
+            self.ensure_activity_state(act, client, None, false);
+        }
+
+        self.gc_activity_states();
+
         if let Some(area) = self.last_area {
             let (w, h) = (area.width as usize, area.height as usize);
-            self.activity_state.resize(w, h);
-            if let Some(ref mut state) = self.overlay_activity_state {
+            self.activity_state_mut().resize(w, h);
+            if let Some(state) = self.overlay_activity_state_mut() {
                 state.resize(w, h);
             }
         }
     }
 
+    fn gc_activity_states(&mut self) {
+        let states: HashSet<_> = self.activity_stack.iter().collect();
+        self.activity_states.retain(|k, _| states.contains(k));
+    }
+
     fn pop_overlay_activity(&mut self) {
         self.activity_stack.pop_overlay();
-        self.overlay_activity_state = None;
+        self.gc_activity_states();
     }
 
     fn new_activity_state(
@@ -1115,4 +1139,45 @@ impl TuiLogicalState {
             }
         }
     }
+
+    fn activity_state(&self) -> &dyn ActivityState {
+        self.activity_states
+            .get(&self.activity_stack.top())
+            .expect("every current activity ought to have an entry here")
+            .as_ref()
+    }
+    fn activity_state_mut(&mut self) -> &mut dyn ActivityState {
+        self.activity_states
+            .get_mut(&self.activity_stack.top())
+            .expect("every current activity ought to have an entry here")
+            .as_mut()
+    }
+    fn overlay_activity_state(&self) -> Option<&dyn ActivityState> {
+        match self.activity_stack.overlay() {
+            Some(activity) => Some(
+                self.activity_states
+                    .get(&activity)
+                    .expect(
+                        "every current activity ought to have an entry here",
+                    )
+                    .as_ref(),
+            ),
+            None => None,
+        }
+    }
+    fn overlay_activity_state_mut(
+        &mut self,
+    ) -> Option<&mut (dyn ActivityState + '_)> {
+        match self.activity_stack.overlay() {
+            Some(activity) => Some(
+                self.activity_states
+                    .get_mut(&activity)
+                    .expect(
+                        "every current activity ought to have an entry here",
+                    )
+                    .as_mut(),
+            ),
+            None => None,
+        }
+    }
 }