Refactor and improve code quality.

* Part of issue #9
* Todo: extensive tests and unit tests.

Removed unit tests as they were slowing down the refactor
but will need to be reimplemented later.

src/main.rs

@@ -1,8 +1,8 @@
-use std::path::{PathBuf, Path};
-use std::{fs, vec::Vec};
+use std::path::{Path, PathBuf};
+use std::{error, fs, vec::Vec};
 
-use comrak::{ComrakOptions, Arena};
-use comrak::nodes::{NodeValue};
+use comrak::nodes::NodeValue;
+use comrak::{Arena, ComrakOptions};
 use structopt::StructOpt;
 
 #[derive(Debug, StructOpt)]
@@ -17,9 +17,32 @@ struct Opt {
     output: PathBuf,
 }
 
+#[derive(Debug)]
+struct GenericError {
+    message: String,
+}
+
+impl GenericError {
+    fn new(error: String) -> GenericError {
+        GenericError { message: error }
+    }
+}
+
+impl std::fmt::Display for GenericError {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        write!(f, "[error] {}", self.message)
+    }
+}
+
+impl error::Error for GenericError {
+    fn description(&self) -> &str {
+        self.message.as_str()
+    }
+}
+
 struct FileData {
     html_content: String,
-    title: String
+    title: String,
 }
 
 fn list_markdown_files(path: &Path) -> Vec<PathBuf> {
@@ -43,17 +66,17 @@ fn list_markdown_files(path: &Path) -> Vec<PathBuf> {
                                     if extension == "md" {
                                         files.push(entry.path());
                                     }
-                                },
+                                }
                                 None => {}
                             }
                         }
-                    },
+                    }
                     Err(_) => {
                         println!("Invalid entry found.");
                     }
                 }
             }
-        },
+        }
         Err(err) => {
             println!("Error while opening directory: {}", err.to_string());
         }
@@ -69,32 +92,31 @@ fn md_to_file_data(file: &Path) -> Result<FileData, String> {
     let ast_root = comrak::parse_document(&arena, file_content.as_str(), &ComrakOptions::default());
 
     // Page title is the first level 1 heading we find.
-    let page_title_node = ast_root.children().find(|item| {
-        match item.data.borrow().value {
+    let page_title_node = ast_root
+        .children()
+        .find(|item| match item.data.borrow().value {
             NodeValue::Heading(ref n) => n.level == 1,
-            _ => false
-        }
-    });
+            _ => false,
+        });
 
     let mut page_title = String::new();
 
     match page_title_node {
-        Some(node) => {
-            match node.first_child() {
-                Some(child) => {
-                    match child.data.borrow().value {
-                        NodeValue::Text(ref utf8_text) => {
-                            page_title = std::str::from_utf8(&utf8_text).unwrap_or("").to_owned();
-                        },
-                        _ => println!("[error] Couldn't extract title from file '{}'.", file.to_str().unwrap())
-                    }
-                },
-                None => println!("[warn] Could not find title (empty?).")
-            }
+        Some(node) => match node.first_child() {
+            Some(child) => match child.data.borrow().value {
+                NodeValue::Text(ref utf8_text) => {
+                    page_title = std::str::from_utf8(&utf8_text).unwrap_or("").to_owned();
+                }
+                _ => println!(
+                    "[error] Couldn't extract title from file '{}'.",
+                    file.to_str().unwrap()
+                ),
+            },
+            None => println!("[warn] Could not find title (empty?)."),
         },
         None => {
             println!("[warn] Could not find title for file '{}'. Consider adding a header level 1: `# My title` at the beginning of your page.", file.to_str().unwrap());
-         }
+        }
     }
 
     let mut output = vec![];
@@ -104,12 +126,12 @@ fn md_to_file_data(file: &Path) -> Result<FileData, String> {
 
     Ok(FileData {
         html_content: String::from_utf8(output).unwrap(),
-        title: page_title
+        title: page_title,
     })
 }
 
 // Create the folders path (equivalent to mkdir -p <path>)
-// If path has a file (has extension), it will ignore it.
+// file is expected to have a filename to it.
 fn create_output_file_path(file: &Path) -> Result<(), Box<dyn std::error::Error + 'static>> {
     let mut path = file.to_path_buf();
     path.pop();
@@ -118,135 +140,173 @@ fn create_output_file_path(file: &Path) -> Result<(), Box<dyn std::error::Error
     Ok(())
 }
 
-// From a given full input path and a destination output directory,
-// obtains the resulting full output path.
-fn get_dest_html_file_path(opt: &Opt, file: &PathBuf) -> Result<PathBuf, Box<dyn std::error::Error>> {
-    if file.is_absolute() {
-        let mut file_stripped = file
-            .strip_prefix(&opt.input)
-            .unwrap().to_owned();
-        let filename = format!("{}.html", file_stripped.file_stem().unwrap().to_str().unwrap());
-        file_stripped.pop();
-
-        Ok(opt.output.join(file_stripped).join(filename))
-    } else {
-        let mut file_stripped = file
-            .canonicalize()?
-            .strip_prefix(&opt.input.canonicalize()?)
-            .unwrap().to_owned();
-        let filename = format!("{}.html", file_stripped.file_stem().unwrap().to_str().unwrap());
-        file_stripped.pop();
-
-        Ok(opt.output.join(file_stripped).join(filename))
+// Expects all input paths to be absolutes (input directory, output directory, file)
+fn destination_for_file(
+    parameters: &Opt,
+    file: &PathBuf,
+) -> Result<PathBuf, Box<dyn std::error::Error + 'static>> {
+    assert!(parameters.input.is_absolute());
+    assert!(parameters.output.is_absolute());
+    assert!(file.is_absolute());
+
+    Ok(parameters
+        .output
+        .join(file.strip_prefix(&parameters.input)?))
+}
+
+fn read_file_string(file: &PathBuf) -> Result<String, String> {
+    let path = Path::new(&file);
+
+    if path.exists() {
+        match fs::read_to_string(path) {
+            Ok(content) => return Ok(content),
+            Err(error) => {
+                let error = format!(
+                    "[error] Could not read file '{}'. Error: {}",
+                    file.to_str().unwrap(),
+                    error.to_string()
+                );
+                println!("{}", &error);
+                return Err(error);
+            }
+        }
     }
+
+    let error = format!("[warn] Couldn't find file '{}'", &file.to_str().unwrap());
+    println!("{}", &error);
+    Err(error)
 }
 
-fn get_dest_file_path(opt: &Opt, file: &PathBuf) -> Result<PathBuf, Box<dyn std::error::Error>> {
-    if file.is_absolute() {
-        let file_stripped = file
-            .strip_prefix(&opt.input)
-            .unwrap()
-            .to_path_buf();
-
-        Ok(opt.output.join(file_stripped))
-    } else {
-        let file_stripped = file.canonicalize()?
-        .strip_prefix(&opt.input.canonicalize()?)
-        .unwrap()
-        .to_path_buf();
-
-        Ok(opt.output.join(file_stripped))
+fn assemble_file(file_data: &FileData, header: &String, footer: &String, destination: &PathBuf) {
+    let assembled_content = format!(
+        "{}{}{}",
+        header.replace("{title}", &file_data.title),
+        file_data.html_content,
+        footer
+    );
+
+    if let Err(error) = fs::write(Path::new(&destination), assembled_content) {
+        println!(
+            "[error] Couldn't not write to file '{}'. Error: {}",
+            destination.to_str().unwrap(),
+            error.to_string()
+        );
     }
 }
 
-fn main() -> Result<(), Box<dyn std::error::Error + 'static>> {
-    let opt: Opt = Opt::from_args();
+// 1. Validates the input directory exists and is not a file.
+// 2. Creates the base output directory.
+// 3. Converts the input and output directory to absolute paths.
+fn normalize_program_arguments(parameters: &Opt) -> Result<Opt, GenericError> {
+    if !parameters.input.exists() {
+        return Err(GenericError::new(
+            "Input directory was not found.".to_owned(),
+        ));
+    }
 
-    let files = list_markdown_files(Path::new(&opt.input));
+    if !parameters.output.exists() {
+        if let Err(error) = fs::create_dir_all(&parameters.output) {
+            return Err(GenericError::new(format!(
+                "Could not create output directory. Error: {}",
+                error.to_string()
+            )));
+        }
+    }
 
-    let header_path = format!("{}/header.html", opt.input.to_str().unwrap_or("."));
-    let header_path = Path::new(&header_path);
-    let footer_path = format!("{}/footer.html", opt.input.to_str().unwrap_or("."));
-    let footer_path = Path::new(&footer_path);
+    let mut new_parameters = Opt {
+        input: parameters.input.to_path_buf(),
+        output: parameters.output.to_path_buf(),
+    };
 
-    if !header_path.exists() {
-        println!("[warn] No header.html found.");
+    match parameters.input.canonicalize() {
+        Ok(path) => {
+            new_parameters.input = path;
+        }
+        Err(error) => {
+            return Err(GenericError::new(format!(
+                "Could not resolve path for input directory '{}'. Error: {}",
+                parameters.input.to_str().unwrap_or_default(),
+                error.to_string()
+            )));
+        }
     }
 
-    if !footer_path.exists() {
-        println!("[warn] No footer.html found.");
+    match parameters.output.canonicalize() {
+        Ok(path) => {
+            new_parameters.output = path;
+        }
+        Err(error) => {
+            return Err(GenericError::new(format!(
+                "Could not resolve path for input directory '{}'. Error: {}",
+                parameters.output.to_str().unwrap_or_default(),
+                error.to_string()
+            )));
+        }
     }
 
-    let header_content = fs::read_to_string(header_path).unwrap_or("<html><head><title>{title}</title><body>".to_owned());
-    let footer_content = fs::read_to_string(footer_path).unwrap_or("</body></html>".to_owned());
+    Ok(new_parameters)
+}
 
-    for file in files {
-        println!("[info] Processing file {}", file.to_str().unwrap());
-        let dest_path = get_dest_html_file_path(&opt, &file)?;
-        let _ = create_output_file_path(&dest_path)?;
+fn main() -> Result<(), Box<dyn error::Error + 'static>> {
+    let arguments = normalize_program_arguments(&Opt::from_args())?;
 
-        let file_data = md_to_file_data(&file)?;
+    let files = list_markdown_files(Path::new(&arguments.input));
 
-        let assembled_content = format!("{}{}{}",
-            header_content.replace("{title}", &file_data.title),
-            file_data.html_content, footer_content);
+    let mut header_path = PathBuf::new();
+    header_path.push("header.html");
 
-        if fs::write(Path::new(&dest_path), assembled_content).is_err() {
-            println!("[error] Couldn't not write to file: {}", dest_path.to_str().unwrap());
-        }
-    }
+    let header_content = read_file_string(&header_path)
+    .unwrap_or("<html><head><title>{title}</title><body>".to_owned());
 
-    let assets_path = format!("{}/assets.config", opt.input.to_str().unwrap_or("."));
-    let assets_path = Path::new(&assets_path);
+    let mut footer_path = PathBuf::new();
+    footer_path.push("footer.html");
 
-    if assets_path.exists() {
-        if assets_path.is_file() {
-            let assets_list: Vec<String> = fs::read_to_string(assets_path)?
-                .split("\n")
-                .map(|line| line.trim().to_owned())
-                .collect();
+    let footer_content = read_file_string(&footer_path)
+    .unwrap_or("</body></html>".to_owned());
 
-            println!("[info] Copying {} assets...", assets_list.len());
+    for file in files {
+        println!("[info] Processing file {}", file.to_str().unwrap());
 
-            for asset in assets_list {
-                let asset_path = Path::new(&asset);
+        let mut destination = destination_for_file(&arguments, &file)?;
+        destination.set_extension("html");
 
-                if asset_path.exists() {
-                    let asset_dest = get_dest_file_path(&opt, &asset_path.to_path_buf())?;
-                    let _ = create_output_file_path(asset_path);
+        let file_data = md_to_file_data(&file)?;
 
-                    match fs::copy(&asset_path, &asset_dest) {
-                        Ok(_) => {},
-                        Err(e) => println!("[error] Couldn't copy file '{}' to '{}'. Error: {}", asset, asset_dest.to_str().unwrap(), e.to_string())
-                    }
-                } else {
-                    println!("[warn] Couldn't find asset '{}'. Skipping.", &asset);
-                }
-            }
-        } else {
-            println!("[warn] A folder named assets.config has been found but is not an asset descriptor file. Skipping.");
-        }
+        create_output_file_path(&destination)?;
+        assemble_file(&file_data, &header_content, &footer_content, &destination);
     }
 
-    Ok(())
-}
+    let mut path = PathBuf::new();
+    path.push(&arguments.input);
+    path.push("assets.config");
+
+    let assets: Vec<PathBuf> = read_file_string(&path)
+    .unwrap_or("".to_owned())
+    .split("\n")
+    .skip_while(|e| e == &"")
+    .map(|line| {
+        let buf = Path::new(line.trim()).to_path_buf();
+        buf.canonicalize().unwrap_or(buf)
+    })
+    .collect();
 
-#[cfg(test)]
-mod tests {
-    use std::path::{Path};
-    use crate::{Opt, list_markdown_files};
+    println!("[info] Copying {} assets...", assets.len());
 
-    #[test]
-    fn list_files_recursively() {
-        let files_list = list_markdown_files(Path::new("./src/tests"));
+    for asset in &assets {
+        let destination = destination_for_file(&arguments, &asset)?;
 
-        assert_eq!(files_list.len(), 2);
-    }
+        println!("[info] Copying '{}'\n \tto '{}'.", asset.to_str().unwrap(), destination.to_str().unwrap());
 
-    #[test]
-    fn on_nonexistant_dir_empty_list() {
-        let files_list = list_markdown_files(Path::new("/usr/fake/path"));
+        create_output_file_path(&destination)?;
 
-        assert_eq!(files_list.len(), 0);
+        let _ = fs::copy(&asset, &destination)
+        .map_err(|error| {
+            println!(
+                "[error] Could not copy asset '{}'. Error: ",
+                error.to_string()
+            );
+        });
     }
+
+    Ok(())
 }