dependency tracking; code cleanup; removed option to discard; move to

trash rather than delete

.gitignore

@@ -1,2 +1 @@
 /target
-/old

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

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 {
+pub fn id(id : Id) -> colored::ColoredString {
-    text.truecolor(52, 152, 219)
+    id.to_string().truecolor(52, 152, 219)
 }
 
 // Grey

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))))
             }
         }
 

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<process::ExitStatus, error::Error> {
@@ -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(())
         }

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),
         }
     }
 }

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<DisplayFromStr, _>")]
+    pub edges : HashMap<Id, HashSet<Id>>,
+}
+
+impl Graph {
+    pub fn create(tasks : Vec<tasks::Task>) -> 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<bool, error::Error> {
+        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<Vec<Id>> {
+
+        // All unvisited nodes, populated with all nodes at the start, to not miss disconnected
+        // components.
+        let mut unvisited = BTreeSet::<Id>::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<Id>, current_path_visited : &mut HashSet<Id>) -> Option<Vec<Id>> {
+
+        // 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<Id>) -> 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
+}

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()
+                                ids.iter()
-                                .map(|i| colour::id(&i.to_string()))
+                                .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)))),
                 }
             }
         }
-
     }
 }

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)) => {
+        Err(err) => {
-            println!("{} {}", colour::error("Error:"), message);
+            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!(),
         }

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()

src/stats.rs

@@ -39,7 +39,6 @@ pub fn time_per_tag(days : u16, vault_folder : &path::Path) -> Result<(), error:
     let mut times = BTreeMap::<String, tasks::Duration>::new();
 
     for task in &tasks {
-        if !task.data.discarded {
         let mut time = tasks::Duration::zero();
 
         for entry in &task.data.time_entries {
@@ -62,7 +61,6 @@ pub fn time_per_tag(days : u16, vault_folder : &path::Path) -> Result<(), error:
             }
         }
     }
-    }
 
     let mut table = comfy_table::Table::new();
     table

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<chrono::NaiveDateTime>,
     pub created : chrono::NaiveDateTime,
     pub completed : Option<chrono::NaiveDateTime>,
-    pub discarded : bool,
     pub info : Option<String>,
     pub time_entries : Vec<TimeEntry>,
 }
@@ -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<Vec<Self>, error::Error> {
+    fn id_iter(vault_folder : &path::Path) -> impl Iterator<Item = u64> {
-        let ids : Vec<Id> =
         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::<Id>().ok())
-            .collect();
+    }
 
-        let mut tasks = Vec::with_capacity(ids.len());
+    pub fn load_all(vault_folder : &path::Path, read_only : bool) -> Result<Vec<Self>, 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<HashMap<Id, Self>, 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)),
+                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() + id.chars().count() + discarded.chars().count()
+                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<T : fmt::Display>(set : &HashSet<T>) -> Result<String, error:
     Ok(output)
 }
 
+fn dependency_tree(start : Id, prefix : &String, is_last_item : bool, graph : &graph::Graph, tasks : &HashMap<Id, Task>) {
+    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))
+                format!("{} ({} overdue)", due.round_subsecs(0), 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))
-
             }
             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<TimeEntry>) -> Duration {
+    fn total(entries : &[TimeEntry]) -> Duration {
         entries
-        .into_iter()
+        .iter()
         .map(|e| e.duration)
         .fold(Duration::zero(), |a, d| a + d)
     }