diff --git a/cli/src/invocation/cmd/add.rs b/cli/src/invocation/cmd/add.rs index 1957e6dfc..5dfd5c215 100644 --- a/cli/src/invocation/cmd/add.rs +++ b/cli/src/invocation/cmd/add.rs @@ -1,19 +1,19 @@ -use crate::argparse::{DescriptionMod, Modification}; -use crate::invocation::apply_modification; +use crate::argparse::DescriptionMod; +use crate::invocation::{apply_modification, ResolvedModification}; use taskchampion::{Replica, Status}; use termcolor::WriteColor; -pub(crate) fn execute( +pub(in crate::invocation) fn execute( w: &mut W, replica: &mut Replica, - mut modification: Modification, + mut modification: ResolvedModification, ) -> Result<(), crate::Error> { // extract the description from the modification to handle it specially - let description = match modification.description { + let description = match modification.0.description { DescriptionMod::Set(ref s) => s.clone(), _ => "(no description)".to_owned(), }; - modification.description = DescriptionMod::None; + modification.0.description = DescriptionMod::None; let task = replica.new_task(Status::Pending, description).unwrap(); let mut task = task.into_mut(replica); @@ -25,6 +25,7 @@ pub(crate) fn execute( #[cfg(test)] mod test { use super::*; + use crate::argparse::Modification; use crate::invocation::test::*; use pretty_assertions::assert_eq; @@ -32,10 +33,10 @@ mod test { fn test_add() { let mut w = test_writer(); let mut replica = test_replica(); - let modification = Modification { + let modification = ResolvedModification(Modification { description: DescriptionMod::Set(s!("my description")), ..Default::default() - }; + }); execute(&mut w, &mut replica, modification).unwrap(); // check that the task appeared.. @@ -54,11 +55,11 @@ mod test { fn test_add_with_tags() { let mut w = test_writer(); let mut replica = test_replica(); - let modification = Modification { + let modification = ResolvedModification(Modification { description: DescriptionMod::Set(s!("my description")), add_tags: vec![tag!("tag1")].drain(..).collect(), ..Default::default() - }; + }); execute(&mut w, &mut replica, modification).unwrap(); // check that the task appeared.. diff --git a/cli/src/invocation/cmd/modify.rs b/cli/src/invocation/cmd/modify.rs index 3bf7e5b41..cb8eaf5b1 100644 --- a/cli/src/invocation/cmd/modify.rs +++ b/cli/src/invocation/cmd/modify.rs @@ -1,6 +1,6 @@ -use crate::argparse::{Filter, Modification}; +use crate::argparse::Filter; use crate::invocation::util::{confirm, summarize_task}; -use crate::invocation::{apply_modification, filtered_tasks}; +use crate::invocation::{apply_modification, filtered_tasks, ResolvedModification}; use crate::settings::Settings; use taskchampion::Replica; use termcolor::WriteColor; @@ -39,12 +39,12 @@ fn check_modification( Ok(false) } -pub(crate) fn execute( +pub(in crate::invocation) fn execute( w: &mut W, replica: &mut Replica, settings: &Settings, filter: Filter, - modification: Modification, + modification: ResolvedModification, ) -> Result<(), crate::Error> { let tasks = filtered_tasks(replica, &filter)?; @@ -68,7 +68,7 @@ pub(crate) fn execute( #[cfg(test)] mod test { use super::*; - use crate::argparse::DescriptionMod; + use crate::argparse::{DescriptionMod, Modification}; use crate::invocation::test::test_replica; use crate::invocation::test::*; use pretty_assertions::assert_eq; @@ -87,10 +87,10 @@ mod test { let filter = Filter { ..Default::default() }; - let modification = Modification { + let modification = ResolvedModification(Modification { description: DescriptionMod::Set(s!("new description")), ..Default::default() - }; + }); execute(&mut w, &mut replica, &settings, filter, modification).unwrap(); // check that the task appeared.. diff --git a/cli/src/invocation/mod.rs b/cli/src/invocation/mod.rs index 0ae3f44e0..0b75799f4 100644 --- a/cli/src/invocation/mod.rs +++ b/cli/src/invocation/mod.rs @@ -15,7 +15,7 @@ mod util; mod test; use filter::filtered_tasks; -use modify::apply_modification; +use modify::{apply_modification, resolve_modification, ResolvedModification}; use report::display_report; /// Invoke the given Command in the context of the given settings @@ -52,7 +52,10 @@ pub(crate) fn invoke(command: Command, settings: Settings) -> Result<(), crate:: Command { subcommand: Subcommand::Add { modification }, .. - } => return cmd::add::execute(&mut w, &mut replica, modification), + } => { + let modification = resolve_modification(modification, &mut replica)?; + return cmd::add::execute(&mut w, &mut replica, modification); + } Command { subcommand: @@ -61,7 +64,10 @@ pub(crate) fn invoke(command: Command, settings: Settings) -> Result<(), crate:: modification, }, .. - } => return cmd::modify::execute(&mut w, &mut replica, &settings, filter, modification), + } => { + let modification = resolve_modification(modification, &mut replica)?; + return cmd::modify::execute(&mut w, &mut replica, &settings, filter, modification); + } Command { subcommand: diff --git a/cli/src/invocation/modify.rs b/cli/src/invocation/modify.rs index 7a6896565..934ab1bc2 100644 --- a/cli/src/invocation/modify.rs +++ b/cli/src/invocation/modify.rs @@ -1,12 +1,97 @@ -use crate::argparse::{DescriptionMod, Modification}; +use crate::argparse::{DescriptionMod, Modification, TaskId}; +use std::collections::HashSet; use taskchampion::chrono::Utc; -use taskchampion::{Annotation, TaskMut}; +use taskchampion::{Annotation, Replica, TaskMut}; + +/// A wrapper for Modification, promising that all TaskId instances are of variant TaskId::Uuid. +pub(super) struct ResolvedModification(pub(super) Modification); + +/// Resolve a Modification to a ResolvedModification, based on access to a Replica. +/// +/// This is not automatically done in `apply_modification` because, by that time, the TaskMut being +/// modified has an exclusive reference to the Replica, so it is impossible to search for matching +/// tasks. +pub(super) fn resolve_modification( + unres: Modification, + replica: &mut Replica, +) -> anyhow::Result { + Ok(ResolvedModification(Modification { + description: unres.description, + status: unres.status, + wait: unres.wait, + active: unres.active, + add_tags: unres.add_tags, + remove_tags: unres.remove_tags, + add_dependencies: resolve_task_ids(replica, unres.add_dependencies)?, + remove_dependencies: resolve_task_ids(replica, unres.remove_dependencies)?, + annotate: unres.annotate, + })) +} + +/// Convert a set of arbitrary TaskId's into TaskIds containing only TaskId::Uuid. +fn resolve_task_ids( + replica: &mut Replica, + task_ids: HashSet, +) -> anyhow::Result> { + // already all UUIDs (or empty)? + if task_ids.iter().all(|tid| matches!(tid, TaskId::Uuid(_))) { + return Ok(task_ids); + } + + let mut result = HashSet::new(); + let mut working_set = None; + let mut all_tasks = None; + for tid in task_ids { + match tid { + TaskId::WorkingSetId(i) => { + let ws = match working_set { + Some(ref ws) => ws, + None => { + working_set = Some(replica.working_set()?); + working_set.as_ref().unwrap() + } + }; + if let Some(u) = ws.by_index(i) { + result.insert(TaskId::Uuid(u)); + } + } + TaskId::PartialUuid(partial) => { + let ts = match all_tasks { + Some(ref ts) => ts, + None => { + all_tasks = Some( + replica + .all_task_uuids()? + .drain(..) + .map(|u| (u, u.to_string())) + .collect::>(), + ); + all_tasks.as_ref().unwrap() + } + }; + for (u, ustr) in ts { + if ustr.starts_with(&partial) { + result.insert(TaskId::Uuid(*u)); + } + } + } + TaskId::Uuid(u) => { + result.insert(TaskId::Uuid(u)); + } + } + } + + Ok(result) +} /// Apply the given modification pub(super) fn apply_modification( task: &mut TaskMut, - modification: &Modification, + modification: &ResolvedModification, ) -> anyhow::Result<()> { + // unwrap the "Resolved" promise + let modification = &modification.0; + match modification.description { DescriptionMod::Set(ref description) => task.set_description(description.clone())?, DescriptionMod::Prepend(ref description) => { @@ -49,5 +134,114 @@ pub(super) fn apply_modification( })?; } + for tid in &modification.add_dependencies { + if let TaskId::Uuid(u) = tid { + task.add_dependency(*u)?; + } else { + // this Modification is resolved, so all TaskIds should + // be the Uuid variant. + unreachable!(); + } + } + + for tid in &modification.remove_dependencies { + if let TaskId::Uuid(u) = tid { + task.remove_dependency(*u)?; + } else { + // this Modification is resolved, so all TaskIds should + // be the Uuid variant. + unreachable!(); + } + } + Ok(()) } + +#[cfg(test)] +mod test { + use super::*; + use crate::invocation::test::*; + use pretty_assertions::assert_eq; + use taskchampion::{Status, Uuid}; + + #[test] + fn test_resolve_modifications() { + let mut replica = test_replica(); + let u1 = Uuid::new_v4(); + let t1 = replica.new_task(Status::Pending, "a task".into()).unwrap(); + replica.rebuild_working_set(true).unwrap(); + + let modi = Modification { + add_dependencies: set![TaskId::Uuid(u1), TaskId::WorkingSetId(1)], + ..Default::default() + }; + + let res = resolve_modification(modi, &mut replica).unwrap(); + + assert_eq!( + res.0.add_dependencies, + set![TaskId::Uuid(u1), TaskId::Uuid(t1.get_uuid())], + ); + } + + #[test] + fn test_resolve_task_ids_empty() { + let mut replica = test_replica(); + + assert_eq!( + resolve_task_ids(&mut replica, HashSet::new()).unwrap(), + HashSet::new() + ); + } + + #[test] + fn test_resolve_task_ids_all_uuids() { + let mut replica = test_replica(); + let uuid = Uuid::new_v4(); + let tids = set![TaskId::Uuid(uuid)]; + assert_eq!(resolve_task_ids(&mut replica, tids.clone()).unwrap(), tids); + } + + #[test] + fn test_resolve_task_ids_working_set_not_found() { + let mut replica = test_replica(); + let tids = set![TaskId::WorkingSetId(13)]; + assert_eq!( + resolve_task_ids(&mut replica, tids.clone()).unwrap(), + HashSet::new() + ); + } + + #[test] + fn test_resolve_task_ids_working_set() { + let mut replica = test_replica(); + let t1 = replica.new_task(Status::Pending, "a task".into()).unwrap(); + let t2 = replica + .new_task(Status::Pending, "another task".into()) + .unwrap(); + replica.rebuild_working_set(true).unwrap(); + let tids = set![TaskId::WorkingSetId(1), TaskId::WorkingSetId(2)]; + let resolved = set![TaskId::Uuid(t1.get_uuid()), TaskId::Uuid(t2.get_uuid())]; + assert_eq!(resolve_task_ids(&mut replica, tids).unwrap(), resolved); + } + + #[test] + fn test_resolve_task_ids_partial_not_found() { + let mut replica = test_replica(); + let tids = set![TaskId::PartialUuid("abcd".into())]; + assert_eq!( + resolve_task_ids(&mut replica, tids.clone()).unwrap(), + HashSet::new() + ); + } + + #[test] + fn test_resolve_task_ids_partial() { + let mut replica = test_replica(); + let t1 = replica.new_task(Status::Pending, "a task".into()).unwrap(); + let uuid_str = t1.get_uuid().to_string(); + let tids = set![TaskId::PartialUuid(uuid_str[..8].into())]; + let resolved = set![TaskId::Uuid(t1.get_uuid())]; + assert_eq!(resolve_task_ids(&mut replica, tids).unwrap(), resolved); + } +} diff --git a/cli/src/macros.rs b/cli/src/macros.rs index 1a3024c13..0f0eab6ad 100644 --- a/cli/src/macros.rs +++ b/cli/src/macros.rs @@ -12,6 +12,7 @@ macro_rules! argv { } /// Create a hashset, similar to vec! +// NOTE: in Rust 1.56.0, this can be changed to HashSet::from([..]) #[cfg(test)] macro_rules! set( { $($key:expr),+ } => {