Merge pull request #107 from djmitche/issue103

Implement tags in the Replica API and the CLI

cli/src/argparse/args.rs

@@ -58,7 +58,6 @@ pub(super) fn id_list(input: &str) -> IResult<&str, Vec<&str>> {
 }
 
 /// Recognizes a tag prefixed with `+` and returns the tag value
-#[allow(dead_code)] // tags not implemented yet
 pub(super) fn plus_tag(input: &str) -> IResult<&str, &str> {
     fn to_tag(input: (char, &str)) -> Result<&str, ()> {
         Ok(input.1)
@@ -70,7 +69,6 @@ pub(super) fn plus_tag(input: &str) -> IResult<&str, &str> {
 }
 
 /// Recognizes a tag prefixed with `-` and returns the tag value
-#[allow(dead_code)] // tags not implemented yet
 pub(super) fn minus_tag(input: &str) -> IResult<&str, &str> {
     fn to_tag(input: (char, &str)) -> Result<&str, ()> {
         Ok(input.1)
@@ -81,18 +79,6 @@ pub(super) fn minus_tag(input: &str) -> IResult<&str, &str> {
     )(input)
 }
 
-/// Recognizes a tag prefixed with either `-` or `+`, returning true for + and false for -
-#[allow(dead_code)] // tags not implemented yet
-pub(super) fn tag(input: &str) -> IResult<&str, (bool, &str)> {
-    fn to_plus(input: &str) -> Result<(bool, &str), ()> {
-        Ok((true, input))
-    }
-    fn to_minus(input: &str) -> Result<(bool, &str), ()> {
-        Ok((false, input))
-    }
-    alt((map_res(plus_tag, to_plus), map_res(minus_tag, to_minus)))(input)
-}
-
 /// Consume a single argument from an argument list that matches the given string parser (one
 /// of the other functions in this module).  The given parser must consume the entire input.
 pub(super) fn arg_matching<'a, O, F>(f: F) -> impl Fn(ArgList<'a>) -> IResult<ArgList, O>
@@ -131,14 +117,10 @@ mod test {
     #[test]
     fn test_arg_matching() {
         assert_eq!(
-            arg_matching(tag)(argv!["+foo", "bar"]).unwrap(),
+            arg_matching(plus_tag)(argv!["+foo", "bar"]).unwrap(),
-            (argv!["bar"], (true, "foo"))
+            (argv!["bar"], "foo")
         );
-        assert_eq!(
+        assert!(arg_matching(plus_tag)(argv!["foo", "bar"]).is_err());
-            arg_matching(tag)(argv!["-foo", "bar"]).unwrap(),
-            (argv!["bar"], (false, "foo"))
-        );
-        assert!(arg_matching(tag)(argv!["foo", "bar"]).is_err());
     }
 
     #[test]
@@ -161,16 +143,6 @@ mod test {
         assert!(minus_tag("-1abc").is_err());
     }
 
-    #[test]
-    fn test_tag() {
-        assert_eq!(tag("-abc").unwrap().1, (false, "abc"));
-        assert_eq!(tag("+abc123").unwrap().1, (true, "abc123"));
-        assert!(tag("+abc123 --").is_err());
-        assert!(tag("-abc123  ").is_err());
-        assert!(tag("  -abc123").is_err());
-        assert!(tag("-1abc").is_err());
-    }
-
     #[test]
     fn test_literal() {
         assert_eq!(literal("list")("list").unwrap().1, "list");

cli/src/argparse/modification.rs

@@ -1,6 +1,7 @@
-use super::args::{any, arg_matching};
+use super::args::{any, arg_matching, minus_tag, plus_tag};
 use super::ArgList;
-use nom::{combinator::*, multi::fold_many0, IResult};
+use nom::{branch::alt, combinator::*, multi::fold_many0, IResult};
+use std::collections::HashSet;
 use taskchampion::Status;
 
 #[derive(Debug, PartialEq, Clone)]
@@ -34,13 +35,21 @@ pub struct Modification {
     /// Set the status
     pub status: Option<Status>,
 
-    /// Set the "active" status, that is, start (true) or stop (false) the task.
+    /// Set the "active" state, that is, start (true) or stop (false) the task.
     pub active: Option<bool>,
+
+    /// Add tags
+    pub add_tags: HashSet<String>,
+
+    /// Remove tags
+    pub remove_tags: HashSet<String>,
 }
 
 /// A single argument that is part of a modification, used internally to this module
 enum ModArg<'a> {
     Description(&'a str),
+    PlusTag(&'a str),
+    MinusTag(&'a str),
 }
 
 impl Modification {
@@ -55,11 +64,22 @@ impl Modification {
                         acc.description = DescriptionMod::Set(description.to_string());
                     }
                 }
+                ModArg::PlusTag(tag) => {
+                    acc.add_tags.insert(tag.to_owned());
+                }
+                ModArg::MinusTag(tag) => {
+                    acc.remove_tags.insert(tag.to_owned());
+                }
             }
             acc
         }
         fold_many0(
-            Self::description,
+            alt((
+                Self::plus_tag,
+                Self::minus_tag,
+                // this must come last
+                Self::description,
+            )),
             Modification {
                 ..Default::default()
             },
@@ -73,6 +93,20 @@ impl Modification {
         }
         map_res(arg_matching(any), to_modarg)(input)
     }
+
+    fn plus_tag(input: ArgList) -> IResult<ArgList, ModArg> {
+        fn to_modarg(input: &str) -> Result<ModArg, ()> {
+            Ok(ModArg::PlusTag(input))
+        }
+        map_res(arg_matching(plus_tag), to_modarg)(input)
+    }
+
+    fn minus_tag(input: ArgList) -> IResult<ArgList, ModArg> {
+        fn to_modarg(input: &str) -> Result<ModArg, ()> {
+            Ok(ModArg::MinusTag(input))
+        }
+        map_res(arg_matching(minus_tag), to_modarg)(input)
+    }
 }
 
 #[cfg(test)]
@@ -104,6 +138,19 @@ mod test {
         );
     }
 
+    #[test]
+    fn test_add_tags() {
+        let (input, modification) = Modification::parse(argv!["+abc", "+def"]).unwrap();
+        assert_eq!(input.len(), 0);
+        assert_eq!(
+            modification,
+            Modification {
+                add_tags: set!["abc".to_owned(), "def".to_owned()],
+                ..Default::default()
+            }
+        );
+    }
+
     #[test]
     fn test_multi_arg_description() {
         let (input, modification) = Modification::parse(argv!["new", "desc", "fun"]).unwrap();
@@ -116,4 +163,20 @@ mod test {
             }
         );
     }
+
+    #[test]
+    fn test_multi_arg_description_and_tags() {
+        let (input, modification) =
+            Modification::parse(argv!["new", "+next", "desc", "-daytime", "fun"]).unwrap();
+        assert_eq!(input.len(), 0);
+        assert_eq!(
+            modification,
+            Modification {
+                description: DescriptionMod::Set("new desc fun".to_owned()),
+                add_tags: set!["next".to_owned()],
+                remove_tags: set!["daytime".to_owned()],
+                ..Default::default()
+            }
+        );
+    }
 }

cli/src/invocation/cmd/info.rs

@@ -30,6 +30,11 @@ pub(crate) fn execute<W: WriteColor>(
             t.add_row(row![b->"Description", task.get_description()]);
             t.add_row(row![b->"Status", task.get_status()]);
             t.add_row(row![b->"Active", task.is_active()]);
+            let mut tags: Vec<_> = task.get_tags().map(|t| format!("+{}", t)).collect();
+            if !tags.is_empty() {
+                tags.sort();
+                t.add_row(row![b->"Tags", tags.join(" ")]);
+            }
         }
         t.print(w)?;
     }

cli/src/invocation/modify.rs

@@ -1,5 +1,6 @@
 use crate::argparse::{DescriptionMod, Modification};
 use failure::Fallible;
+use std::convert::TryInto;
 use taskchampion::TaskMut;
 use termcolor::WriteColor;
 
@@ -32,6 +33,18 @@ pub(super) fn apply_modification<W: WriteColor>(
         task.stop()?;
     }
 
+    for tag in modification.add_tags.iter() {
+        // note that the parser should have already ensured that this tag was valid
+        let tag = tag.try_into()?;
+        task.add_tag(&tag)?;
+    }
+
+    for tag in modification.remove_tags.iter() {
+        // note that the parser should have already ensured that this tag was valid
+        let tag = tag.try_into()?;
+        task.remove_tag(&tag)?;
+    }
+
     write!(w, "modified task {}\n", task.get_uuid())?;
 
     Ok(())

cli/src/macros.rs

@@ -10,3 +10,17 @@ macro_rules! argv {
         &[$($x),*][..]
     );
 }
+
+/// Create a hashset, similar to vec!
+#[cfg(test)]
+macro_rules! set(
+    { $($key:expr),+ } => {
+        {
+            let mut s = ::std::collections::HashSet::new();
+            $(
+                s.insert($key);
+            )+
+            s
+        }
+     };
+);

docs/src/tasks.md

@@ -31,9 +31,9 @@ The following keys, and key formats, are defined:
 * `description` - the one-line summary of the task
 * `modified` - the time of the last modification of this task
 * `start.<timestamp>` - either an empty string (representing work on the task to the task that has not been stopped) or a timestamp (representing the time that work stopped)
+* `tag.<tag>` - indicates this task has tag `<tag>` (value is an empty string)
 
 The following are not yet implemented:
 
 * `dep.<uuid>` - indicates this task depends on `<uuid>` (value is an empty string)
-* `tag.<tag>` - indicates this task has tag `<tag>` (value is an empty string)
 * `annotation.<timestamp>` - value is an annotation created at the given time

taskchampion/src/lib.rs

@@ -34,9 +34,7 @@ mod utils;
 
 pub use config::{ReplicaConfig, ServerConfig};
 pub use replica::Replica;
-pub use task::Priority;
+pub use task::{Priority, Status, Tag, Task, TaskMut};
-pub use task::Status;
-pub use task::{Task, TaskMut};
 
 /// Re-exported type from the `uuid` crate, for ease of compatibility for consumers of this crate.
 pub use uuid::Uuid;

taskchampion/src/task.rs

@@ -1,8 +1,10 @@
 use crate::replica::Replica;
 use crate::taskstorage::TaskMap;
 use chrono::prelude::*;
-use failure::Fallible;
+use failure::{format_err, Fallible};
 use log::trace;
+use std::convert::{TryFrom, TryInto};
+use std::fmt;
 use uuid::Uuid;
 
 pub type Timestamp = DateTime<Utc>;
@@ -81,6 +83,56 @@ impl Status {
     }
 }
 
+/// A Tag is a newtype around a String that limits its values to valid tags.
+#[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug, Default)]
+pub struct Tag(String);
+
+impl Tag {
+    fn from_str(value: &str) -> Result<Tag, failure::Error> {
+        if let Some(c) = value.chars().next() {
+            if !c.is_ascii_alphabetic() {
+                return Err(format_err!("first character of a tag must be alphabetic"));
+            }
+        } else {
+            return Err(format_err!("tags must have at least one character"));
+        }
+        if !value.chars().skip(1).all(|c| c.is_ascii_alphanumeric()) {
+            return Err(format_err!(
+                "characters of a tag after the first must be alphanumeric"
+            ));
+        }
+        Ok(Self(String::from(value)))
+    }
+}
+
+impl TryFrom<&str> for Tag {
+    type Error = failure::Error;
+
+    fn try_from(value: &str) -> Result<Tag, Self::Error> {
+        Self::from_str(value)
+    }
+}
+
+impl TryFrom<&String> for Tag {
+    type Error = failure::Error;
+
+    fn try_from(value: &String) -> Result<Tag, Self::Error> {
+        Self::from_str(&value[..])
+    }
+}
+
+impl fmt::Display for Tag {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        self.0.fmt(f)
+    }
+}
+
+impl AsRef<str> for Tag {
+    fn as_ref(&self) -> &str {
+        self.0.as_ref()
+    }
+}
+
 #[derive(Debug, PartialEq)]
 pub struct Annotation {
     pub entry: Timestamp,
@@ -155,13 +207,31 @@ impl Task {
             .any(|(k, v)| k.starts_with("start.") && v.is_empty())
     }
 
+    /// Check if this task has the given tag
+    pub fn has_tag(&self, tag: &Tag) -> bool {
+        self.taskmap.contains_key(&format!("tag.{}", tag))
+    }
+
+    /// Iterate over the task's tags
+    pub fn get_tags(&self) -> impl Iterator<Item = Tag> + '_ {
+        self.taskmap.iter().filter_map(|(k, _)| {
+            if k.starts_with("tag.") {
+                if let Ok(tag) = (&k[4..]).try_into() {
+                    return Some(tag);
+                }
+                // note that invalid "tag.*" are ignored
+            }
+            None
+        })
+    }
+
     pub fn get_modified(&self) -> Option<DateTime<Utc>> {
         self.get_timestamp("modified")
     }
 
     // -- utility functions
 
-    pub fn get_timestamp(&self, property: &str) -> Option<DateTime<Utc>> {
+    fn get_timestamp(&self, property: &str) -> Option<DateTime<Utc>> {
         if let Some(ts) = self.taskmap.get(property) {
             if let Ok(ts) = ts.parse() {
                 return Some(Utc.timestamp(ts, 0));
@@ -204,7 +274,7 @@ impl<'r> TaskMut<'r> {
             return Ok(());
         }
         let k = format!("start.{}", Utc::now().timestamp());
-        self.set_string(k.as_ref(), Some(String::from("")))
+        self.set_string(k, Some(String::from("")))
     }
 
     /// Stop the task by adding the current timestamp to all un-resolved "start.<timestamp>" keys.
@@ -224,6 +294,16 @@ impl<'r> TaskMut<'r> {
         Ok(())
     }
 
+    /// Add a tag to this task.  Does nothing if the tag is already present.
+    pub fn add_tag(&mut self, tag: &Tag) -> Fallible<()> {
+        self.set_string(format!("tag.{}", tag), Some("".to_owned()))
+    }
+
+    /// Remove a tag from this task.  Does nothing if the tag is not present.
+    pub fn remove_tag(&mut self, tag: &Tag) -> Fallible<()> {
+        self.set_string(format!("tag.{}", tag), None)
+    }
+
     // -- utility functions
 
     fn lastmod(&mut self) -> Fallible<()> {
@@ -238,17 +318,18 @@ impl<'r> TaskMut<'r> {
         Ok(())
     }
 
-    fn set_string(&mut self, property: &str, value: Option<String>) -> Fallible<()> {
+    fn set_string<S: Into<String>>(&mut self, property: S, value: Option<String>) -> Fallible<()> {
+        let property = property.into();
         self.lastmod()?;
         self.replica
-            .update_task(self.task.uuid, property, value.as_ref())?;
+            .update_task(self.task.uuid, &property, value.as_ref())?;
 
         if let Some(v) = value {
             trace!("task {}: set property {}={:?}", self.task.uuid, property, v);
             self.task.taskmap.insert(property.to_string(), v);
         } else {
             trace!("task {}: remove property {}", self.task.uuid, property);
-            self.task.taskmap.remove(property);
+            self.task.taskmap.remove(&property);
         }
         Ok(())
     }
@@ -297,6 +378,30 @@ mod test {
         f(task)
     }
 
+    #[test]
+    fn test_tag_from_str() {
+        let tag: Tag = "abc".try_into().unwrap();
+        assert_eq!(tag, Tag("abc".to_owned()));
+
+        let tag: Result<Tag, _> = "".try_into();
+        assert_eq!(
+            tag.unwrap_err().to_string(),
+            "tags must have at least one character"
+        );
+
+        let tag: Result<Tag, _> = "999".try_into();
+        assert_eq!(
+            tag.unwrap_err().to_string(),
+            "first character of a tag must be alphabetic"
+        );
+
+        let tag: Result<Tag, _> = "abc!!".try_into();
+        assert_eq!(
+            tag.unwrap_err().to_string(),
+            "characters of a tag after the first must be alphanumeric"
+        );
+    }
+
     #[test]
     fn test_is_active_never_started() {
         let task = Task::new(Uuid::new_v4(), TaskMap::new());
@@ -327,6 +432,55 @@ mod test {
         assert!(!task.is_active());
     }
 
+    #[test]
+    fn test_has_tag() {
+        let task = Task::new(
+            Uuid::new_v4(),
+            vec![(String::from("tag.abc"), String::from(""))]
+                .drain(..)
+                .collect(),
+        );
+
+        assert!(task.has_tag(&"abc".try_into().unwrap()));
+        assert!(!task.has_tag(&"def".try_into().unwrap()));
+    }
+
+    #[test]
+    fn test_get_tags() {
+        let task = Task::new(
+            Uuid::new_v4(),
+            vec![
+                (String::from("tag.abc"), String::from("")),
+                (String::from("tag.def"), String::from("")),
+            ]
+            .drain(..)
+            .collect(),
+        );
+
+        let mut tags: Vec<_> = task.get_tags().collect();
+        tags.sort();
+        assert_eq!(tags, vec![Tag("abc".to_owned()), Tag("def".to_owned())]);
+    }
+
+    #[test]
+    fn test_get_tags_invalid_tags() {
+        let task = Task::new(
+            Uuid::new_v4(),
+            vec![
+                (String::from("tag.ok"), String::from("")),
+                (String::from("tag."), String::from("")),
+                (String::from("tag.123"), String::from("")),
+                (String::from("tag.a!!"), String::from("")),
+            ]
+            .drain(..)
+            .collect(),
+        );
+
+        // only "ok" is OK
+        let tags: Vec<_> = task.get_tags().collect();
+        assert_eq!(tags, vec![Tag("ok".to_owned())]);
+    }
+
     fn count_taskmap(task: &TaskMut, f: fn(&(&String, &String)) -> bool) -> usize {
         task.taskmap.iter().filter(f).count()
     }
@@ -416,6 +570,34 @@ mod test {
         });
     }
 
+    #[test]
+    fn test_add_tags() {
+        with_mut_task(|mut task| {
+            task.add_tag(&Tag("abc".to_owned())).unwrap();
+            assert!(task.taskmap.contains_key("tag.abc"));
+            task.reload().unwrap();
+            assert!(task.taskmap.contains_key("tag.abc"));
+            // redundant add has no effect..
+            task.add_tag(&Tag("abc".to_owned())).unwrap();
+            assert!(task.taskmap.contains_key("tag.abc"));
+        });
+    }
+
+    #[test]
+    fn test_remove_tags() {
+        with_mut_task(|mut task| {
+            task.add_tag(&Tag("abc".to_owned())).unwrap();
+            task.reload().unwrap();
+            assert!(task.taskmap.contains_key("tag.abc"));
+
+            task.remove_tag(&Tag("abc".to_owned())).unwrap();
+            assert!(!task.taskmap.contains_key("tag.abc"));
+            // redundant remove has no effect..
+            task.remove_tag(&Tag("abc".to_owned())).unwrap();
+            assert!(!task.taskmap.contains_key("tag.abc"));
+        });
+    }
+
     #[test]
     fn test_priority() {
         assert_eq!(Priority::L.to_taskmap(), "L");