fix(tasks): creation position in filter_or_create

src/helpers.rs

@@ -10,17 +10,6 @@ pub fn some_non_empty(str: &str) -> Option<String> {
     if str.is_empty() { None } else { Some(str.to_string()) }
 }
 
-// TODO as macro so that log comes from appropriate module
-pub fn or_print<T, U: Display>(result: Result<T, U>) -> Option<T> {
-    match result {
-        Ok(value) => Some(value),
-        Err(error) => {
-            warn!("{}", error);
-            None
-        }
-    }
-}
-
 pub fn prompt(prompt: &str) -> Option<String> {
     print!("{} ", prompt);
     stdout().flush().unwrap();

src/main.rs

@@ -36,6 +36,28 @@ mod kinds;
 const UNDO_DELAY: u64 = 60;
 const INACTVITY_DELAY: u64 = 200;
 
+/// Turn a Result into an Option, showing a warning on error with optional prefix
+macro_rules! or_warn {
+    ($result:expr) => {
+        match $result {
+            Ok(value) => Some(value),
+            Err(error) => {
+                warn!("{}", error);
+                None
+            }
+        }
+    };
+    ($result:expr, $msg:expr $(, $($arg:tt)*)?) => {
+        match $result {
+            Ok(value) => Some(value),
+            Err(error) => {
+                warn!("{}: {}", format!($msg, $($($arg)*)?), error);
+                None
+            }
+        }
+    }
+}
+
 type Events = Vec<Event>;
 
 #[derive(Debug, Clone)]
@@ -131,8 +153,8 @@ async fn main() {
             .init();
     }
 
-    let config_dir = or_print(BaseDirectories::new())
-        .and_then(|d| or_print(d.create_config_directory("mostr")))
+    let config_dir = or_warn!(BaseDirectories::new(), "Could not determine config directory")
+        .and_then(|d| or_warn!(d.create_config_directory("mostr"), "Could not create config directory"))
         .unwrap_or(PathBuf::new());
     let keysfile = config_dir.join("key");
     let relayfile = config_dir.join("relays");
@@ -141,10 +163,13 @@ async fn main() {
         Ok(Ok(key)) => key,
         _ => {
             warn!("Could not read keys from {}", keysfile.to_string_lossy());
-            let keys = prompt("Secret Key?")
-                .and_then(|s| or_print(Keys::from_str(&s)))
-                .unwrap_or_else(|| Keys::generate());
-            or_print(fs::write(&keysfile, keys.secret_key().unwrap().to_string()));
+            let keys = prompt("Secret key?")
+                .and_then(|s| or_warn!(Keys::from_str(&s)))
+                .unwrap_or_else(|| {
+                    info!("Generating and persisting new key");
+                    Keys::generate()
+                });
+            or_warn!(fs::write(&keysfile, keys.secret_key().unwrap().to_string()));
             keys
         }
     };
@@ -155,12 +180,12 @@ async fn main() {
     // TODO use NewRelay message for all relays
     match var("MOSTR_RELAY") {
         Ok(relay) => {
-            or_print(client.add_relay(relay).await);
+            or_warn!(client.add_relay(relay).await);
         }
         _ => match File::open(&relayfile).map(|f| BufReader::new(f).lines().flatten()) {
             Ok(lines) => {
                 for line in lines {
-                    or_print(client.add_relay(line).await);
+                    or_warn!(client.add_relay(line).await);
                 }
             }
             Err(e) => {
@@ -171,9 +196,9 @@ async fn main() {
                     } else {
                         "wss://".to_string() + &line
                     };
-                    or_print(client.add_relay(url.clone()).await).map(|bool| {
+                    or_warn!(client.add_relay(url.clone()).await).map(|bool| {
                         if bool {
-                            or_print(fs::write(&relayfile, url));
+                            or_warn!(fs::write(&relayfile, url));
                         }
                     });
                 };
@@ -205,7 +230,7 @@ async fn main() {
         if let Ok(user) = var("USER") {
             let metadata = Metadata::new()
                 .name(user);
-            or_print(client.set_metadata(&metadata).await);
+            or_warn!(client.set_metadata(&metadata).await);
         }
 
         loop {
@@ -584,12 +609,11 @@ async fn main() {
                             tasks.move_to(None);
                             if let Some((url, tasks)) = relays.iter().find(|(key, _)| key.as_str().starts_with(&input)) {
                                 selected_relay = Some(url.clone());
-                                or_print(tasks.print_tasks());
+                                or_warn!(tasks.print_tasks());
                                 continue;
                             }
-                            match Url::parse(&input) {
-                                Err(e) => warn!("Failed to parse url \"{input}\": {}", e),
-                                Ok(url) => match tx.send(MostrMessage::NewRelay(url.clone())).await {
+                            or_warn!(Url::parse(&input), "Failed to parse url {}", input).map(|url| {
+                                match tx.try_send(MostrMessage::NewRelay(url.clone())) {
                                     Err(e) => error!("Nostr communication thread failure, cannot add relay \"{url}\": {e}"),
                                     Ok(_) => {
                                         info!("Connecting to {url}");
@@ -597,13 +621,13 @@ async fn main() {
                                         relays.insert(url.clone(), tasks_for_url(Some(url)));
                                     }
                                 }
-                            }
+                            });
                             continue;
                         } else {
                             tasks.filter_or_create(tasks.get_position().as_ref(), &input);
                         }
                 }
-                or_print(tasks.print_tasks());
+                or_warn!(tasks.print_tasks());
             }
             Some(Err(e)) => warn!("{}", e),
             None => break,
@@ -616,5 +640,5 @@ async fn main() {
     drop(relays);
 
     info!("Submitting pending updates...");
-    or_print(sender.await);
+    or_warn!(sender.await);
 }

src/tasks.rs

@@ -595,7 +595,7 @@ impl Tasks {
                 // No match, new task
                 self.view.clear();
                 if arg.len() > 2 {
-                    Some(self.make_task(arg))
+                    Some(self.make_task_with(arg, self.position_tags_for(position), true))
                 } else {
                     warn!("Name of a task needs to have at least 3 characters");
                     None
@@ -674,9 +674,14 @@ impl Tasks {
     }
 
     pub(crate) fn position_tags(&self) -> Vec<Tag> {
+        self.position_tags_for(self.get_position_ref())
+    }
+
+    pub(crate) fn position_tags_for(&self, position: Option<&EventId>) -> Vec<Tag> {
+        position.map_or(vec![], |pos| {
             let mut tags = Vec::with_capacity(2);
-        self.parent_tag().map(|t| tags.push(t));
-        self.get_current_task()
+            tags.push(self.make_event_tag_from_id(*pos, MARKER_PARENT));
+            self.get_by_id(pos)
                 .map(|t| {
                     if t.pure_state() == State::Procedure {
                         t.children.iter()
@@ -686,29 +691,30 @@ impl Tasks {
                     }
                 });
             tags
+        })
     }
 
     /// Creates a task following the current state
+    ///
     /// Sanitizes input
     pub(crate) fn make_task(&mut self, input: &str) -> EventId {
-        self.make_task_with(input, empty(), true)
+        self.make_task_with(input, self.position_tags(), true)
     }
 
-    pub(crate) fn make_task_and_enter(&mut self, input: &str, state: State) -> EventId {
-        let id = self.make_task_with(input, empty(), false);
+    pub(crate) fn make_task_and_enter(&mut self, input: &str, state: State) {
+        let id = self.make_task_with(input, self.position_tags(), false);
         self.set_state_for(id, "", state);
         self.move_to(Some(id));
-        id
     }
 
-    /// Creates a task with tags from filter and position
+    /// Creates a task including current tag filters
+    ///
     /// Sanitizes input
     pub(crate) fn make_task_with(&mut self, input: &str, tags: impl IntoIterator<Item=Tag>, set_state: bool) -> EventId {
         let (input, input_tags) = extract_tags(input.trim());
         let id = self.submit(
             build_task(input, input_tags, None)
                 .add_tags(self.tags.iter().cloned())
-                .add_tags(self.position_tags())
                 .add_tags(tags.into_iter())
         );
         if set_state {
@@ -1104,19 +1110,16 @@ mod tasks_test {
         })
     }
 
-
     macro_rules! assert_position {
         ($left:expr, $right:expr $(,)?) => {
             assert_eq!($left.get_position_ref(), Some(&$right))
-            //assert_eq!($left, $right)
         };
     }
 
     #[test]
     fn test_procedures() {
         let mut tasks = stub_tasks();
-        let id = tasks.make_task_and_enter("proc: tags", State::Procedure);
-        assert_position!(tasks, id);
+        tasks.make_task_and_enter("proc: tags", State::Procedure);
         assert_eq!(tasks.get_own_history().unwrap().len(), 1);
         let side = tasks.submit(build_task("side", vec![tasks.make_event_tag(&tasks.get_current_task().unwrap().event, MARKER_DEPENDS)], None));
         assert_eq!(tasks.get_current_task().unwrap().children, HashSet::<EventId>::new());
@@ -1127,6 +1130,31 @@ mod tasks_test {
         assert_eq!(sub.get_dependendees(), Vec::<&EventId>::new());
     }
 
+    #[test]
+    fn test_filter_or_create() {
+        let mut tasks = stub_tasks();
+        let zeros = EventId::all_zeros();
+        let zero = Some(&zeros);
+
+        let id1 = tasks.filter_or_create(zero, "new");
+        assert_eq!(tasks.len(), 1);
+        assert_eq!(tasks.visible_tasks().len(), 0);
+        assert_eq!(tasks.get_by_id(&id1.unwrap()).unwrap().parent_id(), zero);
+
+        tasks.move_to(zero.cloned());
+        assert_eq!(tasks.visible_tasks().len(), 1);
+        let sub = tasks.make_task("test");
+        assert_eq!(tasks.len(), 2);
+        assert_eq!(tasks.visible_tasks().len(), 2);
+        assert_eq!(tasks.get_by_id(&sub).unwrap().parent_id(), zero);
+
+        let id2 = tasks.filter_or_create(None, "new");
+        assert_eq!(tasks.len(), 3);
+        assert_eq!(tasks.visible_tasks().len(), 2);
+        let new2 = tasks.get_by_id(&id2.unwrap()).unwrap();
+        assert_eq!(new2.props, Default::default());
+    }
+
     #[test]
     fn test_tracking() {
         let mut tasks = stub_tasks();
@@ -1138,6 +1166,7 @@ mod tasks_test {
         let almost_now: Timestamp = Timestamp::now() - 12u64;
         tasks.track_at(Timestamp::from(11), Some(zero));
         tasks.track_at(Timestamp::from(13), Some(zero));
+        assert_position!(tasks, zero);
         assert!(tasks.time_tracked(zero) > almost_now.as_u64());
 
         tasks.track_at(Timestamp::from(22), None);
@@ -1260,7 +1289,7 @@ mod tasks_test {
         assert_eq!(tasks.relative_path(dangling), "test");
     }
 
-    #[allow(dead_code)]
+    #[allow(dead_code)] // #[test]
     fn test_itertools() {
         use itertools::Itertools;
         assert_eq!(