enhance(tasks): make state update movement more intuitive

Cargo.lock

@@ -1462,7 +1462,7 @@ dependencies = [
 
 [[package]]
 name = "mostr"
-version = "0.9.2"
+version = "0.9.3"
 dependencies = [
  "chrono",
  "colog",

Cargo.toml

@@ -5,7 +5,7 @@ repository  = "https://forge.ftt.gmbh/janek/mostr"
 readme = "README.md"
 license = "GPL 3.0"
 authors = ["melonion"]
-version = "0.9.2"
+version = "0.9.3"
 rust-version = "1.82"
 edition = "2021"
 default-run = "mostr"

src/lib.rs

@@ -0,0 +1,2 @@
+// Mostr Library Crate
+// Placeholder for tests to not include main.rs bulk

src/main.rs

@@ -460,13 +460,11 @@ async fn main() -> Result<()> {
                         }
 
                     Some('>') => {
-                        tasks.update_state(arg_default, State::Done);
-                        if tasks.custom_time.is_none() { tasks.move_up(); }
+                        tasks.update_state_and_up(arg_default, State::Done);
                     }
 
                     Some('<') => {
-                        tasks.update_state(arg_default, State::Closed);
-                        if tasks.custom_time.is_none() { tasks.move_up(); }
+                        tasks.update_state_and_up(arg_default, State::Closed);
                     }
 
                     Some('&') =>
@@ -787,6 +785,7 @@ async fn main() -> Result<()> {
                         }
                 }
                 tasks.custom_time = None;
+                tasks.update_position();
                 println!("{}", tasks);
             }
             Err(ReadlineError::Eof) => break 'repl,

src/tasks.rs

@@ -75,15 +75,14 @@ pub(crate) struct TasksRelay {
     /// The task properties sorted by
     sorting: VecDeque<String>, // TODO prefix +/- for asc/desc, no prefix for default
 
-    /// A filtered view of the current tasks.
-    /// Would like this to be Task references
-    /// but that doesn't work unless I start meddling with Rc everywhere.
+    /// Temporary filtered view of the current tasks.
+    /// Would like this to be Task references but that doesn't work unless I start meddling with Rc everywhere.
     view: Vec<EventId>,
     search_depth: usize,
     view_depth: usize,
     pub(crate) recurse_activities: bool,
-    // Last position used in interface - needs auto-update
-    //position: Option<EventId>,
+    // Last visible position that intuitively interactions should be based on
+    position: Option<EventId>,
 
     /// Currently active tags
     tags: BTreeSet<Hashtag>,
@@ -195,6 +194,7 @@ impl TasksRelay {
             priority: None,
             keys: vec![sender.pubkey()],
             own_keys: vec![sender.pubkey()],
+            position: None,
 
             search_depth: 4,
             view_depth: 0,
@@ -241,15 +241,27 @@ impl TasksRelay {
     fn own_key(&self) -> PublicKey { self.sender.pubkey() }
 
     pub(crate) fn get_position(&self) -> Option<EventId> {
-        self.get_position_at(now()).1
+        self.position
+    }
+    
+    pub(crate) fn calculate_position(&self, time: Option<Timestamp>) -> Option<EventId> {
+        self.get_position_at(time.unwrap_or(now())).1
     }
 
-    pub(crate) fn get_position_timestamped(&self) -> (Timestamp, Option<EventId>) {
-        self.get_position_at(now())
+    // Update the current position
+    // Returns whether the position changed
+    pub(super) fn update_position(&mut self) -> bool {
+        let new_position = self.calculate_position(None);
+        if new_position != self.position {
+            self.view.clear();
+            self.position = new_position;
+            return true
+        }
+        false
     }
 
     pub(super) fn parse_tracking_stamp_relative(&self, input: &str) -> Option<Timestamp> {
-        let pos = self.get_position_timestamped();
+        let pos = self.get_position_at(now());
         let mut pos_time = pos.1.and_then(|_| Local.timestamp_opt(pos.0.as_u64() as i64, 0).earliest());
         parse_tracking_stamp(input, pos_time.take_if(|t| Local::now() - *t > TimeDelta::hours(6)))
     }
@@ -263,6 +275,7 @@ impl TasksRelay {
 
     // TODO binary search
     /// Gets last position change before the given timestamp
+    /// Returns the position together with when it was set
     fn get_position_at(&self, timestamp: Timestamp) -> (Timestamp, Option<EventId>) {
         self.history_from(timestamp)
             .last()
@@ -885,8 +898,10 @@ impl TasksRelay {
         self.state = state;
     }
 
-    pub(crate) fn move_up(&mut self) {
-        self.move_to(self.get_current_task().and_then(|t| t.parent_id()).cloned());
+    pub(crate) fn move_up(&mut self) -> Option<EventId> {
+        let parent = self.get_current_task().and_then(|t| t.parent_id()).cloned();
+        self.move_to(parent);
+        parent
     }
 
     pub(crate) fn flush(&self) {
@@ -983,6 +998,7 @@ impl TasksRelay {
     fn history_from(&self, stamp: Timestamp) -> impl Iterator<Item=&Event> {
         self.history.get(&self.sender.pubkey())
             .map(|hist| {
+                // TODO deduplicate - get earlier time for duplicated tracking?
                 hist.values().rev().take_while_inclusive(move |e| e.created_at > stamp)
             }).into_iter().flatten()
     }
@@ -993,7 +1009,6 @@ impl TasksRelay {
             return;
         }
 
-        self.view.clear();
         let pos = self.get_position();
         if target == pos {
             debug!("Flushing Tasks because of move in place");
@@ -1005,6 +1020,7 @@ impl TasksRelay {
             .and_then(|id| self.get_by_id(&id))
             .is_some_and(|t| t.parent_id() == pos.as_ref())
         {
+            // FIXME this triggers when moving up and into created task, making creation like '..task' not undoable
             debug!("Flushing Tasks because of move beyond child");
             self.sender.flush();
         }
@@ -1015,6 +1031,7 @@ impl TasksRelay {
             .skip_while(|e| e.created_at.as_u64() > now.as_u64() + MAX_OFFSET)
             .count() as u64;
         if offset >= MAX_OFFSET {
+            // This is a very odd edge case when a user moves more than MAX_OFFSET times in MAX_OFFSET seconds so we reject
             warn!("Whoa you are moving around quickly! Give me a few seconds to process.")
         }
         self.submit(
@@ -1022,7 +1039,7 @@ impl TasksRelay {
                 .custom_created_at(now + offset),
         );
     }
-
+    
     // Updates
 
     pub(crate) fn make_event_tag_from_id(&self, id: EventId, marker: &str) -> Tag {
@@ -1082,10 +1099,10 @@ impl TasksRelay {
     /// Returns true if successful, false if there is no current task
     pub(crate) fn make_dependent_sibling(&mut self, input: &str) -> bool {
         if let Some(pos) = self.get_position() {
-            self.move_up();
+            let parent = self.move_up();
             self.make_task_with(
                 input,
-                self.get_position()
+                parent
                     .map(|par| self.make_event_tag_from_id(par, MARKER_PARENT))
                     .into_iter()
                     .chain(once(self.make_event_tag_from_id(pos, MARKER_DEPENDS))),
@@ -1399,9 +1416,14 @@ impl TasksRelay {
         self.submit(prop)
     }
 
-    pub(crate) fn update_state(&mut self, comment: &str, state: State) -> Option<EventId> {
-        let id = self.get_position()?;
-        Some(self.set_state_for(id, comment, state))
+    /// Update state of current task (if one is selected) and move out of it
+    pub(crate) fn update_state_and_up(&mut self, comment: &str, state: State) {
+        if let Some(id) = self.get_position() {
+            self.set_state_for(id, comment, state);
+            if self.calculate_position(self.custom_time) == Some(id) {
+                self.move_up();
+            }
+        }
     }
 
     /// Creates a note or activity, depending on whether the parent is a task.

src/tasks/tests.rs

@@ -68,17 +68,19 @@ fn test_recursive_closing() {
     tasks.custom_time = Some(Timestamp::zero());
     let parent = tasks.make_task_unwrapped("parent #tag1");
     tasks.move_to(Some(parent));
+    tasks.update_position();
     let sub = tasks.make_task_unwrapped("sub #oi # tag2");
     assert_eq!(
         tasks.all_hashtags(),
         ["oi", "tag1", "tag2"].into_iter().map(Hashtag::from).collect()
     );
+    tasks.update_position();
     tasks.make_note("note with #tag3 # yeah");
     let all_tags = ["oi", "tag1", "tag2", "tag3", "yeah"].into_iter().map(Hashtag::from).collect();
     assert_eq!(tasks.all_hashtags(), all_tags);
 
     tasks.custom_time = Some(Timestamp::now());
-    tasks.update_state("Finished #YeaH # oi", State::Done);
+    tasks.update_state_and_up("Finished #YeaH # oi", State::Done);
     assert_eq!(
         tasks.get_by_id(&parent).unwrap().list_hashtags().collect_vec(),
         ["YeaH", "oi", "tag3", "yeah", "tag1"].map(Hashtag::from)
@@ -86,7 +88,7 @@ fn test_recursive_closing() {
     assert_eq!(tasks.all_hashtags(), all_tags);
 
     tasks.custom_time = Some(now());
-    tasks.update_state("Closing Down", State::Closed);
+    tasks.update_state_and_up("Closing Down", State::Closed);
     assert_eq!(tasks.get_by_id(&sub).unwrap().pure_state(), State::Closed);
     assert_eq!(tasks.get_by_id(&parent).unwrap().pure_state(), State::Closed);
     assert_eq!(tasks.nonclosed_tasks().next(), None);
@@ -193,11 +195,13 @@ fn test_sibling_dependency() {
     );
     assert_tasks_view!(tasks, [parent]);
     tasks.track_at(Timestamp::now(), Some(sub));
+    tasks.update_position();
     assert_eq!(tasks.get_own_events_history().count(), 1);
     assert_tasks_view!(tasks, []);
 
     tasks.make_dependent_sibling("sibling");
     assert_eq!(tasks.len(), 3);
+    tasks.update_position();
     assert_eq!(tasks.viewed_tasks().len(), 2);
 }
 
@@ -495,3 +499,5 @@ fn test_itertools() {
     assert_eq!("test  toast".split(' ').collect_vec().len(), 3);
     assert_eq!("test  toast".split_ascii_whitespace().collect_vec().len(), 2);
 }
+
+// TODO subtask of done task visible in quick access but not accessible