From 170dbdcc2c6259a684a2e709861da24c3ba11b7c Mon Sep 17 00:00:00 2001 From: aaron-jack-manning Date: Thu, 25 Aug 2022 16:40:56 +1000 Subject: [PATCH] dependency tracking; code cleanup; removed option to discard; move to trash rather than delete --- .gitignore | 1 - README.md | 7 --- src/colour.rs | 6 ++- src/config.rs | 2 +- src/edit.rs | 22 +++++++- src/error.rs | 4 +- src/graph.rs | 134 ++++++++++++++++++++++++++++++++++++++++++++++++ src/index.rs | 10 ++-- src/main.rs | 36 ++++--------- src/state.rs | 7 ++- src/stats.rs | 30 +++++------ src/tasks.rs | 138 +++++++++++++++++++++++++++++++++----------------- 12 files changed, 287 insertions(+), 110 deletions(-) create mode 100644 src/graph.rs diff --git a/.gitignore b/.gitignore index 131f159..ea8c4bf 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1 @@ /target -/old diff --git a/README.md b/README.md index 53ea10e..3a7b4b3 100644 --- a/README.md +++ b/README.md @@ -85,12 +85,6 @@ Then you can run `toru new` to create your first task. ## Roadmap -- Dependency tracker - - Store dependencies in a file and correctly update them upon creation and removal of notes - - Error if any circular dependencies are introduced - - Make sure dependencies written to file are only those that could be successfully created - - List dependencies as a tree on note view below info - - Disallow marking a task as complete if it has incomplete dependencies - Options to configure and customise output of `list` - Simple query language to select: - which columns to include @@ -100,4 +94,3 @@ Then you can run `toru new` to create your first task. - only include tasks with incomplete dependencies, and similarly only tasks which are not dependents - due date, completion date, etc greater than or less than specific value - If no values given, use a default query stored in `state.toml` -- Automatically added recurring notes system diff --git a/src/colour.rs b/src/colour.rs index 7dd6e7f..4b78594 100644 --- a/src/colour.rs +++ b/src/colour.rs @@ -1,3 +1,5 @@ +use crate::tasks::Id; + use colored::Colorize; // Yellow @@ -26,8 +28,8 @@ pub fn file(text : &str) -> colored::ColoredString { } // Blue -pub fn id(text : &str) -> colored::ColoredString { - text.truecolor(52, 152, 219) +pub fn id(id : Id) -> colored::ColoredString { + id.to_string().truecolor(52, 152, 219) } // Grey diff --git a/src/config.rs b/src/config.rs index fca2dd8..03d909f 100644 --- a/src/config.rs +++ b/src/config.rs @@ -60,7 +60,7 @@ impl Config { Ok(()) }, None => { - Err(error::Error::Generic(format!("No vault named {} exists", colour::vault(&old_name)))) + Err(error::Error::Generic(format!("No vault named {} exists", colour::vault(old_name)))) } } diff --git a/src/edit.rs b/src/edit.rs index 56aae62..d5ac795 100644 --- a/src/edit.rs +++ b/src/edit.rs @@ -5,7 +5,9 @@ use std::process; use crate::tasks; use crate::error; +use crate::graph; use crate::state; +use crate::colour; use crate::tasks::Id; pub fn open_editor(path : &path::Path, editor : &str) -> Result { @@ -76,8 +78,24 @@ pub fn edit_raw(id : Id, vault_folder : path::PathBuf, editor : &str, state : &m Err(error::Error::Generic(String::from("Name must not be purely numeric"))) } else { + // Dependencies were edited so the graph needs to be updated. if edited_task.data.dependencies != task.data.dependencies { - // This is where the other dependencies graph needs to be updated. + for dependency in &task.data.dependencies { + state.data.deps.remove_edge(id, *dependency); + } + + for dependency in &edited_task.data.dependencies { + if state.data.deps.contains_node(*dependency) { + state.data.deps.insert_edge(id, *dependency)?; + } + else { + return Err(error::Error::Generic(format!("No task with an ID of {} exists", colour::id(*dependency)))); + } + } + + if let Some(cycle) = state.data.deps.find_cycle() { + return Err(error::Error::Generic(format!("Note edit aborted due to circular dependency: {}", graph::format_cycle(&cycle)))); + } } // Name change means index needs to be updated. if edited_task.data.name != task.data.name { @@ -90,7 +108,7 @@ pub fn edit_raw(id : Id, vault_folder : path::PathBuf, editor : &str, state : &m task.save()?; - fs::remove_file(&temp_path)?; + trash::delete(&temp_path)?; Ok(()) } diff --git a/src/error.rs b/src/error.rs index 0f0c0b9..88a73c3 100644 --- a/src/error.rs +++ b/src/error.rs @@ -14,6 +14,7 @@ pub enum Error { Utf8(str::Utf8Error), Fmt(fmt::Error), Generic(String), + Internal(String), } impl fmt::Display for Error { @@ -26,7 +27,8 @@ impl fmt::Display for Error { Error::TomlSer(err) => write!(f, "{} {}", colour::error("Internal Error:"), err), Error::Utf8(err) => write!(f, "{} {}", colour::error("Internal Error:"), err), Error::Fmt(err) => write!(f, "{} {}", colour::error("Internal Error:"), err), - Error::Generic(message) => write!(f, "{}", message), + Error::Generic(message) => write!(f, "{} {}", colour::error("Error:"), message), + Error::Internal(message) => write!(f, "{} {}", colour::error("Internal Error:"), message), } } } diff --git a/src/graph.rs b/src/graph.rs new file mode 100644 index 0000000..4f9caed --- /dev/null +++ b/src/graph.rs @@ -0,0 +1,134 @@ +use crate::error; +use crate::tasks; +use crate::colour; +use crate::tasks::Id; + +use std::fmt::Write; +use std::collections::{HashSet, HashMap, BTreeSet}; +use serde_with::{serde_as, DisplayFromStr}; + +#[serde_as] +#[derive(Clone, Debug, serde::Serialize, serde::Deserialize)] +pub struct Graph { + #[serde_as(as = "HashMap")] + pub edges : HashMap>, +} + +impl Graph { + pub fn create(tasks : Vec) -> Self { + let mut edges = HashMap::with_capacity(tasks.len()); + + for task in tasks { + edges.insert(task.data.id, task.data.dependencies); + } + + Self { + edges + } + } + + pub fn contains_node(&self, node : Id) -> bool { + self.edges.contains_key(&node) + } + + pub fn insert_node(&mut self, node : Id) -> bool { + self.edges.insert(node, HashSet::new()).is_none() + } + + pub fn insert_edge(&mut self, first : Id, second : Id) -> Result { + if !self.contains_node(first) || !self.contains_node(second) { + Err(error::Error::Internal(String::from("Attempt to insert an edge in the dependency graph with a node which wasn't present"))) + } + else if first == second { + Err(error::Error::Generic(format!("Note with ID {} cannot depend on itself", colour::id(first)))) + } + else { + let outgoing = self.edges.get_mut(&first).unwrap(); + Ok(outgoing.insert(second)) + } + } + + pub fn remove_node(&mut self, node : Id) -> bool { + if self.edges.remove(&node).is_some() { + for outgoing in self.edges.values_mut() { + outgoing.remove(&node); + } + true + } + else { + false + } + } + + pub fn remove_edge(&mut self, first : Id, second : Id) -> bool { + match self.edges.get_mut(&first) { + Some(outgoing) => { + outgoing.remove(&second) + }, + None => { + false + } + } + } + + pub fn find_cycle(&self) -> Option> { + + // All unvisited nodes, populated with all nodes at the start, to not miss disconnected + // components. + let mut unvisited = BTreeSet::::new(); + for node in self.edges.keys() { + unvisited.insert(*node); + } + + while !unvisited.is_empty() { + let start = unvisited.iter().next().unwrap(); + + let result = self.find_cycle_local(*start, &mut unvisited, &mut HashSet::new()); + if result.is_some() { + return result; + } + } + + None + } + + fn find_cycle_local(&self, start : Id, unvisited : &mut BTreeSet, current_path_visited : &mut HashSet) -> Option> { + + // If already visited in the current path, then there is a cycle + if current_path_visited.contains(&start) { + Some(vec![start]) + } + else { + unvisited.remove(&start); + current_path_visited.insert(start); + + // Iterate over the outgoing edges + for node in self.edges.get(&start).unwrap() { + let result = self.find_cycle_local(*node, unvisited, current_path_visited); + if let Some(mut path) = result { + path.push(start); + return Some(path); + } + // Remove the searched node from the current_path_visited set because already + // reached full search depth on it. + current_path_visited.remove(node); + } + + None + } + } +} + +pub fn format_cycle(cycle : &Vec) -> String { + let mut formatted = String::new(); + + for (index, node) in cycle.iter().enumerate() { + write!(&mut formatted, "{}", colour::id(*node)).unwrap(); + + if index != cycle.len() - 1 { + formatted.push_str(" -> "); + } + } + + formatted +} diff --git a/src/index.rs b/src/index.rs index f468b2f..9849fb5 100644 --- a/src/index.rs +++ b/src/index.rs @@ -3,6 +3,7 @@ use crate::error; use crate::colour; use crate::tasks::Id; +use std::fmt::Write; use std::collections::HashMap; use serde_with::{serde_as, DisplayFromStr}; @@ -67,14 +68,14 @@ impl Index { } else { let coloured_ids : Vec<_> = - ids.into_iter() - .map(|i| colour::id(&i.to_string())) + ids.iter() + .map(|i| colour::id(*i)) .collect(); let mut display_ids = String::new(); for id in coloured_ids { - display_ids.push_str(&format!("{}, ", id)); + write!(&mut display_ids, "{}, ", id).unwrap(); } if !display_ids.is_empty() { @@ -85,10 +86,9 @@ impl Index { Err(error::Error::Generic(format!("Multiple notes (Ids: [{}]) by that name exist", display_ids))) } }, - None => Err(error::Error::Generic(format!("A note by the name {} does not exist", colour::task_name(&name)))), + None => Err(error::Error::Generic(format!("A note by the name {} does not exist", colour::task_name(name)))), } } } - } } diff --git a/src/main.rs b/src/main.rs index de5dd15..b2b04db 100644 --- a/src/main.rs +++ b/src/main.rs @@ -5,6 +5,7 @@ mod index; mod error; mod tasks; mod state; +mod graph; mod stats; mod config; mod colour; @@ -49,16 +50,10 @@ enum Command { #[clap(short, long)] info : bool, }, - /// Delete a task completely. + /// Delete a task (move file to trash). Delete { id_or_name : String, }, - /// Deletes all discarded tasks. - Clean, - /// Discard a task without deleting the underlying file. - Discard { - id_or_name : String, - }, /// Mark a task as complete. Complete { id_or_name : String, @@ -165,10 +160,9 @@ fn main() { match result { Ok(()) => (), - Err(error::Error::Generic(message)) => { - println!("{} {}", colour::error("Error:"), message); + Err(err) => { + println!("{}", err); } - result => println!("{:?}", result), } } @@ -250,21 +244,22 @@ fn program() -> Result<(), error::Error> { match command { New { name, info, tags, dependencies, priority, due } => { let task = tasks::Task::new(name, info, tags, dependencies, priority, due, vault_folder, &mut state)?; - println!("Created task {} (ID: {})", colour::task_name(&task.data.name), colour::id(&task.data.id.to_string())); + println!("Created task {} (ID: {})", colour::task_name(&task.data.name), colour::id(task.data.id)); }, Delete { id_or_name } => { let id = state.data.index.lookup(&id_or_name)?; let task = tasks::Task::load(id, vault_folder, false)?; let name = task.data.name.clone(); state.data.index.remove(task.data.name.clone(), task.data.id); + state.data.deps.remove_node(task.data.id); task.delete()?; - println!("Deleted task {} (ID: {})", colour::task_name(&name), colour::id(&id.to_string())); + println!("Deleted task {} (ID: {})", colour::task_name(&name), colour::id(id)); }, View { id_or_name } => { let id = state.data.index.lookup(&id_or_name)?; let task = tasks::Task::load(id, vault_folder, true)?; - task.display()?; + task.display(vault_folder, &state)?; }, Edit { id_or_name, info } => { let id = state.data.index.lookup(&id_or_name)?; @@ -274,7 +269,7 @@ fn program() -> Result<(), error::Error> { else { edit::edit_raw(id, vault_folder.clone(), &config.editor, &mut state)?; } - println!("Updated task {}", colour::id(&id.to_string())); + println!("Updated task {}", colour::id(id)); }, Track { id_or_name, hours, minutes } => { let id = state.data.index.lookup(&id_or_name)?; @@ -294,27 +289,16 @@ fn program() -> Result<(), error::Error> { } } }, - Discard { id_or_name } => { - let id = state.data.index.lookup(&id_or_name)?; - let mut task = tasks::Task::load(id, vault_folder, false)?; - task.data.discarded = true; - task.save()?; - println!("Discarded task {}", colour::id(&id.to_string())); - }, Complete { id_or_name } => { let id = state.data.index.lookup(&id_or_name)?; let mut task = tasks::Task::load(id, vault_folder, false)?; task.data.completed = Some(chrono::Local::now().naive_local()); task.save()?; - println!("Marked task {} as complete", colour::id(&id.to_string())); + println!("Marked task {} as complete", colour::id(id)); }, List {} => { tasks::list(vault_folder)?; }, - Clean => { - tasks::clean(vault_folder)?; - println!("Deleted all discarded tasks"); - } // All commands which are dealt with in if let chain at start. Vault(_) | Config(_) | Git { args : _ } | Svn { args : _ } | Switch { name : _ } | GitIgnore => unreachable!(), } diff --git a/src/state.rs b/src/state.rs index b8ead06..c32f860 100644 --- a/src/state.rs +++ b/src/state.rs @@ -1,6 +1,7 @@ use crate::error; use crate::tasks; use crate::index; +use crate::graph; use crate::tasks::Id; use std::fs; @@ -18,6 +19,7 @@ pub struct State { pub struct InternalState { pub next_id : Id, pub index : index::Index, + pub deps : graph::Graph, } impl State { @@ -52,14 +54,15 @@ impl State { } } - // Calculating out the index. + // Calculating out the index and graph. let tasks = tasks::Task::load_all(vault_location, true)?; - let index = index::Index::create(&tasks); + let deps = graph::Graph::create(tasks); let data = InternalState { next_id : u64::try_from(max_id + 1).unwrap(), index, + deps, }; let mut file = fs::File::options() diff --git a/src/stats.rs b/src/stats.rs index 1d584b8..07fc244 100644 --- a/src/stats.rs +++ b/src/stats.rs @@ -39,26 +39,24 @@ pub fn time_per_tag(days : u16, vault_folder : &path::Path) -> Result<(), error: let mut times = BTreeMap::::new(); for task in &tasks { - if !task.data.discarded { - let mut time = tasks::Duration::zero(); + let mut time = tasks::Duration::zero(); - for entry in &task.data.time_entries { - if chrono::Utc::now().naive_local().date() - entry.logged_date < chrono::Duration::days(i64::from(days)) { - time = time + entry.duration; - } + for entry in &task.data.time_entries { + if chrono::Utc::now().naive_local().date() - entry.logged_date < chrono::Duration::days(i64::from(days)) { + time = time + entry.duration; } + } - let tag_count = task.data.tags.len(); - let time_per_tag = time / tag_count; + let tag_count = task.data.tags.len(); + let time_per_tag = time / tag_count; - for tag in &task.data.tags { - match times.get_mut(tag) { - Some(time) => { - *time = *time + time_per_tag; - }, - None => { - times.insert(tag.clone(), time_per_tag); - } + for tag in &task.data.tags { + match times.get_mut(tag) { + Some(time) => { + *time = *time + time_per_tag; + }, + None => { + times.insert(tag.clone(), time_per_tag); } } } diff --git a/src/tasks.rs b/src/tasks.rs index c1cd33b..e23eee0 100644 --- a/src/tasks.rs +++ b/src/tasks.rs @@ -1,5 +1,6 @@ use crate::error; use crate::state; +use crate::graph; use crate::colour; use std::io; @@ -9,7 +10,7 @@ use std::ops; use std::mem; use std::path; use std::io::{Write, Seek}; -use std::collections::HashSet; +use std::collections::{HashSet, HashMap}; use colored::Colorize; use chrono::SubsecRound; @@ -77,7 +78,6 @@ pub struct InternalTask { pub due : Option, pub created : chrono::NaiveDateTime, pub completed : Option, - pub discarded : bool, pub info : Option, pub time_entries : Vec, } @@ -99,6 +99,19 @@ impl Task { .create(true) .open(&path)?; + // Adding to dependency graph appropriately. + state.data.deps.insert_node(id); + if !dependencies.is_empty() { + for dependency in &dependencies { + if state.data.deps.contains_node(*dependency) { + state.data.deps.insert_edge(id, *dependency)?; + } + else { + return Err(error::Error::Generic(format!("No task with an ID of {} exists", colour::id(*dependency)))); + } + } + } + let data = InternalTask { id, name, @@ -110,7 +123,6 @@ impl Task { time_entries : Vec::new(), created : chrono::Local::now().naive_local(), completed : None, - discarded : false, }; let file_contents = toml::to_string(&data)?; @@ -160,17 +172,19 @@ impl Task { Task::load_direct(path, read_only) } - pub fn load_all(vault_folder : &path::Path, read_only : bool) -> Result, error::Error> { - let ids : Vec = - fs::read_dir(vault_folder.join("notes")) - .unwrap() - .map(|entry| entry.unwrap().path()) - .filter(|p| p.is_file()) - .map(|p| p.file_stem().unwrap().to_str().unwrap().to_string()) - .filter_map(|n| n.parse::().ok()) - .collect(); + fn id_iter(vault_folder : &path::Path) -> impl Iterator { + fs::read_dir(vault_folder.join("notes")) + .unwrap() + .map(|entry| entry.unwrap().path()) + .filter(|p| p.is_file()) + .map(|p| p.file_stem().unwrap().to_str().unwrap().to_string()) + .filter_map(|n| n.parse::().ok()) + } - let mut tasks = Vec::with_capacity(ids.len()); + pub fn load_all(vault_folder : &path::Path, read_only : bool) -> Result, error::Error> { + let ids = Task::id_iter(vault_folder); + + let mut tasks = Vec::new(); for id in ids { tasks.push(Task::load(id, vault_folder, read_only)?); } @@ -178,6 +192,17 @@ impl Task { Ok(tasks) } + pub fn load_all_as_map(vault_folder : &path::Path, read_only : bool) -> Result, error::Error> { + let ids = Task::id_iter(vault_folder); + + let mut tasks = HashMap::new(); + for id in ids { + tasks.insert(id, Task::load(id, vault_folder, read_only)?); + } + + Ok(tasks) + } + pub fn path(&self) -> &path::Path { &self.path } @@ -188,7 +213,7 @@ impl Task { Ok(path) } else { - Err(error::Error::Generic(format!("No task with the ID {} exists", colour::id(&id.to_string())))) + Err(error::Error::Generic(format!("No task with the ID {} exists", colour::id(id)))) } } @@ -216,12 +241,12 @@ impl Task { } = self; mem::drop(file); - fs::remove_file(&path)?; + trash::delete(&path)?; Ok(()) } - pub fn display(&self) -> Result<(), error::Error> { + pub fn display(&self, vault_folder : &path::Path, state : &state::State) -> Result<(), error::Error> { fn line(len : usize) { for _ in 0..len { @@ -231,12 +256,10 @@ impl Task { } let (heading, heading_length) = { - let id = &self.data.id.to_string(); - let discarded = if self.data.discarded { String::from(" (discarded)") } else { String::new() }; ( - format!("[{}] {} {}{}", if self.data.completed.is_some() {"X"} else {" "}, colour::id(id), colour::task_name(&self.data.name), colour::greyed_out(&discarded)), - 5 + self.data.name.chars().count() + id.chars().count() + discarded.chars().count() + format!("[{}] {} {}", if self.data.completed.is_some() {"X"} else {" "}, colour::id(self.data.id), colour::task_name(&self.data.name)), + 5 + self.data.name.chars().count() + self.data.id.to_string().chars().count() ) }; @@ -286,7 +309,12 @@ impl Task { } } - // dependencies as a tree + if !self.data.dependencies.is_empty() { + let tasks = Task::load_all_as_map(vault_folder, true)?; + + println!("Dependencies:"); + dependency_tree(self.data.id, &String::new(), true, &state.data.deps, &tasks); + } Ok(()) } @@ -308,6 +336,43 @@ fn format_hash_set(set : &HashSet) -> Result) { + let next = graph.edges.get(&start).unwrap(); + + { + let task = tasks.get(&start).unwrap(); + + let name = if task.data.completed.is_some() { + colour::greyed_out(&task.data.name) + } + else { + colour::task_name(&task.data.name) + }; + + if is_last_item { + println!("{}└──{} (ID: {})", prefix, name, colour::id(start)) + } + else { + println!("{}├──{} (ID: {})", prefix, name, colour::id(start)) + } + } + + let count = next.len(); + + for (i, node) in next.iter().enumerate() { + let new_is_last_item = i == count - 1; + + let new_prefix = if is_last_item { + format!("{} ", prefix) + } + else { + format!("{}│ ", prefix) + }; + + dependency_tree(*node, &new_prefix, new_is_last_item, graph, tasks); + } +} + fn format_due_date(due : &chrono::NaiveDateTime, include_fuzzy_period : bool, colour : bool) -> String { let remaining = *due - chrono::Local::now().naive_local(); @@ -347,18 +412,10 @@ fn format_due_date(due : &chrono::NaiveDateTime, include_fuzzy_period : bool, co } else { if remaining < chrono::Duration::zero() { - format!("{} {}", due.round_subsecs(0), format!("({} overdue)", fuzzy_period)) - } - else if remaining < chrono::Duration::days(1) { - format!("{} {}", due.round_subsecs(0), format!("({} remaining)", fuzzy_period)) - - } - else if remaining < chrono::Duration::days(3) { - format!("{} {}", due.round_subsecs(0), format!("({} remaining)", fuzzy_period)) - + format!("{} ({} overdue)", due.round_subsecs(0), fuzzy_period) } else { - format!("{} {}", due.round_subsecs(0), format!("({} remaining)", fuzzy_period)) + format!("{} ({} remaining)", due.round_subsecs(0), fuzzy_period) } } } @@ -380,7 +437,7 @@ pub fn list(vault_folder : &path::Path) -> Result<(), error::Error> { tasks.sort_by(|t1, t2| t2.data.priority.cmp(&t1.data.priority)); for task in tasks { - if !task.data.discarded && task.data.completed.is_none() { + if task.data.completed.is_none() { let duration = TimeEntry::total(&task.data.time_entries); @@ -402,19 +459,6 @@ pub fn list(vault_folder : &path::Path) -> Result<(), error::Error> { Ok(()) } -pub fn clean(vault_folder : &path::Path) -> Result<(), error::Error> { - - let tasks = Task::load_all(vault_folder, false)?; - - for task in tasks { - if task.data.discarded { - task.delete()?; - } - } - - Ok(()) -} - impl ops::Add for Duration { type Output = Self; @@ -460,9 +504,9 @@ impl fmt::Display for Duration { impl TimeEntry { /// Adds up the times from a collection of time entries. - fn total(entries : &Vec) -> Duration { + fn total(entries : &[TimeEntry]) -> Duration { entries - .into_iter() + .iter() .map(|e| e.duration) .fold(Duration::zero(), |a, d| a + d) }