From bb237c5552a72efe10de8054c72cfc0f5993cf89 Mon Sep 17 00:00:00 2001
From: Lars Wirzenius <liw@liw.fi>
Date: Sat, 28 Sep 2019 11:24:56 +0300
Subject: Refactor: cleanliness, clarity and simplicity, and rustfmt

---
 src/parser.rs  | 28 ++++++++++++++--------------
 src/roadmap.rs |  4 ++--
 src/status.rs  |  6 ++----
 src/step.rs    |  2 +-
 4 files changed, 19 insertions(+), 21 deletions(-)

diff --git a/src/parser.rs b/src/parser.rs
index 48c4c52..cd1ed56 100644
--- a/src/parser.rs
+++ b/src/parser.rs
@@ -3,14 +3,14 @@ use serde_yaml::Value;
 use std::collections::HashMap;
 use std::error::Error;
 
+pub use crate::Roadmap;
 pub use crate::Status;
 pub use crate::Step;
-pub use crate::Roadmap;
 
 /// Result type for roadmap parsing.
 type ParseResult<T> = Result<T, String>;
 
-/// Create a new roadmap from a YAML representation.
+/// Create a new roadmap from a textual YAML representation.
 pub fn from_yaml(yaml: &str) -> Result<Roadmap, Box<dyn Error>> {
     let mut roadmap = Roadmap::new();
     let map: HashMap<String, serde_yaml::Value> = serde_yaml::from_str(yaml)?;
@@ -29,9 +29,9 @@ fn step_from_value(name: &str, value: &Value) -> ParseResult<Step> {
     match value {
         Value::Mapping(_) => {
             let label = parse_label(&value);
-            let mut step = Step::new(name, label);
-
             let status = parse_status(&value)?;
+
+            let mut step = Step::new(name, label);
             step.set_status(status);
 
             for depname in parse_depends(&value).iter() {
@@ -63,22 +63,20 @@ fn parse_depends(map: &Value) -> Vec<&str> {
 
 // Get label string from a Mapping element, or empty string.
 fn parse_label<'a>(map: &'a Value) -> &'a str {
-    
-parse_string("label", map)
+    parse_string("label", map)
 }
 
 // Get status string from a Mapping element. Default to unknown status.
 fn parse_status<'a>(map: &'a Value) -> ParseResult<Status> {
     let text = parse_string("status", map);
-    let status = Status::from_text(text);
-    if let Some(status) = status {
-        Ok(status)
-    } else {
-        Err(format!("unknown status: {:?}", text))
+    match Status::from_text(text) {
+        Some(status) => Ok(status),
+        _ => Err(format!("unknown status: {:?}", text)),
     }
 }
 
-// Get string value from a Mapping element, or empty string.
+// 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) {
@@ -90,8 +88,10 @@ fn parse_string<'a>(key_name: &str, map: &'a Value) -> &'a str {
 // Validate that the parsed, constructed roadmap is valid.
 fn validate(roadmap: &Roadmap) -> ParseResult<()> {
     // Is there exactly one goal?
-    if roadmap.count_goals() != 1 {
-        return Err(format!("must have exactly one goal for roadmap"));
+    match roadmap.count_goals() {
+        0 => return Err(format!("the roadmap doesn't have a goal")),
+        1 => (),
+        _ => return Err(format!("must have exactly one goal for roadmap")),
     }
 
     // Does every dependency exist?
diff --git a/src/roadmap.rs b/src/roadmap.rs
index 257441b..c043e1f 100644
--- a/src/roadmap.rs
+++ b/src/roadmap.rs
@@ -1,8 +1,8 @@
 use textwrap::fill;
 
+pub use crate::from_yaml;
 pub use crate::Status;
 pub use crate::Step;
-pub use crate::from_yaml;
 
 /// All the steps to get to the end goal.
 pub struct Roadmap {
@@ -185,7 +185,7 @@ impl Roadmap {
 
 #[cfg(test)]
 mod tests {
-    use super::{Roadmap, Step, Status, from_yaml};
+    use super::{from_yaml, Roadmap, Status, Step};
 
     #[test]
     fn new_roadmap() {
diff --git a/src/status.rs b/src/status.rs
index 247023c..e6bac17 100644
--- a/src/status.rs
+++ b/src/status.rs
@@ -5,7 +5,7 @@
 /// example, a step is inferred to be blocked if any of it
 /// dependencies are not finished.
 
-#[derive(Clone,Debug,PartialEq)]
+#[derive(Clone, Debug, PartialEq)]
 pub enum Status {
     Unknown,
     Goal,
@@ -41,7 +41,6 @@ impl PartialEq<&Status> for Status {
     }
 }
 
-
 #[cfg(test)]
 mod test {
     use super::Status;
@@ -56,10 +55,9 @@ mod test {
         assert_eq!(Status::from_text("blocked").unwrap(), Status::Blocked);
     }
 
-
     #[test]
     fn sad_from_text() {
         let x = Status::from_text("x");
         assert_eq!(x, None);
-    }    
+    }
 }
diff --git a/src/step.rs b/src/step.rs
index acbb078..e8d9117 100644
--- a/src/step.rs
+++ b/src/step.rs
@@ -64,7 +64,7 @@ impl Step {
 
 #[cfg(test)]
 mod tests {
-    use super::{Step, Status};
+    use super::{Status, Step};
 
     #[test]
     fn new_step() {
-- 
cgit v1.2.1