From a73712df8840c63210ce8aceed9d0738931e5317 Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Fri, 21 Jan 2022 09:07:20 +0200 Subject: refactor: format code with rustfmt Sponsored-by: author --- src/bin/roadmap2dot.rs | 2 +- src/err.rs | 10 ++-------- src/map.rs | 10 ++++------ src/parser.rs | 2 +- 4 files changed, 8 insertions(+), 16 deletions(-) diff --git a/src/bin/roadmap2dot.rs b/src/bin/roadmap2dot.rs index b180da8..a9b7302 100644 --- a/src/bin/roadmap2dot.rs +++ b/src/bin/roadmap2dot.rs @@ -11,12 +11,12 @@ //! test the library crate. It is expected that serious use of the //! crate will be as a library. +use anyhow::Result; use roadmap::from_yaml; use std::fs::File; use std::io::Read; use std::path::PathBuf; use structopt::StructOpt; -use anyhow::Result; const LABEL_WIDTH: usize = 30; diff --git a/src/err.rs b/src/err.rs index b928b09..c5210f1 100644 --- a/src/err.rs +++ b/src/err.rs @@ -7,16 +7,10 @@ pub enum RoadmapError { NoGoals, #[error("too many goals, must have exactly one: found {count:}: {}", .names.join(", "))] - ManyGoals { - count: usize, - names: Vec, - }, + ManyGoals { count: usize, names: Vec }, #[error("step {name:} depends on missing {missing:}")] - MissingDep { - name: String, - missing: String, - }, + MissingDep { name: String, missing: String }, #[error("step is not a mapping")] StepNotMapping, diff --git a/src/map.rs b/src/map.rs index 874e537..e6a24a9 100644 --- a/src/map.rs +++ b/src/map.rs @@ -135,10 +135,7 @@ impl Roadmap { 1 => (), _ => { let names: Vec = goals.iter().map(|s| s.name().into()).collect(); - return Err(RoadmapError::ManyGoals { - count: n, - names, - }); + return Err(RoadmapError::ManyGoals { count: n, names }); } } @@ -146,11 +143,12 @@ impl Roadmap { for step in self.iter() { for depname in step.dependencies() { match self.get_step(depname) { - None => + None => { return Err(RoadmapError::MissingDep { name: step.name().into(), missing: depname.into(), - }), + }) + } Some(_) => (), } } diff --git a/src/parser.rs b/src/parser.rs index d83f618..6cc3d25 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -46,7 +46,7 @@ fn parse_depends(map: &Value) -> RoadmapResult> { let key_name = "depends"; let key = Value::String(key_name.to_string()); let mut depends: Vec<&str> = vec![]; - + match map.get(&key) { None => (), Some(Value::Sequence(deps)) => { -- cgit v1.2.1 From 8745b8524173ff8a2261f3d6209cf87a384be68d Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Fri, 21 Jan 2022 09:07:43 +0200 Subject: refactor: sort dependencies in Cargo.toml Sponsored-by: author --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 76b4c2f..a5f1129 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,8 +10,8 @@ repostiory = "https://gitlab.com/larswirzenius/roadmap" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] +anyhow = "1" serde_yaml = "0.8" structopt = "0.3" textwrap = "0.14" thiserror = "1" -anyhow = "1" -- cgit v1.2.1 From 82073da80b0f38d605204e96bf4b430c2b785813 Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Fri, 21 Jan 2022 09:08:24 +0200 Subject: chore: update versions of dependencies Sponsored-by: author --- Cargo.lock | 62 +++++++++++++++++++++++++++++++------------------------------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 914623a..87e468d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -13,18 +13,18 @@ dependencies = [ [[package]] name = "ansi_term" -version = "0.11.0" +version = "0.12.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ee49baf6cb617b853aa8d93bf420db2383fab46d314482ca2803b40d5fde979b" +checksum = "d52a9bb7ec0cf484c551830a7ce27bd20d67eac647e1befb56b0be4ee39a55d2" dependencies = [ "winapi", ] [[package]] name = "anyhow" -version = "1.0.48" +version = "1.0.52" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "62e1f47f7dc0422027a4e370dd4548d4d66b26782e513e98dca1e689e058a80e" +checksum = "84450d0b4a8bd1ba4144ce8ce718fbc5d071358b1e5384bace6536b3d1f2d5b3" [[package]] name = "atty" @@ -51,9 +51,9 @@ checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" [[package]] name = "clap" -version = "2.33.3" +version = "2.34.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "37e58ac78573c40708d45522f0d80fa2f01cc4f9b4e2bf749807255454312002" +checksum = "a0610544180c38b88101fecf2dd634b174a62eef6946f84dfc6a7127512b381c" dependencies = [ "ansi_term", "atty", @@ -64,12 +64,6 @@ dependencies = [ "vec_map", ] -[[package]] -name = "dtoa" -version = "0.4.8" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "56899898ce76aaf4a0f24d914c97ea6ed976d42fec6ad33fcbb0a1103e07b2b0" - [[package]] name = "hashbrown" version = "0.11.2" @@ -96,9 +90,9 @@ dependencies = [ [[package]] name = "indexmap" -version = "1.7.0" +version = "1.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bc633605454125dec4b66843673f01c7df2b89479b32e0ed634e43a91cff62a5" +checksum = "282a6247722caba404c065016bbfa522806e51714c34f5dfc3e4a3a46fcb4223" dependencies = [ "autocfg", "hashbrown", @@ -112,9 +106,9 @@ checksum = "e2abad23fbc42b3700f2f279844dc832adb2b2eb069b2df918f455c4e18cc646" [[package]] name = "libc" -version = "0.2.108" +version = "0.2.113" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8521a1b57e76b1ec69af7599e75e38e7b7fad6610f037db8c79b127201b5d119" +checksum = "eef78b64d87775463c549fbd80e19249ef436ea3bf1de2a1eb7e717ec7fab1e9" [[package]] name = "linked-hash-map" @@ -154,18 +148,18 @@ dependencies = [ [[package]] name = "proc-macro2" -version = "1.0.32" +version = "1.0.36" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ba508cc11742c0dc5c1659771673afbab7a0efab23aa17e854cbab0837ed0b43" +checksum = "c7342d5883fbccae1cc37a2353b09c87c9b0f3afd73f5fb9bba687a1f733b029" dependencies = [ "unicode-xid", ] [[package]] name = "quote" -version = "1.0.10" +version = "1.0.14" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "38bc8cc6a5f2e3655e0899c1b848643b2562f853f114bfec7be120678e3ace05" +checksum = "47aa80447ce4daf1717500037052af176af5d38cc3e571d9ec1c7353fc10c87d" dependencies = [ "proc-macro2", ] @@ -198,20 +192,26 @@ dependencies = [ "thiserror", ] +[[package]] +name = "ryu" +version = "1.0.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "73b4b750c782965c211b42f022f59af1fbceabdd026623714f104152f1ec149f" + [[package]] name = "serde" -version = "1.0.130" +version = "1.0.134" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f12d06de37cf59146fbdecab66aa99f9fe4f78722e3607577a5375d66bd0c913" +checksum = "96b3c34c1690edf8174f5b289a336ab03f568a4460d8c6df75f2f3a692b3bc6a" [[package]] name = "serde_yaml" -version = "0.8.21" +version = "0.8.23" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d8c608a35705a5d3cdc9fbe403147647ff34b921f8e833e49306df898f9b20af" +checksum = "a4a521f2940385c165a24ee286aa8599633d162077a54bdcae2a6fd5a7bfa7a0" dependencies = [ - "dtoa", "indexmap", + "ryu", "serde", "yaml-rust", ] @@ -230,9 +230,9 @@ checksum = "8ea5119cdb4c55b55d432abb513a0429384878c15dde60cc77b1c99de1a95a6a" [[package]] name = "structopt" -version = "0.3.25" +version = "0.3.26" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "40b9788f4202aa75c240ecc9c15c65185e6a39ccdeb0fd5d008b98825464c87c" +checksum = "0c6b5c64445ba8094a6ab0c3cd2ad323e07171012d9c98b0b15651daf1787a10" dependencies = [ "clap", "lazy_static", @@ -254,9 +254,9 @@ dependencies = [ [[package]] name = "syn" -version = "1.0.81" +version = "1.0.86" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f2afee18b8beb5a596ecb4a2dce128c719b4ba399d34126b9e4396e3f9860966" +checksum = "8a65b3f4ffa0092e9887669db0eae07941f023991ab58ea44da8fe8e2d511c6b" dependencies = [ "proc-macro2", "quote", @@ -338,9 +338,9 @@ checksum = "f1bddf1187be692e79c5ffeab891132dfb0f236ed36a43c7ed39f1165ee20191" [[package]] name = "version_check" -version = "0.9.3" +version = "0.9.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5fecdca9a5291cc2b8dcf7dc02453fee791a280f3743cb0905f8822ae463b3fe" +checksum = "49874b5167b65d7193b8aba1567f5c7d93d001cafc34600cee003eda787e483f" [[package]] name = "winapi" -- cgit v1.2.1 From f49a7c9eaa14ce1aaf342d862ec292b84fa2e7bc Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Fri, 21 Jan 2022 09:34:47 +0200 Subject: refactor: parse with serde directly to data structure Use serde to parse directly to a hashmap of steps, instead of generic YAML values. This gives us better error messages. Sponsored-by: author --- Cargo.lock | 15 +++++++++ Cargo.toml | 1 + src/map.rs | 18 +++++++---- src/parser.rs | 101 +++++----------------------------------------------------- src/status.rs | 11 ++++++- src/step.rs | 15 +++++++-- 6 files changed, 58 insertions(+), 103 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 87e468d..be68abe 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -186,6 +186,7 @@ name = "roadmap" version = "0.3.0" dependencies = [ "anyhow", + "serde", "serde_yaml", "structopt", "textwrap 0.14.2", @@ -203,6 +204,20 @@ name = "serde" version = "1.0.134" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "96b3c34c1690edf8174f5b289a336ab03f568a4460d8c6df75f2f3a692b3bc6a" +dependencies = [ + "serde_derive", +] + +[[package]] +name = "serde_derive" +version = "1.0.134" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "784ed1fbfa13fe191077537b0d70ec8ad1e903cfe04831da608aa36457cb653d" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] [[package]] name = "serde_yaml" diff --git a/Cargo.toml b/Cargo.toml index a5f1129..40ef824 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,6 +11,7 @@ repostiory = "https://gitlab.com/larswirzenius/roadmap" [dependencies] anyhow = "1" +serde = { version = "1.0.134", features = ["derive"] } serde_yaml = "0.8" structopt = "0.3" textwrap = "0.14" diff --git a/src/map.rs b/src/map.rs index e6a24a9..b592fbd 100644 --- a/src/map.rs +++ b/src/map.rs @@ -1,3 +1,5 @@ +use std::collections::HashMap; + use textwrap::fill; pub use crate::from_yaml; @@ -21,8 +23,10 @@ impl Roadmap { /// Create a new, empty roadmap. /// /// You probably want the `from_yaml` function instead. - pub fn new() -> Self { - Self::default() + pub fn new(map: HashMap) -> Self { + Self { + steps: map.values().cloned().collect(), + } } // Find steps that nothing depends on. @@ -220,13 +224,13 @@ mod tests { #[test] fn new_roadmap() { - let roadmap = Roadmap::new(); + let roadmap = Roadmap::default(); assert_eq!(roadmap.step_names().count(), 0); } #[test] fn add_step_to_roadmap() { - let mut roadmap = Roadmap::new(); + let mut roadmap = Roadmap::default(); let first = Step::new("first", "the first step"); roadmap.add_step(first); let names: Vec<&str> = roadmap.step_names().collect(); @@ -235,7 +239,7 @@ mod tests { #[test] fn get_step_from_roadmap() { - let mut roadmap = Roadmap::new(); + let mut roadmap = Roadmap::default(); let first = Step::new("first", "the first step"); roadmap.add_step(first); let gotit = roadmap.get_step("first").unwrap(); @@ -279,7 +283,7 @@ blocked: #[test] fn empty_dot() { - let roadmap = Roadmap::new(); + let roadmap = Roadmap::default(); match roadmap.format_as_dot(999) { Err(_) => (), _ => panic!("expected error for empty roadmap"), @@ -288,7 +292,7 @@ blocked: #[test] fn simple_dot() { - let mut roadmap = Roadmap::new(); + let mut roadmap = Roadmap::default(); let mut first = Step::new("first", ""); first.set_status(Status::Ready); let mut second = Step::new("second", ""); diff --git a/src/parser.rs b/src/parser.rs index 6cc3d25..8c7a9c1 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1,4 +1,3 @@ -use serde_yaml::Value; use std::collections::HashMap; pub use crate::Roadmap; @@ -9,121 +8,37 @@ pub use crate::Step; /// Create a new roadmap from a textual YAML representation. pub fn from_yaml(yaml: &str) -> RoadmapResult { - let mut roadmap = Roadmap::new(); - let map: HashMap = serde_yaml::from_str(yaml)?; - - for (name, value) in map { - let step = step_from_value(&name, &value)?; - roadmap.add_step(step); + let mut roadmap: HashMap = serde_yaml::from_str(yaml)?; + for (name, step) in roadmap.iter_mut() { + step.set_name(name); } - + let roadmap = Roadmap::new(roadmap); roadmap.validate()?; Ok(roadmap) } -// Convert a Value into a Step, if possible. -fn step_from_value(name: &str, value: &Value) -> RoadmapResult { - match value { - Value::Mapping(_) => { - let label = parse_label(value); - let status = parse_status(value)?; - - let mut step = Step::new(name, label); - step.set_status(status); - - for depname in parse_depends(value)?.iter() { - step.add_dependency(depname); - } - - Ok(step) - } - _ => Err(RoadmapError::StepNotMapping), - } -} - -// Get a sequence of depenencies. -fn parse_depends(map: &Value) -> RoadmapResult> { - let key_name = "depends"; - let key = Value::String(key_name.to_string()); - let mut depends: Vec<&str> = vec![]; - - match map.get(&key) { - None => (), - Some(Value::Sequence(deps)) => { - for depname in deps.iter() { - match depname { - Value::String(depname) => depends.push(depname), - _ => return Err(RoadmapError::DependsNotNames), - } - } - } - _ => return Err(RoadmapError::DependsNotNames), - } - - Ok(depends) -} - -// Get label string from a Mapping element, or empty string. -fn parse_label(map: &Value) -> &str { - parse_string("label", map) -} - -// Get status string from a Mapping element. Default to unknown status. -fn parse_status(map: &Value) -> RoadmapResult { - let text = parse_string("status", map); - match Status::from_text(text) { - Some(status) => Ok(status), - _ => Err(RoadmapError::UnknownStatus(text.into())), - } -} - -// Get string value from a Mapping field, or empty string if field -// isn't there. -fn parse_string<'a>(key_name: &str, map: &'a Value) -> &'a str { - let key = Value::String(key_name.to_string()); - match map.get(&key) { - Some(Value::String(s)) => s, - _ => "", - } -} - #[cfg(test)] mod tests { use super::from_yaml; #[test] fn yaml_is_empty() { - if from_yaml("").is_ok() { - panic!("expected a parse error"); - } + assert!(from_yaml("").is_err()); } #[test] fn yaml_is_list() { - if from_yaml("[]").is_ok() { - panic!("expected a parse error"); - } - } - - #[test] - fn yaml_map_entries_not_maps() { - if from_yaml("foo: []").is_ok() { - panic!("expected a parse error"); - } + assert!(from_yaml("[]").is_err()); } #[test] fn yaml_unknown_dep() { - if from_yaml("foo: {depends: [bar]}").is_ok() { - panic!("expected a parse error"); - } + assert!(from_yaml("foo: {depends: [bar]}").is_err()); } #[test] fn yaml_unknown_status() { - if from_yaml(r#"foo: {status: "bar"}"#).is_ok() { - panic!("expected a parse error"); - } + assert!(from_yaml(r#"foo: {status: "bar"}"#).is_err()); } #[test] diff --git a/src/status.rs b/src/status.rs index 9075377..811d1ca 100644 --- a/src/status.rs +++ b/src/status.rs @@ -1,3 +1,5 @@ +use serde::Deserialize; + /// Represent the status of a step in a roadmap. /// /// The unknown status allows the user to not specify the status, and @@ -5,7 +7,8 @@ /// example, a step is inferred to be blocked if any of it /// dependencies are not finished. -#[derive(Clone, Copy, Debug, Eq, PartialEq)] +#[derive(Clone, Copy, Debug, Eq, PartialEq, Deserialize)] +#[serde(rename_all = "lowercase")] pub enum Status { Unknown, Goal, @@ -15,6 +18,12 @@ pub enum Status { Blocked, } +impl Default for Status { + fn default() -> Self { + Self::Unknown + } +} + impl Status { pub fn from_text(text: &str) -> Option { match text { diff --git a/src/step.rs b/src/step.rs index 92b8eec..c37994a 100644 --- a/src/step.rs +++ b/src/step.rs @@ -1,4 +1,5 @@ use super::Status; +use serde::Deserialize; use std::fmt; /// A roadmap step. @@ -6,11 +7,16 @@ use std::fmt; /// See the crate documentation for an example. You /// probably don't want to create steps manually, but via the roadmap /// YAML parsing function. -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, PartialEq, Eq, Deserialize)] +#[serde(deny_unknown_fields)] pub struct Step { + #[serde(skip)] name: String, - status: Status, + #[serde(default)] label: String, + #[serde(default)] + status: Status, + #[serde(default)] depends: Vec, } @@ -25,6 +31,11 @@ impl Step { } } + /// Set the name of a step. + pub fn set_name(&mut self, name: &str) { + self.name = name.to_string(); + } + /// Return the name of a step. pub fn name(&self) -> &str { &self.name -- cgit v1.2.1