From 28c5fb2268f8bea698419d6762ba892ae2170384 Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Mon, 21 Dec 2020 19:31:30 +0000 Subject: [PATCH 1/3] Add support for task tags Based on properties named `tag.` as already documented --- docs/src/tasks.md | 2 +- taskchampion/src/lib.rs | 4 +- taskchampion/src/task.rs | 194 +++++++++++++++++++++++++++++++++++++-- 3 files changed, 190 insertions(+), 10 deletions(-) diff --git a/docs/src/tasks.md b/docs/src/tasks.md index d53715bbd..8bdf8fa72 100644 --- a/docs/src/tasks.md +++ b/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.` - 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.` - indicates this task has tag `` (value is an empty string) The following are not yet implemented: * `dep.` - indicates this task depends on `` (value is an empty string) -* `tag.` - indicates this task has tag `` (value is an empty string) * `annotation.` - value is an annotation created at the given time diff --git a/taskchampion/src/lib.rs b/taskchampion/src/lib.rs index 64c6cb238..715dcaefa 100644 --- a/taskchampion/src/lib.rs +++ b/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::Status; -pub use task::{Task, TaskMut}; +pub use task::{Priority, Status, Tag, Task, TaskMut}; /// Re-exported type from the `uuid` crate, for ease of compatibility for consumers of this crate. pub use uuid::Uuid; diff --git a/taskchampion/src/task.rs b/taskchampion/src/task.rs index ac2baacf5..2710a241c 100644 --- a/taskchampion/src/task.rs +++ b/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; @@ -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 { + 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 { + Self::from_str(value) + } +} + +impl TryFrom<&String> for Tag { + type Error = failure::Error; + + fn try_from(value: &String) -> Result { + Self::from_str(&value[..]) + } +} + +impl fmt::Display for Tag { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.0.fmt(f) + } +} + +impl AsRef 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 + '_ { + 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> { self.get_timestamp("modified") } // -- utility functions - pub fn get_timestamp(&self, property: &str) -> Option> { + fn get_timestamp(&self, property: &str) -> Option> { 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." 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) -> Fallible<()> { + fn set_string>(&mut self, property: S, value: Option) -> 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 = "".try_into(); + assert_eq!( + tag.unwrap_err().to_string(), + "tags must have at least one character" + ); + + let tag: Result = "999".try_into(); + assert_eq!( + tag.unwrap_err().to_string(), + "first character of a tag must be alphabetic" + ); + + let tag: Result = "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"); From 54e8484bc2a2ae2da1ec445cfad4e03e63a01bef Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Mon, 21 Dec 2020 19:40:07 +0000 Subject: [PATCH 2/3] Support CLI syntax for changing tags --- cli/src/argparse/args.rs | 34 ++------------- cli/src/argparse/modification.rs | 71 ++++++++++++++++++++++++++++++-- cli/src/invocation/modify.rs | 13 ++++++ cli/src/macros.rs | 14 +++++++ 4 files changed, 97 insertions(+), 35 deletions(-) diff --git a/cli/src/argparse/args.rs b/cli/src/argparse/args.rs index c59a4d730..d0fb9cc2d 100644 --- a/cli/src/argparse/args.rs +++ b/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 @@ -131,14 +117,10 @@ mod test { #[test] fn test_arg_matching() { assert_eq!( - arg_matching(tag)(argv!["+foo", "bar"]).unwrap(), - (argv!["bar"], (true, "foo")) + arg_matching(plus_tag)(argv!["+foo", "bar"]).unwrap(), + (argv!["bar"], "foo") ); - assert_eq!( - arg_matching(tag)(argv!["-foo", "bar"]).unwrap(), - (argv!["bar"], (false, "foo")) - ); - assert!(arg_matching(tag)(argv!["foo", "bar"]).is_err()); + assert!(arg_matching(plus_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"); diff --git a/cli/src/argparse/modification.rs b/cli/src/argparse/modification.rs index 6778ba30b..892e2ff5b 100644 --- a/cli/src/argparse/modification.rs +++ b/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, - /// 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, + + /// Add tags + pub add_tags: HashSet, + + /// Remove tags + pub remove_tags: HashSet, } /// 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 { + fn to_modarg(input: &str) -> Result { + Ok(ModArg::PlusTag(input)) + } + map_res(arg_matching(plus_tag), to_modarg)(input) + } + + fn minus_tag(input: ArgList) -> IResult { + fn to_modarg(input: &str) -> Result { + 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() + } + ); + } } diff --git a/cli/src/invocation/modify.rs b/cli/src/invocation/modify.rs index 29802dbab..26e190e2c 100644 --- a/cli/src/invocation/modify.rs +++ b/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( 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(()) diff --git a/cli/src/macros.rs b/cli/src/macros.rs index 284a2f426..83a347dd0 100644 --- a/cli/src/macros.rs +++ b/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 + } + }; +); From 5ea72f878a13bf4bac53429f4bc436702ff61e25 Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Mon, 21 Dec 2020 20:38:12 +0000 Subject: [PATCH 3/3] display tags in 'task info' --- cli/src/invocation/cmd/info.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cli/src/invocation/cmd/info.rs b/cli/src/invocation/cmd/info.rs index 852c8faa9..99f470eb4 100644 --- a/cli/src/invocation/cmd/info.rs +++ b/cli/src/invocation/cmd/info.rs @@ -30,6 +30,11 @@ pub(crate) fn execute( 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)?; }