Add error context to parser.

src/filesystem.rs

@@ -1,17 +1,23 @@
 use crate::structures::option::Config;
 use anyhow::Context;
 use anyhow::Error;
+use core::fmt::Display;
 use std::fs;
 use std::fs::File;
-use std::io::{self, BufRead, BufReader, Lines};
+use std::io::{self, BufRead};
 use std::path::{Path, PathBuf};
 
-pub fn read_lines<P>(filename: P) -> io::Result<Lines<BufReader<File>>>
+pub fn read_lines<P>(filename: P) -> Result<Vec<String>, Error>
 where
-    P: AsRef<Path>,
+    P: AsRef<Path> + Display,
 {
+    let error_string = format!("Failed to read lines from {}", filename);
     let file = File::open(filename)?;
-    Ok(io::BufReader::new(file).lines())
+    io::BufReader::new(file)
+        .lines()
+        .map(|line| line.map_err(Error::from))
+        .collect::<Result<Vec<String>, Error>>()
+        .with_context(|| error_string)
 }
 
 pub fn pathbuf_to_string(pathbuf: PathBuf) -> Result<String, Error> {

src/parser.rs

@@ -5,7 +5,7 @@ use crate::structures::fnv::HashLine;
 use crate::structures::fzf::{Opts as FzfOpts, SuggestionType};
 use crate::structures::option::Config;
 use crate::welcome;
-use anyhow::Error;
+use anyhow::{Context, Error};
 use regex::Regex;
 use std::collections::HashSet;
 use std::fs;
@@ -16,31 +16,64 @@ lazy_static! {
         Regex::new(r"^\$\s*([^:]+):(.*)").expect("Invalid regex");
 }
 
-fn parse_opts(text: &str) -> FzfOpts {
+fn parse_opts(text: &str) -> Result<FzfOpts, Error> {
     let mut multi = false;
     let mut prevent_extra = false;
     let mut opts = FzfOpts::default();
-    let parts_vec = shellwords::split(text).unwrap();
+    let parts = shellwords::split(text)
-    let mut parts = parts_vec.into_iter();
+        .map_err(|_| anyhow!("Given options are missing a closing quote"))?;
 
-    while let Some(p) = parts.next() {
+    parts
-        match p.as_str() {
+        .into_iter()
-            "--multi" => multi = true,
+        .filter(|part| {
-            "--prevent-extra" => prevent_extra = true,
+            // We'll take parts in pairs of 2: (argument, value). Flags don't have a value tho, so we filter and handle them beforehand.
-            "--headers" | "--header-lines" => {
+            match part.as_str() {
-                opts.header_lines = parts.next().unwrap().parse::<u8>().unwrap()
+                "--multi" => {
+                    multi = true;
+                    false
+                }
+                "--prevent-extra" => {
+                    prevent_extra = true;
+                    false
+                }
+                _ => true,
             }
-            "--column" => opts.column = Some(parts.next().unwrap().parse::<u8>().unwrap()),
+        })
-            "--delimiter" => opts.delimiter = Some(parts.next().unwrap().to_string()),
+        .collect::<Vec<_>>()
-            "--query" => opts.query = Some(parts.next().unwrap().to_string()),
+        .chunks(2)
-            "--filter" => opts.filter = Some(parts.next().unwrap().to_string()),
+        .map(|flag_and_value| {
-            "--preview" => opts.preview = Some(parts.next().unwrap().to_string()),
+            if let [flag, value] = flag_and_value {
-            "--preview-window" => opts.preview_window = Some(parts.next().unwrap().to_string()),
+                match flag.as_str() {
-            "--header" => opts.header = Some(parts.next().unwrap().to_string()),
+                    "--headers" | "--header-lines" => {
-            "--overrides" => opts.overrides = Some(parts.next().unwrap().to_string()),
+                        opts.header_lines = value
-            _ => (),
+                            .parse::<u8>()
-        }
+                            .context("Value for `--headers` is invalid u8")?
-    }
+                    }
+                    "--column" => {
+                        opts.column = Some(
+                            value
+                                .parse::<u8>()
+                                .context("Value for `--column` is invalid u8")?,
+                        )
+                    }
+                    "--delimiter" => opts.delimiter = Some(value.to_string()),
+                    "--query" => opts.query = Some(value.to_string()),
+                    "--filter" => opts.filter = Some(value.to_string()),
+                    "--preview" => opts.preview = Some(value.to_string()),
+                    "--preview-window" => opts.preview_window = Some(value.to_string()),
+                    "--header" => opts.header = Some(value.to_string()),
+                    "--overrides" => opts.overrides = Some(value.to_string()),
+                    _ => (),
+                }
+                Ok(())
+            } else if let [flag] = flag_and_value {
+                Err(anyhow!("No value provided for the flag {}", flag))
+            } else {
+                unreachable!() // Chunking by 2 allows only for tuples of 1 or 2 items...
+            }
+        })
+        .collect::<Result<_, _>>()
+        .context("Failed to parse fzf options")?;
 
     let suggestion_type = match (multi, prevent_extra) {
         (true, _) => SuggestionType::MultipleSelections, // multi wins over allow-extra
@@ -49,16 +82,31 @@ fn parse_opts(text: &str) -> FzfOpts {
     };
     opts.suggestion_type = suggestion_type;
 
-    opts
+    Ok(opts)
 }
 
-fn parse_variable_line(line: &str) -> (&str, &str, Option<FzfOpts>) {
+fn parse_variable_line(line: &str) -> Result<(&str, &str, Option<FzfOpts>), Error> {
-    let caps = VAR_LINE_REGEX.captures(line).unwrap();
+    let caps = VAR_LINE_REGEX.captures(line).ok_or_else(|| {
-    let variable = caps.get(1).unwrap().as_str().trim();
+        anyhow!(
-    let mut command_plus_opts = caps.get(2).unwrap().as_str().split("---");
+            "No variables, command, and options found in the line {}",
-    let command = command_plus_opts.next().unwrap();
+            line
-    let command_options = command_plus_opts.next().map(parse_opts);
+        )
-    (variable, command, command_options)
+    })?;
+    let variable = caps
+        .get(1)
+        .ok_or_else(|| anyhow!("No variable captured in the line {}", line))?
+        .as_str()
+        .trim();
+    let mut command_plus_opts = caps
+        .get(2)
+        .ok_or_else(|| anyhow!("No command and options captured in the line {}", line))?
+        .as_str()
+        .split("---");
+    let command = command_plus_opts
+        .next()
+        .ok_or_else(|| anyhow!("No command captured in the line {}", line))?;
+    let command_options = command_plus_opts.next().map(parse_opts).transpose()?;
+    Ok((variable, command, command_options))
 }
 
 fn write_cmd(
@@ -68,16 +116,16 @@ fn write_cmd(
     tag_width: usize,
     comment_width: usize,
     stdin: &mut std::process::ChildStdin,
-) -> bool {
+) -> Result<(), Error> {
     if snippet.is_empty() {
-        true
+        Ok(())
     } else {
         stdin
             .write_all(
                 display::format_line(&tags, &comment, &snippet, tag_width, comment_width)
                     .as_bytes(),
             )
-            .is_ok()
+            .context("Failed to write command to fzf's stdin")
     }
 }
 
@@ -86,7 +134,7 @@ fn read_file(
     variables: &mut VariableMap,
     visited_lines: &mut HashSet<u64>,
     stdin: &mut std::process::ChildStdin,
-) -> bool {
+) -> Result<(), Error> {
     let mut tags = String::from("");
     let mut comment = String::from("");
     let mut snippet = String::from("");
@@ -94,71 +142,71 @@ fn read_file(
 
     let (tag_width, comment_width) = *display::WIDTHS;
 
-    if let Ok(lines) = filesystem::read_lines(path) {
+    for (line_nr, line) in filesystem::read_lines(path)?.into_iter().enumerate() {
-        for l in lines {
+        if should_break {
-            if should_break {
+            break;
-                break;
-            }
-
-            let line = l.unwrap();
-
-            // duplicate
-            if !tags.is_empty() && !comment.is_empty() {}
-
-            // blank
-            if line.is_empty() {
-            }
-            // tag
-            else if line.starts_with('%') {
-                if !write_cmd(&tags, &comment, &snippet, tag_width, comment_width, stdin) {
-                    should_break = true
-                }
-                snippet = String::from("");
-                tags = String::from(&line[2..]);
-            }
-            // metacomment
-            else if line.starts_with(';') {
-            }
-            // comment
-            else if line.starts_with('#') {
-                if !write_cmd(&tags, &comment, &snippet, tag_width, comment_width, stdin) {
-                    should_break = true
-                }
-                snippet = String::from("");
-                comment = String::from(&line[2..]);
-            }
-            // variable
-            else if line.starts_with('$') {
-                if !write_cmd(&tags, &comment, &snippet, tag_width, comment_width, stdin) {
-                    should_break = true
-                }
-                snippet = String::from("");
-                let (variable, command, opts) = parse_variable_line(&line);
-                variables.insert(&tags, &variable, (String::from(command), opts));
-            }
-            // snippet
-            else {
-                let hash = format!("{}{}", &comment, &line).hash_line();
-                if visited_lines.contains(&hash) {
-                    continue;
-                }
-                visited_lines.insert(hash);
-
-                if !(&snippet).is_empty() {
-                    snippet.push_str(display::LINE_SEPARATOR);
-                }
-                snippet.push_str(&line);
-            }
         }
 
-        if !should_break {
+        // duplicate
-            write_cmd(&tags, &comment, &snippet, tag_width, comment_width, stdin);
+        if !tags.is_empty() && !comment.is_empty() {}
-        }
 
-        return true;
+        // blank
+        if line.is_empty() {
+        }
+        // tag
+        else if line.starts_with('%') {
+            if write_cmd(&tags, &comment, &snippet, tag_width, comment_width, stdin).is_err() {
+                should_break = true
+            }
+            snippet = String::from("");
+            tags = String::from(&line[2..]);
+        }
+        // metacomment
+        else if line.starts_with(';') {
+        }
+        // comment
+        else if line.starts_with('#') {
+            if write_cmd(&tags, &comment, &snippet, tag_width, comment_width, stdin).is_err() {
+                should_break = true
+            }
+            snippet = String::from("");
+            comment = String::from(&line[2..]);
+        }
+        // variable
+        else if line.starts_with('$') {
+            if write_cmd(&tags, &comment, &snippet, tag_width, comment_width, stdin).is_err() {
+                should_break = true
+            }
+            snippet = String::from("");
+            let (variable, command, opts) = parse_variable_line(&line).with_context(|| {
+                format!(
+                    "Failed to parse variable line. See line nr.{} in cheatsheet {}",
+                    line_nr + 1,
+                    path
+                )
+            })?;
+            variables.insert(&tags, &variable, (String::from(command), opts));
+        }
+        // snippet
+        else {
+            let hash = format!("{}{}", &comment, &line).hash_line();
+            if visited_lines.contains(&hash) {
+                continue;
+            }
+            visited_lines.insert(hash);
+
+            if !(&snippet).is_empty() {
+                snippet.push_str(display::LINE_SEPARATOR);
+            }
+            snippet.push_str(&line);
+        }
     }
 
-    false
+    if !should_break {
+        let _ = write_cmd(&tags, &comment, &snippet, tag_width, comment_width, stdin);
+    }
+
+    Ok(())
 }
 
 pub fn read_all(
@@ -172,16 +220,21 @@ pub fn read_all(
     let folders = paths.split(':');
 
     for folder in folders {
-        if let Ok(paths) = fs::read_dir(folder) {
+        let dir_entries =
-            for path in paths {
+            fs::read_dir(folder).with_context(|| format!("Unable to read directory {}", folder))?;
-                let path = path.unwrap().path();
+
-                let path_str = path.to_str().unwrap();
+        for entry in dir_entries {
-                if path_str.ends_with(".cheat")
+            let path = entry
-                    && read_file(path_str, &mut variables, &mut visited_lines, stdin)
+                .with_context(|| format!("Unable to read directory {}", folder))?
-                    && !found_something
+                .path();
-                {
+            let path_str = path
-                    found_something = true;
+                .to_str()
-                }
+                .ok_or_else(|| anyhow!("Invalid path {}", path.display()))?;
+            if path_str.ends_with(".cheat")
+                && read_file(path_str, &mut variables, &mut visited_lines, stdin).is_ok()
+                && !found_something
+            {
+                found_something = true;
             }
         }
     }
@@ -200,7 +253,7 @@ mod tests {
     #[test]
     fn test_parse_variable_line() {
         let (variable, command, command_options) =
-            parse_variable_line("$ user : echo -e \"$(whoami)\\nroot\" --- --allow-extra");
+            parse_variable_line("$ user : echo -e \"$(whoami)\\nroot\" --- --allow-extra").unwrap();
         assert_eq!(command, " echo -e \"$(whoami)\\nroot\" ");
         assert_eq!(variable, "user");
         assert_eq!(
@@ -223,7 +276,7 @@ mod tests {
         let mut child = Command::new("cat").stdin(Stdio::piped()).spawn().unwrap();
         let child_stdin = child.stdin.as_mut().unwrap();
         let mut visited_lines: HashSet<u64> = HashSet::new();
-        read_file(path, &mut variables, &mut visited_lines, child_stdin);
+        read_file(path, &mut variables, &mut visited_lines, child_stdin).unwrap();
         let expected_suggestion = (
             r#" echo -e "$(whoami)\nroot" "#.to_string(),
             Some(FzfOpts {