Fix xyz parser (#44), better error messages (#43) (#45)

* Remove unused imports

* change if else to match in molecule parser

* make find_config a struct method, add log for writing inputs

* Improve errors

src/config.rs

@@ -49,7 +49,6 @@ impl Config {
         Ok(())
     }
 
-    // check how to parse params
     pub fn write(&self) -> Result<(), Error> {
         let cfg_path = Config::get_path()?;
         std::fs::write(&cfg_path, toml::to_string(self)?)?;
@@ -105,6 +104,7 @@ impl Config {
             key, value, toml_type
         );
 
+        // TODO: add array and table as well
         match toml_type {
             ArgType::Int => {
                 self.parameters
@@ -128,12 +128,27 @@ impl Config {
 
     pub fn get_path() -> Result<PathBuf, Error> {
         let current_dir = std::env::current_dir()?;
-        let cfg_path: PathBuf = [find_config(current_dir)?, PathBuf::from(CONFIG_NAME)]
+        let cfg_path: PathBuf = [Config::find(current_dir)?, PathBuf::from(CONFIG_NAME)]
             .iter()
             .collect();
         Ok(cfg_path)
     }
 
+    fn find(dir: PathBuf) -> Result<PathBuf, Error> {
+        let cwd = dir.clone();
+        let gedent_config: PathBuf = [dir.clone(), PathBuf::from(CONFIG_NAME)].iter().collect();
+
+        if std::path::Path::try_exists(&gedent_config)? {
+            Ok(cwd)
+        } else {
+            let parent_folder = dir.parent();
+            match parent_folder {
+                Some(parent) => Config::find(parent.to_path_buf()),
+                None => crate::get_gedent_home(),
+            }
+        }
+    }
+
     #[cfg(test)]
     fn new() -> Config {
         Config {
@@ -145,22 +160,6 @@ impl Config {
     }
 }
 
-// git-like search, stop if gedent.toml is found or if dir.parent = none
-fn find_config(dir: PathBuf) -> Result<PathBuf, Error> {
-    let cwd = dir.clone();
-    let gedent_config: PathBuf = [dir.clone(), PathBuf::from(CONFIG_NAME)].iter().collect();
-
-    if std::path::Path::try_exists(&gedent_config)? {
-        Ok(cwd)
-    } else {
-        let parent_folder = dir.parent();
-        match parent_folder {
-            Some(parent) => find_config(parent.to_path_buf()),
-            None => crate::get_gedent_home(),
-        }
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;

src/main.rs

@@ -1,15 +1,11 @@
-#![allow(dead_code, unused_variables, unused_imports)]
 use crate::config::Config;
 use crate::molecule::Molecule;
 use crate::template::Template;
 use anyhow::{anyhow, Context, Error, Result};
 use clap::{Parser, Subcommand};
 use dialoguer::{theme::ColorfulTheme, FuzzySelect};
-use dirs::config_dir;
-use serde::Deserialize;
-use std::fs::{copy, read_dir, read_to_string, write, File};
-use std::path::{Path, PathBuf};
-use tera::Tera;
+use std::fs::{copy, read_dir, write};
+use std::path::PathBuf;
 
 mod config;
 mod molecule;
@@ -26,6 +22,7 @@ struct Input {
 
 impl Input {
     fn write(self) -> Result<(), Error> {
+        println!("Writing {:?}", &self.filename);
         write(&self.filename, &self.content).context(anyhow!("Failed to save input."))
     }
 }
@@ -261,8 +258,18 @@ fn main() -> Result<()> {
 
 //Search for paths
 fn get_gedent_home() -> Result<PathBuf, Error> {
-    let mut config_dir = config_dir().ok_or(anyhow!("Cant retrieve gedent home"))?;
+    let mut config_dir =
+        dirs::config_dir().ok_or(anyhow!("Cant retrieve system config directory."))?;
     config_dir.push("gedent");
+    match config_dir.try_exists() {
+        Ok(exists) => {
+            match exists {
+                true => (),
+                false => anyhow::bail!(format!("Failed to retrieve gedent home, {:?} doesn't exist. \nCheck if you've finished the installation procces and create the config directory.", config_dir)), 
+            }
+        },
+        Err(err) => anyhow::bail!(format!("Failed to retrieve gedent home, caused by {:?}", err)), 
+    }
     Ok(config_dir)
 }
 
@@ -283,10 +290,7 @@ fn select_template() -> Result<String, Error> {
     let gedent_home: PathBuf = [get_gedent_home()?, Into::into(TEMPLATES_DIR)]
         .iter()
         .collect();
-    let gedent_home_len = gedent_home
-        .to_str()
-        .ok_or(anyhow!("Cant retrieve gedent home len"))?
-        .len();
+    let gedent_home_len = gedent_home.to_string_lossy().len();
     let templates = Template::get_templates(gedent_home, gedent_home_len, vec![])?;
     let selection = FuzzySelect::with_theme(&ColorfulTheme::default())
         .default(0)
@@ -341,6 +345,8 @@ fn generate_input(
         context.insert(key, &value);
     }
 
+    // todo: pass option, if none send just solvation
+    // if Some() pass solvent as well
     if let Some(solvent) = solvent {
         context.insert("solvation", &true);
         context.insert("solvent", &solvent);

src/molecule.rs

@@ -1,4 +1,4 @@
-use anyhow::{anyhow, Error, Result};
+use anyhow::{Context, Error, Result};
 use serde::{Deserialize, Serialize};
 use std::path::PathBuf;
 
@@ -20,7 +20,9 @@ impl Molecule {
 
     pub fn split(&self, index: usize) -> Result<(Molecule, Molecule), Error> {
         if index >= self.atoms.len() {
-            anyhow::bail!("Index given bigger than size of molecule, exiting...")
+            anyhow::bail!(
+                "Can't split molecule at given index. \nIndex is bigger than size of molecule."
+            )
         }
         let mut molecule1 = self.clone();
         let mut molecule2 = self.clone();
@@ -36,13 +38,10 @@ impl Molecule {
     // returns a vec because we support a file with multiple xyz
     // the check for atom length got kinda ugly.. see if there is some smarter way to do this
     pub fn from_xyz(mut xyz_path: PathBuf) -> Result<Vec<Molecule>, Error> {
-        let xyz_file = std::fs::read_to_string(&xyz_path)?;
+        let xyz_file = std::fs::read_to_string(&xyz_path)
+            .context(format!("Failed to read xyz file {:?}", xyz_path))?;
         xyz_path.set_extension("");
-        let name = String::from(
-            xyz_path
-                .to_str()
-                .ok_or(anyhow!("Cant convert path of xyz file to name"))?,
-        );
+        let name = xyz_path.to_string_lossy().to_string();
         let mut xyz_lines = xyz_file.lines().peekable();
         let mut molecules: Vec<Molecule> = vec![];
         let mut mol = Molecule::new();
@@ -51,44 +50,49 @@ impl Molecule {
         let mut counter = 0;
 
         loop {
-            if xyz_lines.peek().is_none() {
-                if mol.atoms.len() != natoms {
-                    anyhow::bail!(
-                        "Expected {} atoms found {}, exiting...",
-                        natoms,
-                        mol.atoms.len()
-                    )
-                }
-                match counter {
-                    0 => (),
-                    _ => {
-                        mol.filename = [name.clone(), counter.clone().to_string()].join("_");
-                    }
-                };
-                molecules.push(mol.clone());
-                break;
-            }
-
-            if xyz_lines.peek().unwrap().parse::<i64>().is_ok() {
-                if !mol.atoms.is_empty() {
+            match xyz_lines.next() {
+                None => {
                     if mol.atoms.len() != natoms {
                         anyhow::bail!(
-                            "Expected {} atoms found {}, exiting...",
+                            "Error parsing xyz files {:?}. \nExpected {} atoms found {}, exiting...",
+                            xyz_path,
                             natoms,
                             mol.atoms.len()
                         )
                     }
-                    natoms -= natoms; // set to 0 again
-                    mol.filename = [name.clone(), counter.clone().to_string()].join("_");
+                    match counter {
+                        0 => (),
+                        _ => {
+                            mol.filename = [name.clone(), counter.clone().to_string()].join("_");
+                        }
+                    };
                     molecules.push(mol.clone());
-                    counter += 1;
+                    break;
+                }
+                Some(line) => {
+                    if let Ok(n) = line.trim().parse::<usize>() {
+                        if !mol.atoms.is_empty() {
+                            if mol.atoms.len() != natoms {
+                                anyhow::bail!(
+                                    "Error parsing xyz files {:?}. \nExpected {} atoms found {}, exiting...",
+                                    xyz_path,
+                                    natoms,
+                                    mol.atoms.len()
+                                )
+                            }
+                            natoms -= natoms; // set to 0 again
+                            mol.filename = [name.clone(), counter.clone().to_string()].join("_");
+                            molecules.push(mol.clone());
+                            counter += 1;
+                        }
+
+                        natoms += n;
+                        mol.description = xyz_lines.next().unwrap_or("").to_string();
+                        mol.atoms.clear();
+                    } else {
+                        mol.atoms.push(line.to_string());
+                    }
                 }
-
-                natoms += xyz_lines.next().unwrap().parse::<usize>()?;
-                mol.description = xyz_lines.next().unwrap_or("").to_string();
-                mol.atoms.clear();
-            } else {
-                mol.atoms.push(xyz_lines.next().unwrap().to_string());
             }
         }
 

src/template.rs

@@ -1,5 +1,4 @@
 use crate::get_gedent_home;
-use crate::Config;
 use crate::Molecule;
 use anyhow::{anyhow, Context, Error, Result};
 use serde::Deserialize;
@@ -155,10 +154,13 @@ impl Template {
         ]
         .iter()
         .collect();
-        Ok(template_path)
+        match template_path.try_exists()? {
+            true => Ok(template_path),
+            false => anyhow::bail!(format!("Cant find template {:?}.", template_path)),
+        }
     }
 
-    // #[cfg(test)]
+    #[cfg(test)]
     fn new() -> Template {
         Template {
             name: "".to_string(),
@@ -251,7 +253,7 @@ pub fn split_molecule(args: &HashMap<String, Value>) -> Result<Value, tera::Erro
 #[cfg(test)]
 mod tests {
     use super::*;
-    use toml::{map::Map, Value};
+    use toml::Value;
 
     #[test]
     fn render_template_works() {