Better errors messages for file opening (e.g. config file permissions)

src/config.rs

@@ -45,9 +45,9 @@ impl Config {
     pub fn load() -> Result<Self, Error> {
         let mut conf = Self::default();
 
-        let dirs: Vec<_> = cdirs().iter().map(|d| PathBuf::from(d).join("config.ini")).collect();
+        let paths: Vec<_> = cdirs().iter().map(|d| PathBuf::from(d).join("config.ini")).collect();
 
-        for path in dirs.iter().filter(|p| p.exists()).rev() {
+        for path in paths.iter().filter(|p| p.is_file()).rev() {
             process_ini_file(path, &mut |key, value| {
                 match key {
                     "tab_stop" => match parse_value(value)? {
@@ -74,7 +74,8 @@ impl Config {
 /// function will update a configuration instance.
 pub fn process_ini_file<F>(path: &Path, kv_fn: &mut F) -> Result<(), Error>
 where F: FnMut(&str, &str) -> Result<(), String> {
-    for (i, line) in BufReader::new(File::open(path)?).lines().enumerate() {
+    let file = File::open(path).map_err(|e| ConfErr(path.into(), 0, e.to_string()))?;
+    for (i, line) in BufReader::new(file).lines().enumerate() {
         let (i, line) = (i + 1, line?);
         let mut parts = line.trim_start().splitn(2, '=');
         match (parts.next(), parts.next()) {
@@ -180,6 +181,18 @@ mod tests {
         }
     }
 
+    #[test]
+    fn ini_processing_invalid_path() {
+        let kv_fn = &mut |_: &str, _: &str| Ok(());
+        let tmp_dir = TempDir::new().expect("Could not create temporary directory");
+        let tmp_path = tmp_dir.path().join("path_does_not_exist.ini");
+        match process_ini_file(&tmp_path, kv_fn) {
+            Ok(_) => panic!("process_ini_file should return an error"),
+            Err(Error::Config(path, 0, _)) if path == tmp_path => (),
+            Err(e) => panic!("Unexpected error {:?}", e),
+        }
+    }
+
     fn test_config_dir(env_key: &OsStr, env_val: &OsStr, kibi_config_home: &Path) {
         let custom_config = Config { tab_stop: 99, quit_times: 50, ..Config::default() };
         let ini_content = format!(

src/syntax.rs

@@ -67,7 +67,7 @@ impl Conf {
         Ok(None)
     }
     /// Load a `SyntaxConf` from file.
-    fn from_file(path: &Path) -> Result<(Self, Vec<String>), Error> {
+    pub fn from_file(path: &Path) -> Result<(Self, Vec<String>), Error> {
         let (mut sc, mut extensions) = (Self::default(), Vec::new());
         config::process_ini_file(path, &mut |key, val| {
             match key {
@@ -97,6 +97,8 @@ mod tests {
     use std::collections::HashSet;
     use std::fs;
 
+    use tempfile::TempDir;
+
     use super::*;
 
     #[test]
@@ -112,4 +114,15 @@ mod tests {
         assert!(file_count > 0);
         assert_eq!(file_count, syntax_names.len());
     }
+
+    #[test]
+    fn conf_from_invalid_path() {
+        let tmp_dir = TempDir::new().expect("Could not create temporary directory");
+        let tmp_path = tmp_dir.path().join("path_does_not_exist.ini");
+        match Conf::from_file(&tmp_path) {
+            Ok(_) => panic!("Conf::from_file should return an error"),
+            Err(Error::Config(path, 0, _)) if path == tmp_path => (),
+            Err(e) => panic!("Unexpected error {:?}", e),
+        }
+    }
 }