From 3d698f7939f715e53621e147dd95dd8d939fd6a1 Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Sat, 5 Jun 2021 20:07:33 -0400 Subject: [PATCH] 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")); }); }