From 3d698f7939f715e53621e147dd95dd8d939fd6a1 Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Sat, 5 Jun 2021 20:07:33 -0400 Subject: [PATCH 1/5] add support for synthetic tags --- Cargo.lock | 25 +++++- docs/src/tags.md | 11 +++ taskchampion/Cargo.toml | 3 + taskchampion/src/task.rs | 183 +++++++++++++++++++++++++++++---------- 4 files changed, 174 insertions(+), 48 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 37d2536bc..6b01a4d2b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -826,8 +826,8 @@ dependencies = [ "serde", "serde_derive", "serde_json", - "strum", - "strum_macros", + "strum 0.18.0", + "strum_macros 0.18.0", ] [[package]] @@ -2814,6 +2814,12 @@ version = "0.18.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "57bd81eb48f4c437cadc685403cad539345bf703d78e63707418431cecd4522b" +[[package]] +name = "strum" +version = "0.21.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "aaf86bbcfd1fa9670b7a129f64fc0c9fcbbfe4f1bc4210e9e98fe71ffc12cde2" + [[package]] name = "strum_macros" version = "0.18.0" @@ -2826,6 +2832,18 @@ dependencies = [ "syn", ] +[[package]] +name = "strum_macros" +version = "0.21.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d06aaeeee809dbc59eb4556183dd927df67db1540de5be8d3ec0b6636358a5ec" +dependencies = [ + "heck", + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "syn" version = "1.0.72" @@ -2853,8 +2871,11 @@ dependencies = [ "lmdb-rkv 0.14.0", "log", "proptest", + "rstest", "serde", "serde_json", + "strum 0.21.0", + "strum_macros 0.21.1", "tempfile", "thiserror", "tindercrypt", diff --git a/docs/src/tags.md b/docs/src/tags.md index 7c159c644..ea070d987 100644 --- a/docs/src/tags.md +++ b/docs/src/tags.md @@ -10,3 +10,14 @@ For example, when it's time to continue the job search, `ta +jobsearch` will sho Specifically, tags must be at least one character long and cannot contain whitespace or any of the characters `+-*/(<>^! %=~`. The first character cannot be a digit, and `:` is not allowed after the first character. +All-capital tags are reserved for synthetic tags (below) and cannot be added or removed from tasks. + +## Synthetic Tags + +Synthetic tags are present on tasks that meet specific criteria, that are commonly used for filtering. +For example, `WAITING` is set for tasks that are currently waiting. +These tags cannot be added or removed from a task, but appear and disappear as the task changes. +The following synthetic tags are defined: + +* `WAITING` - set if the task is waiting (has a `wait` property with a date in the future) +* `ACTIVE` - set if the task is active (has been started and not stopped) diff --git a/taskchampion/Cargo.toml b/taskchampion/Cargo.toml index 2505d1ef9..df9a7b52f 100644 --- a/taskchampion/Cargo.toml +++ b/taskchampion/Cargo.toml @@ -22,7 +22,10 @@ lmdb-rkv = {version = "^0.14.0"} ureq = "^2.1.0" log = "^0.4.14" tindercrypt = { version = "^0.2.2", default-features = false } +strum = "0.21" +strum_macros = "0.21" [dev-dependencies] proptest = "^1.0.0" tempfile = "3" +rstest = "0.10" diff --git a/taskchampion/src/task.rs b/taskchampion/src/task.rs index 2cebf11ae..8c7167cf7 100644 --- a/taskchampion/src/task.rs +++ b/taskchampion/src/task.rs @@ -2,8 +2,10 @@ use crate::replica::Replica; use crate::storage::TaskMap; use chrono::prelude::*; use log::trace; +use std::convert::AsRef; use std::convert::{TryFrom, TryInto}; use std::fmt; +use std::str::FromStr; use uuid::Uuid; pub type Timestamp = DateTime; @@ -82,14 +84,20 @@ impl Status { } } -/// A Tag is a newtype around a String that limits its values to valid tags. +// TODO: separate module, wrap in newtype to avoid pub details +/// A Tag is a descriptor for a task, that is either present or absent, and can be used for +/// filtering. Tags composed of all uppercase letters are reserved for synthetic tags. /// /// Valid tags must not contain whitespace or any of the characters in [`INVALID_TAG_CHARACTERS`]. /// The first characters additionally cannot be a digit, and subsequent characters cannot be `:`. /// This definition is based on [that of /// TaskWarrior](https://github.com/GothenburgBitFactory/taskwarrior/blob/663c6575ceca5bd0135ae884879339dac89d3142/src/Lexer.cpp#L146-L164). -#[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug, Default)] -pub struct Tag(String); +#[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug)] +// TODO: impl Default +pub enum Tag { + User(String), + Synthetic(SyntheticTag), +} pub const INVALID_TAG_CHARACTERS: &str = "+-*/(<>^! %=~"; @@ -99,6 +107,15 @@ impl Tag { anyhow::bail!("invalid tag {:?}", value) } + // first, look for synthetic tags + if value.chars().all(|c| c.is_ascii_uppercase()) { + if let Ok(st) = SyntheticTag::from_str(value) { + return Ok(Self::Synthetic(st)); + } + // all uppercase, but not a valid synthetic tag + return err(value); + } + if let Some(c) = value.chars().next() { if c.is_whitespace() || c.is_ascii_digit() || INVALID_TAG_CHARACTERS.contains(c) { return err(value); @@ -113,7 +130,7 @@ impl Tag { { return err(value); } - Ok(Self(String::from(value))) + Ok(Self::User(String::from(value))) } } @@ -135,16 +152,40 @@ impl TryFrom<&String> for Tag { impl fmt::Display for Tag { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.0.fmt(f) + match self { + Self::User(s) => s.fmt(f), + Self::Synthetic(st) => st.as_ref().fmt(f), + } } } impl AsRef for Tag { fn as_ref(&self) -> &str { - self.0.as_ref() + match self { + Self::User(s) => s.as_ref(), + Self::Synthetic(st) => st.as_ref(), + } } } +#[derive( + Debug, + Clone, + Eq, + PartialEq, + Ord, + PartialOrd, + Hash, + strum_macros::EnumString, + strum_macros::AsRefStr, + strum_macros::EnumIter, +)] +#[strum(serialize_all = "SCREAMING_SNAKE_CASE")] +pub enum SyntheticTag { + Waiting, + Active, +} + #[derive(Debug, PartialEq)] pub struct Annotation { pub entry: Timestamp, @@ -233,22 +274,43 @@ impl Task { .any(|(k, v)| k.starts_with("start.") && v.is_empty()) } + /// Determine whether a given synthetic tag is present on this task. All other + /// synthetic tag calculations are based on this one. + fn has_synthetic_tag(&self, synth: &SyntheticTag) -> bool { + match synth { + SyntheticTag::Waiting => self.is_waiting(), + SyntheticTag::Active => self.is_active(), + } + } + /// Check if this task has the given tag pub fn has_tag(&self, tag: &Tag) -> bool { - self.taskmap.contains_key(&format!("tag.{}", tag)) + match tag { + Tag::User(s) => self.taskmap.contains_key(&format!("tag.{}", s)), + Tag::Synthetic(st) => self.has_synthetic_tag(st), + } } /// Iterate over the task's tags pub fn get_tags(&self) -> impl Iterator + '_ { - self.taskmap.iter().filter_map(|(k, _)| { - if let Some(tag) = k.strip_prefix("tag.") { - if let Ok(tag) = tag.try_into() { - return Some(tag); + use strum::IntoEnumIterator; + + self.taskmap + .iter() + .filter_map(|(k, _)| { + if let Some(tag) = k.strip_prefix("tag.") { + if let Ok(tag) = tag.try_into() { + return Some(tag); + } + // note that invalid "tag.*" are ignored } - // note that invalid "tag.*" are ignored - } - None - }) + None + }) + .chain( + SyntheticTag::iter() + .filter(move |st| self.has_synthetic_tag(st)) + .map(|st| Tag::Synthetic(st)), + ) } pub fn get_modified(&self) -> Option> { @@ -326,11 +388,17 @@ impl<'r> TaskMut<'r> { /// Add a tag to this task. Does nothing if the tag is already present. pub fn add_tag(&mut self, tag: &Tag) -> anyhow::Result<()> { + if let Tag::Synthetic(_) = tag { + anyhow::bail!("Synthetic tags cannot be modified"); + } 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) -> anyhow::Result<()> { + if let Tag::Synthetic(_) = tag { + anyhow::bail!("Synthetic tags cannot be modified"); + } self.set_string(format!("tag.{}", tag), None) } @@ -408,6 +476,7 @@ impl<'r> std::ops::Deref for TaskMut<'r> { #[cfg(test)] mod test { use super::*; + use rstest::rstest; fn with_mut_task(f: F) { let mut replica = Replica::new_inmemory(); @@ -416,28 +485,40 @@ mod test { f(task) } - #[test] - fn test_tag_from_str() { - let tag: Tag = "abc".try_into().unwrap(); - assert_eq!(tag, Tag("abc".to_owned())); + /// Create a user tag, without checking its validity + fn utag(name: &'static str) -> Tag { + Tag::User(name.into()) + } - let tag: Tag = ":abc".try_into().unwrap(); - assert_eq!(tag, Tag(":abc".to_owned())); + /// Create a synthetic tag + fn stag(synth: SyntheticTag) -> Tag { + Tag::Synthetic(synth) + } - let tag: Tag = "a123_456".try_into().unwrap(); - assert_eq!(tag, Tag("a123_456".to_owned())); + #[rstest] + #[case::simple("abc")] + #[case::colon_prefix(":abc")] + #[case::letters_and_numbers("a123_456")] + #[case::synthetic("WAITING")] + fn test_tag_try_into_success(#[case] s: &'static str) { + let tag: Tag = s.try_into().unwrap(); + // check Display (via to_string) and AsRef while we're here + assert_eq!(tag.to_string(), s.to_owned()); + assert_eq!(tag.as_ref(), s); + } - let tag: Result = "".try_into(); - assert_eq!(tag.unwrap_err().to_string(), "invalid tag \"\""); - - let tag: Result = "a:b".try_into(); - assert_eq!(tag.unwrap_err().to_string(), "invalid tag \"a:b\""); - - let tag: Result = "999".try_into(); - assert_eq!(tag.unwrap_err().to_string(), "invalid tag \"999\""); - - let tag: Result = "abc!!".try_into(); - assert_eq!(tag.unwrap_err().to_string(), "invalid tag \"abc!!\""); + #[rstest] + #[case::empty("")] + #[case::colon_infix("a:b")] + #[case::digits("999")] + #[case::bangs("abc!!!")] + #[case::no_such_synthetic("NOSUCH")] + fn test_tag_try_into_err(#[case] s: &'static str) { + let tag: Result = s.try_into(); + assert_eq!( + tag.unwrap_err().to_string(), + format!("invalid tag \"{}\"", s) + ); } #[test] @@ -511,13 +592,18 @@ mod test { fn test_has_tag() { let task = Task::new( Uuid::new_v4(), - vec![(String::from("tag.abc"), String::from(""))] - .drain(..) - .collect(), + vec![ + (String::from("tag.abc"), String::from("")), + (String::from("start.1234"), String::from("")), + ] + .drain(..) + .collect(), ); - assert!(task.has_tag(&"abc".try_into().unwrap())); - assert!(!task.has_tag(&"def".try_into().unwrap())); + assert!(task.has_tag(&utag("abc"))); + assert!(!task.has_tag(&utag("def"))); + assert!(task.has_tag(&stag(SyntheticTag::Active))); + assert!(!task.has_tag(&stag(SyntheticTag::Waiting))); } #[test] @@ -527,6 +613,8 @@ mod test { vec![ (String::from("tag.abc"), String::from("")), (String::from("tag.def"), String::from("")), + // set `wait` so the synthetic tag WAITING is present + (String::from("wait"), String::from("33158909732")), ] .drain(..) .collect(), @@ -534,7 +622,10 @@ mod test { let mut tags: Vec<_> = task.get_tags().collect(); tags.sort(); - assert_eq!(tags, vec![Tag("abc".to_owned()), Tag("def".to_owned())]); + assert_eq!( + tags, + vec![utag("abc"), utag("def"), stag(SyntheticTag::Waiting),] + ); } #[test] @@ -553,7 +644,7 @@ mod test { // only "ok" is OK let tags: Vec<_> = task.get_tags().collect(); - assert_eq!(tags, vec![Tag("ok".to_owned())]); + assert_eq!(tags, vec![utag("ok")]); } fn count_taskmap(task: &TaskMut, f: fn(&(&String, &String)) -> bool) -> usize { @@ -648,12 +739,12 @@ mod test { #[test] fn test_add_tags() { with_mut_task(|mut task| { - task.add_tag(&Tag("abc".to_owned())).unwrap(); + task.add_tag(&utag("abc")).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(); + task.add_tag(&utag("abc")).unwrap(); assert!(task.taskmap.contains_key("tag.abc")); }); } @@ -661,14 +752,14 @@ mod test { #[test] fn test_remove_tags() { with_mut_task(|mut task| { - task.add_tag(&Tag("abc".to_owned())).unwrap(); + task.add_tag(&utag("abc")).unwrap(); task.reload().unwrap(); assert!(task.taskmap.contains_key("tag.abc")); - task.remove_tag(&Tag("abc".to_owned())).unwrap(); + task.remove_tag(&utag("abc")).unwrap(); assert!(!task.taskmap.contains_key("tag.abc")); // redundant remove has no effect.. - task.remove_tag(&Tag("abc".to_owned())).unwrap(); + task.remove_tag(&utag("abc")).unwrap(); assert!(!task.taskmap.contains_key("tag.abc")); }); } From ff23c9148b434619ffd5b0a88e0a3b9544d9aa47 Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Sat, 5 Jun 2021 20:42:39 -0400 Subject: [PATCH 2/5] refactor taskchampion::task into submodules --- taskchampion/src/task/annotation.rs | 10 ++ taskchampion/src/task/mod.rs | 18 +++ taskchampion/src/task/priority.rs | 48 ++++++ taskchampion/src/task/status.rs | 54 +++++++ taskchampion/src/task/tag.rs | 136 ++++++++++++++++ taskchampion/src/{ => task}/task.rs | 236 +--------------------------- 6 files changed, 268 insertions(+), 234 deletions(-) create mode 100644 taskchampion/src/task/annotation.rs create mode 100644 taskchampion/src/task/mod.rs create mode 100644 taskchampion/src/task/priority.rs create mode 100644 taskchampion/src/task/status.rs create mode 100644 taskchampion/src/task/tag.rs rename taskchampion/src/{ => task}/task.rs (72%) diff --git a/taskchampion/src/task/annotation.rs b/taskchampion/src/task/annotation.rs new file mode 100644 index 000000000..dadb72b0f --- /dev/null +++ b/taskchampion/src/task/annotation.rs @@ -0,0 +1,10 @@ +use super::Timestamp; + +/// An annotation for a task +#[derive(Debug, PartialEq)] +pub struct Annotation { + /// Time the annotation was made + pub entry: Timestamp, + /// Content of the annotation + pub description: String, +} diff --git a/taskchampion/src/task/mod.rs b/taskchampion/src/task/mod.rs new file mode 100644 index 000000000..6548bf404 --- /dev/null +++ b/taskchampion/src/task/mod.rs @@ -0,0 +1,18 @@ +#![allow(clippy::module_inception)] +use chrono::prelude::*; + +mod annotation; +mod priority; +mod status; +mod tag; +mod task; + +pub use annotation::Annotation; +pub use priority::Priority; +pub use status::Status; +pub use tag::{Tag, INVALID_TAG_CHARACTERS}; +pub use task::{Task, TaskMut}; + +use tag::SyntheticTag; + +pub type Timestamp = DateTime; diff --git a/taskchampion/src/task/priority.rs b/taskchampion/src/task/priority.rs new file mode 100644 index 000000000..5e3f29d0d --- /dev/null +++ b/taskchampion/src/task/priority.rs @@ -0,0 +1,48 @@ +/// The priority of a task +#[derive(Debug, PartialEq)] +pub enum Priority { + /// Low + L, + /// Medium + M, + /// High + H, +} + +#[allow(dead_code)] +impl Priority { + /// Get a Priority from the 1-character value in a TaskMap, + /// defaulting to M + pub(crate) fn from_taskmap(s: &str) -> Priority { + match s { + "L" => Priority::L, + "M" => Priority::M, + "H" => Priority::H, + _ => Priority::M, + } + } + + /// Get the 1-character value for this priority to use in the TaskMap. + pub(crate) fn to_taskmap(&self) -> &str { + match self { + Priority::L => "L", + Priority::M => "M", + Priority::H => "H", + } + } +} + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn test_priority() { + assert_eq!(Priority::L.to_taskmap(), "L"); + assert_eq!(Priority::M.to_taskmap(), "M"); + assert_eq!(Priority::H.to_taskmap(), "H"); + assert_eq!(Priority::from_taskmap("L"), Priority::L); + assert_eq!(Priority::from_taskmap("M"), Priority::M); + assert_eq!(Priority::from_taskmap("H"), Priority::H); + } +} diff --git a/taskchampion/src/task/status.rs b/taskchampion/src/task/status.rs new file mode 100644 index 000000000..f9f9fe773 --- /dev/null +++ b/taskchampion/src/task/status.rs @@ -0,0 +1,54 @@ +/// The status of a task. The default status in "Pending". +#[derive(Debug, PartialEq, Clone)] +pub enum Status { + Pending, + Completed, + Deleted, +} + +impl Status { + /// Get a Status from the 1-character value in a TaskMap, + /// defaulting to Pending + pub(crate) fn from_taskmap(s: &str) -> Status { + match s { + "P" => Status::Pending, + "C" => Status::Completed, + "D" => Status::Deleted, + _ => Status::Pending, + } + } + + /// Get the 1-character value for this status to use in the TaskMap. + pub(crate) fn to_taskmap(&self) -> &str { + match self { + Status::Pending => "P", + Status::Completed => "C", + Status::Deleted => "D", + } + } + + /// Get the full-name value for this status to use in the TaskMap. + pub fn to_string(&self) -> &str { + // TODO: should be impl Display + match self { + Status::Pending => "Pending", + Status::Completed => "Completed", + Status::Deleted => "Deleted", + } + } +} + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn test_status() { + assert_eq!(Status::Pending.to_taskmap(), "P"); + assert_eq!(Status::Completed.to_taskmap(), "C"); + assert_eq!(Status::Deleted.to_taskmap(), "D"); + assert_eq!(Status::from_taskmap("P"), Status::Pending); + assert_eq!(Status::from_taskmap("C"), Status::Completed); + assert_eq!(Status::from_taskmap("D"), Status::Deleted); + } +} diff --git a/taskchampion/src/task/tag.rs b/taskchampion/src/task/tag.rs new file mode 100644 index 000000000..2a60a66c9 --- /dev/null +++ b/taskchampion/src/task/tag.rs @@ -0,0 +1,136 @@ +use std::convert::TryFrom; +use std::fmt; +use std::str::FromStr; + +/// A Tag is a descriptor for a task, that is either present or absent, and can be used for +/// filtering. Tags composed of all uppercase letters are reserved for synthetic tags. +/// +/// Valid tags must not contain whitespace or any of the characters in [`INVALID_TAG_CHARACTERS`]. +/// The first characters additionally cannot be a digit, and subsequent characters cannot be `:`. +/// This definition is based on [that of +/// TaskWarrior](https://github.com/GothenburgBitFactory/taskwarrior/blob/663c6575ceca5bd0135ae884879339dac89d3142/src/Lexer.cpp#L146-L164). +#[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug)] +pub enum Tag { + User(String), + Synthetic(SyntheticTag), +} + +pub const INVALID_TAG_CHARACTERS: &str = "+-*/(<>^! %=~"; + +impl Tag { + fn from_str(value: &str) -> Result { + fn err(value: &str) -> Result { + anyhow::bail!("invalid tag {:?}", value) + } + + // first, look for synthetic tags + if value.chars().all(|c| c.is_ascii_uppercase()) { + if let Ok(st) = SyntheticTag::from_str(value) { + return Ok(Self::Synthetic(st)); + } + // all uppercase, but not a valid synthetic tag + return err(value); + } + + if let Some(c) = value.chars().next() { + if c.is_whitespace() || c.is_ascii_digit() || INVALID_TAG_CHARACTERS.contains(c) { + return err(value); + } + } else { + return err(value); + } + if !value + .chars() + .skip(1) + .all(|c| !(c.is_whitespace() || c == ':' || INVALID_TAG_CHARACTERS.contains(c))) + { + return err(value); + } + Ok(Self::User(String::from(value))) + } +} + +impl TryFrom<&str> for Tag { + type Error = anyhow::Error; + + fn try_from(value: &str) -> Result { + Self::from_str(value) + } +} + +impl TryFrom<&String> for Tag { + type Error = anyhow::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 { + match self { + Self::User(s) => s.fmt(f), + Self::Synthetic(st) => st.as_ref().fmt(f), + } + } +} + +impl AsRef for Tag { + fn as_ref(&self) -> &str { + match self { + Self::User(s) => s.as_ref(), + Self::Synthetic(st) => st.as_ref(), + } + } +} + +#[derive( + Debug, + Clone, + Eq, + PartialEq, + Ord, + PartialOrd, + Hash, + strum_macros::EnumString, + strum_macros::AsRefStr, + strum_macros::EnumIter, +)] +#[strum(serialize_all = "SCREAMING_SNAKE_CASE")] +pub enum SyntheticTag { + Waiting, + Active, +} + +#[cfg(test)] +mod test { + use super::*; + use rstest::rstest; + use std::convert::TryInto; + + #[rstest] + #[case::simple("abc")] + #[case::colon_prefix(":abc")] + #[case::letters_and_numbers("a123_456")] + #[case::synthetic("WAITING")] + fn test_tag_try_into_success(#[case] s: &'static str) { + let tag: Tag = s.try_into().unwrap(); + // check Display (via to_string) and AsRef while we're here + assert_eq!(tag.to_string(), s.to_owned()); + assert_eq!(tag.as_ref(), s); + } + + #[rstest] + #[case::empty("")] + #[case::colon_infix("a:b")] + #[case::digits("999")] + #[case::bangs("abc!!!")] + #[case::no_such_synthetic("NOSUCH")] + fn test_tag_try_into_err(#[case] s: &'static str) { + let tag: Result = s.try_into(); + assert_eq!( + tag.unwrap_err().to_string(), + format!("invalid tag \"{}\"", s) + ); + } +} diff --git a/taskchampion/src/task.rs b/taskchampion/src/task/task.rs similarity index 72% rename from taskchampion/src/task.rs rename to taskchampion/src/task/task.rs index 8c7167cf7..c3678f730 100644 --- a/taskchampion/src/task.rs +++ b/taskchampion/src/task/task.rs @@ -1,197 +1,12 @@ +use super::{Status, SyntheticTag, Tag}; use crate::replica::Replica; use crate::storage::TaskMap; use chrono::prelude::*; use log::trace; use std::convert::AsRef; -use std::convert::{TryFrom, TryInto}; -use std::fmt; -use std::str::FromStr; +use std::convert::TryInto; use uuid::Uuid; -pub type Timestamp = DateTime; - -/// The priority of a task -#[derive(Debug, PartialEq)] -pub enum Priority { - /// Low - L, - /// Medium - M, - /// High - H, -} - -#[allow(dead_code)] -impl Priority { - /// Get a Priority from the 1-character value in a TaskMap, - /// defaulting to M - pub(crate) fn from_taskmap(s: &str) -> Priority { - match s { - "L" => Priority::L, - "M" => Priority::M, - "H" => Priority::H, - _ => Priority::M, - } - } - - /// Get the 1-character value for this priority to use in the TaskMap. - pub(crate) fn to_taskmap(&self) -> &str { - match self { - Priority::L => "L", - Priority::M => "M", - Priority::H => "H", - } - } -} - -/// The status of a task. The default status in "Pending". -#[derive(Debug, PartialEq, Clone)] -pub enum Status { - Pending, - Completed, - Deleted, -} - -impl Status { - /// Get a Status from the 1-character value in a TaskMap, - /// defaulting to Pending - pub(crate) fn from_taskmap(s: &str) -> Status { - match s { - "P" => Status::Pending, - "C" => Status::Completed, - "D" => Status::Deleted, - _ => Status::Pending, - } - } - - /// Get the 1-character value for this status to use in the TaskMap. - pub(crate) fn to_taskmap(&self) -> &str { - match self { - Status::Pending => "P", - Status::Completed => "C", - Status::Deleted => "D", - } - } - - /// Get the full-name value for this status to use in the TaskMap. - pub fn to_string(&self) -> &str { - // TODO: should be impl Display - match self { - Status::Pending => "Pending", - Status::Completed => "Completed", - Status::Deleted => "Deleted", - } - } -} - -// TODO: separate module, wrap in newtype to avoid pub details -/// A Tag is a descriptor for a task, that is either present or absent, and can be used for -/// filtering. Tags composed of all uppercase letters are reserved for synthetic tags. -/// -/// Valid tags must not contain whitespace or any of the characters in [`INVALID_TAG_CHARACTERS`]. -/// The first characters additionally cannot be a digit, and subsequent characters cannot be `:`. -/// This definition is based on [that of -/// TaskWarrior](https://github.com/GothenburgBitFactory/taskwarrior/blob/663c6575ceca5bd0135ae884879339dac89d3142/src/Lexer.cpp#L146-L164). -#[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug)] -// TODO: impl Default -pub enum Tag { - User(String), - Synthetic(SyntheticTag), -} - -pub const INVALID_TAG_CHARACTERS: &str = "+-*/(<>^! %=~"; - -impl Tag { - fn from_str(value: &str) -> Result { - fn err(value: &str) -> Result { - anyhow::bail!("invalid tag {:?}", value) - } - - // first, look for synthetic tags - if value.chars().all(|c| c.is_ascii_uppercase()) { - if let Ok(st) = SyntheticTag::from_str(value) { - return Ok(Self::Synthetic(st)); - } - // all uppercase, but not a valid synthetic tag - return err(value); - } - - if let Some(c) = value.chars().next() { - if c.is_whitespace() || c.is_ascii_digit() || INVALID_TAG_CHARACTERS.contains(c) { - return err(value); - } - } else { - return err(value); - } - if !value - .chars() - .skip(1) - .all(|c| !(c.is_whitespace() || c == ':' || INVALID_TAG_CHARACTERS.contains(c))) - { - return err(value); - } - Ok(Self::User(String::from(value))) - } -} - -impl TryFrom<&str> for Tag { - type Error = anyhow::Error; - - fn try_from(value: &str) -> Result { - Self::from_str(value) - } -} - -impl TryFrom<&String> for Tag { - type Error = anyhow::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 { - match self { - Self::User(s) => s.fmt(f), - Self::Synthetic(st) => st.as_ref().fmt(f), - } - } -} - -impl AsRef for Tag { - fn as_ref(&self) -> &str { - match self { - Self::User(s) => s.as_ref(), - Self::Synthetic(st) => st.as_ref(), - } - } -} - -#[derive( - Debug, - Clone, - Eq, - PartialEq, - Ord, - PartialOrd, - Hash, - strum_macros::EnumString, - strum_macros::AsRefStr, - strum_macros::EnumIter, -)] -#[strum(serialize_all = "SCREAMING_SNAKE_CASE")] -pub enum SyntheticTag { - Waiting, - Active, -} - -#[derive(Debug, PartialEq)] -pub struct Annotation { - pub entry: Timestamp, - pub description: String, -} - /// A task, as publicly exposed by this crate. /// /// Note that Task objects represent a snapshot of the task at a moment in time, and are not @@ -476,7 +291,6 @@ impl<'r> std::ops::Deref for TaskMut<'r> { #[cfg(test)] mod test { use super::*; - use rstest::rstest; fn with_mut_task(f: F) { let mut replica = Replica::new_inmemory(); @@ -495,32 +309,6 @@ mod test { Tag::Synthetic(synth) } - #[rstest] - #[case::simple("abc")] - #[case::colon_prefix(":abc")] - #[case::letters_and_numbers("a123_456")] - #[case::synthetic("WAITING")] - fn test_tag_try_into_success(#[case] s: &'static str) { - let tag: Tag = s.try_into().unwrap(); - // check Display (via to_string) and AsRef while we're here - assert_eq!(tag.to_string(), s.to_owned()); - assert_eq!(tag.as_ref(), s); - } - - #[rstest] - #[case::empty("")] - #[case::colon_infix("a:b")] - #[case::digits("999")] - #[case::bangs("abc!!!")] - #[case::no_such_synthetic("NOSUCH")] - fn test_tag_try_into_err(#[case] s: &'static str) { - let tag: Result = s.try_into(); - assert_eq!( - tag.unwrap_err().to_string(), - format!("invalid tag \"{}\"", s) - ); - } - #[test] fn test_is_active_never_started() { let task = Task::new(Uuid::new_v4(), TaskMap::new()); @@ -763,24 +551,4 @@ mod test { assert!(!task.taskmap.contains_key("tag.abc")); }); } - - #[test] - fn test_priority() { - assert_eq!(Priority::L.to_taskmap(), "L"); - assert_eq!(Priority::M.to_taskmap(), "M"); - assert_eq!(Priority::H.to_taskmap(), "H"); - assert_eq!(Priority::from_taskmap("L"), Priority::L); - assert_eq!(Priority::from_taskmap("M"), Priority::M); - assert_eq!(Priority::from_taskmap("H"), Priority::H); - } - - #[test] - fn test_status() { - assert_eq!(Status::Pending.to_taskmap(), "P"); - assert_eq!(Status::Completed.to_taskmap(), "C"); - assert_eq!(Status::Deleted.to_taskmap(), "D"); - assert_eq!(Status::from_taskmap("P"), Status::Pending); - assert_eq!(Status::from_taskmap("C"), Status::Completed); - assert_eq!(Status::from_taskmap("D"), Status::Deleted); - } } From 0e60bcedaf38fecabc38e99369a74338a1ef86ca Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Sat, 5 Jun 2021 20:57:28 -0400 Subject: [PATCH 3/5] hide the implementation of Tag --- taskchampion/src/task/mod.rs | 2 -- taskchampion/src/task/tag.rs | 50 +++++++++++++++++++++++++++-------- taskchampion/src/task/task.rs | 19 ++++++------- 3 files changed, 49 insertions(+), 22 deletions(-) diff --git a/taskchampion/src/task/mod.rs b/taskchampion/src/task/mod.rs index 6548bf404..ecb755c29 100644 --- a/taskchampion/src/task/mod.rs +++ b/taskchampion/src/task/mod.rs @@ -13,6 +13,4 @@ pub use status::Status; pub use tag::{Tag, INVALID_TAG_CHARACTERS}; pub use task::{Task, TaskMut}; -use tag::SyntheticTag; - pub type Timestamp = DateTime; diff --git a/taskchampion/src/task/tag.rs b/taskchampion/src/task/tag.rs index 2a60a66c9..d8689e6af 100644 --- a/taskchampion/src/task/tag.rs +++ b/taskchampion/src/task/tag.rs @@ -10,7 +10,11 @@ use std::str::FromStr; /// This definition is based on [that of /// TaskWarrior](https://github.com/GothenburgBitFactory/taskwarrior/blob/663c6575ceca5bd0135ae884879339dac89d3142/src/Lexer.cpp#L146-L164). #[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug)] -pub enum Tag { +pub struct Tag(TagInner); + +/// Inner type to hide the implementation +#[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug)] +pub(super) enum TagInner { User(String), Synthetic(SyntheticTag), } @@ -18,7 +22,7 @@ pub enum Tag { pub const INVALID_TAG_CHARACTERS: &str = "+-*/(<>^! %=~"; impl Tag { - fn from_str(value: &str) -> Result { + pub fn from_str(value: &str) -> Result { fn err(value: &str) -> Result { anyhow::bail!("invalid tag {:?}", value) } @@ -26,7 +30,7 @@ impl Tag { // first, look for synthetic tags if value.chars().all(|c| c.is_ascii_uppercase()) { if let Ok(st) = SyntheticTag::from_str(value) { - return Ok(Self::Synthetic(st)); + return Ok(Self(TagInner::Synthetic(st))); } // all uppercase, but not a valid synthetic tag return err(value); @@ -46,7 +50,29 @@ impl Tag { { return err(value); } - Ok(Self::User(String::from(value))) + Ok(Self(TagInner::User(String::from(value)))) + } + + /// True if this tag is a synthetic tag + pub fn is_synthetic(&self) -> bool { + if let TagInner::Synthetic(_) = self.0 { + true + } else { + false + } + } + + /// True if this tag is a user-provided tag (not synthetic) + pub fn is_user(&self) -> bool { + !self.is_synthetic() + } + + pub(super) fn inner(&self) -> &TagInner { + &self.0 + } + + pub(super) fn from_inner(inner: TagInner) -> Self { + Self(inner) } } @@ -68,22 +94,24 @@ impl TryFrom<&String> for Tag { impl fmt::Display for Tag { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::User(s) => s.fmt(f), - Self::Synthetic(st) => st.as_ref().fmt(f), + match &self.0 { + TagInner::User(s) => s.fmt(f), + TagInner::Synthetic(st) => st.as_ref().fmt(f), } } } impl AsRef for Tag { fn as_ref(&self) -> &str { - match self { - Self::User(s) => s.as_ref(), - Self::Synthetic(st) => st.as_ref(), + match &self.0 { + TagInner::User(s) => s.as_ref(), + TagInner::Synthetic(st) => st.as_ref(), } } } +/// A synthetic tag, represented as an `enum`. This type is used directly by +/// [`taskchampion::task::task`] for efficiency. #[derive( Debug, Clone, @@ -97,7 +125,7 @@ impl AsRef for Tag { strum_macros::EnumIter, )] #[strum(serialize_all = "SCREAMING_SNAKE_CASE")] -pub enum SyntheticTag { +pub(super) enum SyntheticTag { Waiting, Active, } diff --git a/taskchampion/src/task/task.rs b/taskchampion/src/task/task.rs index c3678f730..9695247cf 100644 --- a/taskchampion/src/task/task.rs +++ b/taskchampion/src/task/task.rs @@ -1,4 +1,5 @@ -use super::{Status, SyntheticTag, Tag}; +use super::tag::{SyntheticTag, TagInner}; +use super::{Status, Tag}; use crate::replica::Replica; use crate::storage::TaskMap; use chrono::prelude::*; @@ -100,9 +101,9 @@ impl Task { /// Check if this task has the given tag pub fn has_tag(&self, tag: &Tag) -> bool { - match tag { - Tag::User(s) => self.taskmap.contains_key(&format!("tag.{}", s)), - Tag::Synthetic(st) => self.has_synthetic_tag(st), + match tag.inner() { + TagInner::User(s) => self.taskmap.contains_key(&format!("tag.{}", s)), + TagInner::Synthetic(st) => self.has_synthetic_tag(st), } } @@ -124,7 +125,7 @@ impl Task { .chain( SyntheticTag::iter() .filter(move |st| self.has_synthetic_tag(st)) - .map(|st| Tag::Synthetic(st)), + .map(|st| Tag::from_inner(TagInner::Synthetic(st))), ) } @@ -203,7 +204,7 @@ impl<'r> TaskMut<'r> { /// Add a tag to this task. Does nothing if the tag is already present. pub fn add_tag(&mut self, tag: &Tag) -> anyhow::Result<()> { - if let Tag::Synthetic(_) = tag { + if tag.is_synthetic() { anyhow::bail!("Synthetic tags cannot be modified"); } self.set_string(format!("tag.{}", tag), Some("".to_owned())) @@ -211,7 +212,7 @@ impl<'r> TaskMut<'r> { /// Remove a tag from this task. Does nothing if the tag is not present. pub fn remove_tag(&mut self, tag: &Tag) -> anyhow::Result<()> { - if let Tag::Synthetic(_) = tag { + if tag.is_synthetic() { anyhow::bail!("Synthetic tags cannot be modified"); } self.set_string(format!("tag.{}", tag), None) @@ -301,12 +302,12 @@ mod test { /// Create a user tag, without checking its validity fn utag(name: &'static str) -> Tag { - Tag::User(name.into()) + Tag::from_inner(TagInner::User(name.into())) } /// Create a synthetic tag fn stag(synth: SyntheticTag) -> Tag { - Tag::Synthetic(synth) + Tag::from_inner(TagInner::Synthetic(synth)) } #[test] From cf3a053a0e2150f4a761dc681877d1aba6318fb3 Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Sat, 5 Jun 2021 21:10:11 -0400 Subject: [PATCH 4/5] Add PENDING, COMPLETED, DELETED synthetic tags Note that DELETED is not tested since we don't yet support deleting tasks. --- cli/src/invocation/report.rs | 7 +++++-- docs/src/tags.md | 3 +++ taskchampion/src/task/tag.rs | 5 +++++ taskchampion/src/task/task.rs | 37 ++++++++++++++++++++++++++++++----- 4 files changed, 45 insertions(+), 7 deletions(-) diff --git a/cli/src/invocation/report.rs b/cli/src/invocation/report.rs index cf4008562..d67dae4fb 100644 --- a/cli/src/invocation/report.rs +++ b/cli/src/invocation/report.rs @@ -403,8 +403,11 @@ mod test { }; let task = replica.get_task(uuids[0]).unwrap().unwrap(); - assert_eq!(task_column(&task, &column, &working_set), s!("+bar +foo")); + assert_eq!( + task_column(&task, &column, &working_set), + s!("+PENDING +bar +foo") + ); let task = replica.get_task(uuids[2]).unwrap().unwrap(); - assert_eq!(task_column(&task, &column, &working_set), s!("")); + assert_eq!(task_column(&task, &column, &working_set), s!("+PENDING")); } } diff --git a/docs/src/tags.md b/docs/src/tags.md index ea070d987..cf9d80102 100644 --- a/docs/src/tags.md +++ b/docs/src/tags.md @@ -21,3 +21,6 @@ The following synthetic tags are defined: * `WAITING` - set if the task is waiting (has a `wait` property with a date in the future) * `ACTIVE` - set if the task is active (has been started and not stopped) +* `PENDING` - set if the task is pending (not completed or deleted) +* `COMPLETED` - set if the task has been completed +* `DELETED` - set if the task has been deleted (but not yet flushed from the task list) diff --git a/taskchampion/src/task/tag.rs b/taskchampion/src/task/tag.rs index d8689e6af..7554a57dd 100644 --- a/taskchampion/src/task/tag.rs +++ b/taskchampion/src/task/tag.rs @@ -126,8 +126,13 @@ impl AsRef for Tag { )] #[strum(serialize_all = "SCREAMING_SNAKE_CASE")] pub(super) enum SyntheticTag { + // When adding items here, also implement and test them in `task.rs` and document them in + // `docs/src/tags.md`. Waiting, Active, + Pending, + Completed, + Deleted, } #[cfg(test)] diff --git a/taskchampion/src/task/task.rs b/taskchampion/src/task/task.rs index 9695247cf..40b2d6b0f 100644 --- a/taskchampion/src/task/task.rs +++ b/taskchampion/src/task/task.rs @@ -96,6 +96,9 @@ impl Task { match synth { SyntheticTag::Waiting => self.is_waiting(), SyntheticTag::Active => self.is_active(), + SyntheticTag::Pending => self.get_status() == Status::Pending, + SyntheticTag::Completed => self.get_status() == Status::Completed, + SyntheticTag::Deleted => self.get_status() == Status::Deleted, } } @@ -202,6 +205,11 @@ impl<'r> TaskMut<'r> { Ok(()) } + /// Mark this task as complete + pub fn done(&mut self) -> anyhow::Result<()> { + self.set_status(Status::Completed) + } + /// Add a tag to this task. Does nothing if the tag is already present. pub fn add_tag(&mut self, tag: &Tag) -> anyhow::Result<()> { if tag.is_synthetic() { @@ -392,6 +400,7 @@ mod test { assert!(task.has_tag(&utag("abc"))); assert!(!task.has_tag(&utag("def"))); assert!(task.has_tag(&stag(SyntheticTag::Active))); + assert!(task.has_tag(&stag(SyntheticTag::Pending))); assert!(!task.has_tag(&stag(SyntheticTag::Waiting))); } @@ -411,10 +420,14 @@ mod test { let mut tags: Vec<_> = task.get_tags().collect(); tags.sort(); - assert_eq!( - tags, - vec![utag("abc"), utag("def"), stag(SyntheticTag::Waiting),] - ); + let mut exp = vec![ + utag("abc"), + utag("def"), + stag(SyntheticTag::Pending), + stag(SyntheticTag::Waiting), + ]; + exp.sort(); + assert_eq!(tags, exp); } #[test] @@ -433,7 +446,7 @@ mod test { // only "ok" is OK let tags: Vec<_> = task.get_tags().collect(); - assert_eq!(tags, vec![utag("ok")]); + assert_eq!(tags, vec![utag("ok"), stag(SyntheticTag::Pending)]); } fn count_taskmap(task: &TaskMut, f: fn(&(&String, &String)) -> bool) -> usize { @@ -493,6 +506,20 @@ mod test { }); } + #[test] + fn test_done() { + with_mut_task(|mut task| { + task.done().unwrap(); + assert_eq!(task.get_status(), Status::Completed); + assert!(task.has_tag(&stag(SyntheticTag::Completed))); + + // redundant call does nothing.. + task.done().unwrap(); + assert_eq!(task.get_status(), Status::Completed); + assert!(task.has_tag(&stag(SyntheticTag::Completed))); + }); + } + #[test] fn test_stop_multiple() { with_mut_task(|mut task| { From 7956f6a34b85400eb5b8a418392297301ceee41f Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Sat, 5 Jun 2021 21:18:49 -0400 Subject: [PATCH 5/5] implement FromStr instead of just from_str --- taskchampion/src/task/tag.rs | 46 ++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/taskchampion/src/task/tag.rs b/taskchampion/src/task/tag.rs index 7554a57dd..d3a4842e7 100644 --- a/taskchampion/src/task/tag.rs +++ b/taskchampion/src/task/tag.rs @@ -22,7 +22,29 @@ pub(super) enum TagInner { pub const INVALID_TAG_CHARACTERS: &str = "+-*/(<>^! %=~"; impl Tag { - pub fn from_str(value: &str) -> Result { + /// True if this tag is a synthetic tag + pub fn is_synthetic(&self) -> bool { + matches!(self.0, TagInner::Synthetic(_)) + } + + /// True if this tag is a user-provided tag (not synthetic) + pub fn is_user(&self) -> bool { + matches!(self.0, TagInner::User(_)) + } + + pub(super) fn inner(&self) -> &TagInner { + &self.0 + } + + pub(super) fn from_inner(inner: TagInner) -> Self { + Self(inner) + } +} + +impl FromStr for Tag { + type Err = anyhow::Error; + + fn from_str(value: &str) -> Result { fn err(value: &str) -> Result { anyhow::bail!("invalid tag {:?}", value) } @@ -52,28 +74,6 @@ impl Tag { } Ok(Self(TagInner::User(String::from(value)))) } - - /// True if this tag is a synthetic tag - pub fn is_synthetic(&self) -> bool { - if let TagInner::Synthetic(_) = self.0 { - true - } else { - false - } - } - - /// True if this tag is a user-provided tag (not synthetic) - pub fn is_user(&self) -> bool { - !self.is_synthetic() - } - - pub(super) fn inner(&self) -> &TagInner { - &self.0 - } - - pub(super) fn from_inner(inner: TagInner) -> Self { - Self(inner) - } } impl TryFrom<&str> for Tag {