Merge pull request #57 from Svenskunganka/toml

Change config file format from YAML to TOML

docs/en/docs/options.md

@@ -33,13 +33,13 @@ struct TemplateStruct {
 
 ## Configuration file
 
-Sailfish allows global and local configuration in a file named `sailfish.yml`. Sailfish looks for this file in same directory as `Cargo.toml` and all parent directories.
+Sailfish allows global and local configuration in a file named `sailfish.toml`. Sailfish looks for this file in same directory as `Cargo.toml` and all parent directories.
 If, for example, `Cargo.toml` exists in `/foo/bar/baz` directory, then the following configuration files would be scanned in this order.
 
-- `/foo/bar/baz/sailfish.yml`
-- `/foo/bar/sailfish.yml`
-- `/foo/sailfish.yml`
-- `/sailfish.yml`
+- `/foo/bar/baz/sailfish.toml`
+- `/foo/bar/sailfish.toml`
+- `/foo/sailfish.toml`
+- `/sailfish.toml`
 
 If a key is specified in multiple configuration files, the value in the deeper directory takes precedence over ancestor directories.
 
@@ -47,15 +47,21 @@ If a key is specified in both configuration file and derive options, then the va
 
 ### Configuration file format
 
-Configuration files are written in the YAML 1.2 format. Here is the default configuration.
+Configuration files are written in the TOML 0.5 format. Here is the default configuration:
 
-``` yaml
-template_dir: "templates"
-escape: true
-delimiter: "%"
+``` toml
+template_dirs = ["templates"]
+escape = true
+delimiter = "%"
 
-optimization:
-    rm_whitespace: false
+[optimizations]
+rm_whitespace = false
 ```
 
-You can specify another template directory in `template_dir` option. Other options are same as derive options.
+You can specify another template directory in `template_dirs` option. Other options are same as derive options.
+
+You can also embed environment variables in `template_dirs` paths by wrapping the variable name with `${` and `}` like `${MY_ENV_VAR}`:
+
+```toml
+template_dirs = ["${CI}/path/to/project/${MYVAR}/templates"]
+```

sailfish-compiler/Cargo.toml

@@ -19,12 +19,13 @@ doctest = false
 [features]
 default = ["config"]
 procmacro = []
-config = ["yaml-rust"]
+config = ["serde", "toml"]
 
 [dependencies]
 memchr = "2.3.3"
 quote = { version = "1.0.6", default-features = false }
-yaml-rust = { version = "0.4.4", optional = true }
+serde = { version = "1.0", features = ["derive"], optional = true }
+toml = { version = "0.5", optional = true }
 home = "0.5.3"
 filetime = "0.2.14"
 

sailfish-compiler/src/config.rs

@@ -27,8 +27,8 @@ impl Default for Config {
 
 #[cfg(feature = "config")]
 mod imp {
+    use serde::Deserialize;
     use std::fs;
-    use yaml_rust::yaml::{Yaml, YamlLoader};
 
     use super::*;
     use crate::error::*;
@@ -41,7 +41,7 @@ mod imp {
 
             for component in base.iter() {
                 path.push(component);
-                path.push("sailfish.yml");
+                path.push("sailfish.toml");
 
                 if path.is_file() {
                     let config_file =
@@ -52,6 +52,14 @@ mod imp {
 
                     if let Some(template_dirs) = config_file.template_dirs {
                         for template_dir in template_dirs.into_iter().rev() {
+                            let expanded =
+                                expand_env_vars(template_dir).map_err(|mut e| {
+                                    e.source_file = Some(path.to_owned());
+                                    e
+                                })?;
+
+                            let template_dir = PathBuf::from(expanded);
+
                             if template_dir.is_absolute() {
                                 config.template_dirs.push(template_dir);
                             } else {
@@ -70,8 +78,10 @@ mod imp {
                         config.escape = escape;
                     }
 
-                    if let Some(rm_whitespace) = config_file.rm_whitespace {
-                        config.rm_whitespace = rm_whitespace;
+                    if let Some(optimizations) = config_file.optimizations {
+                        if let Some(rm_whitespace) = optimizations.rm_whitespace {
+                            config.rm_whitespace = rm_whitespace;
+                        }
                     }
                 }
 
@@ -82,154 +92,138 @@ mod imp {
         }
     }
 
-    #[derive(Default)]
+    #[derive(Deserialize, Debug)]
+    #[serde(deny_unknown_fields)]
+    struct Optimizations {
+        rm_whitespace: Option<bool>,
+    }
+
+    #[derive(Deserialize, Debug)]
+    #[serde(deny_unknown_fields)]
     struct ConfigFile {
-        template_dirs: Option<Vec<PathBuf>>,
+        template_dirs: Option<Vec<String>>,
         delimiter: Option<char>,
         escape: Option<bool>,
-        rm_whitespace: Option<bool>,
+        optimizations: Option<Optimizations>,
     }
 
     impl ConfigFile {
         fn read_from_file(path: &Path) -> Result<Self, Error> {
-            let mut config = Self::default();
             let content = fs::read_to_string(path)
                 .chain_err(|| format!("Failed to read configuration file {:?}", path))?;
-
-            let entries = YamlLoader::load_from_str(&*content)
-                .map_err(|e| ErrorKind::ConfigError(e.to_string()))?;
-            drop(content);
-
-            for entry in entries {
-                config.visit_global(entry)?
-            }
-
-            Ok(config)
+            Self::from_string(&content)
         }
 
-        fn visit_global(&mut self, entry: Yaml) -> Result<(), Error> {
-            let hash = entry.into_hash().ok_or_else(|| {
-                ErrorKind::ConfigError("Invalid configuration format".to_owned())
-            })?;
-
-            for (k, v) in hash {
-                match k {
-                    Yaml::String(ref s) => match &**s {
-                        "template_dir" => self.visit_template_dir(v)?,
-                        "delimiter" => self.visit_delimiter(v)?,
-                        "escape" => self.visit_escape(v)?,
-                        "optimization" => self.visit_optimization(v)?,
-                        _ => return Err(Self::error(format!("Unknown key ({})", s))),
-                    },
-                    _ => {
-                        return Err(Self::error("Invalid configuration format"));
-                    }
-                }
-            }
-
-            Ok(())
+        fn from_string(content: &str) -> Result<Self, Error> {
+            toml::from_str::<Self>(content).map_err(|e| error(e.to_string()))
         }
+    }
 
-        fn visit_template_dir(&mut self, value: Yaml) -> Result<(), Error> {
-            if self.template_dirs.is_some() {
-                return Err(Self::error("Duplicate key (template_dir)"));
-            }
+    fn expand_env_vars<S: AsRef<str>>(input: S) -> Result<String, Error> {
+        use std::env;
 
-            match value {
-                Yaml::String(s) => self.template_dirs = Some(vec![PathBuf::from(s)]),
-                Yaml::Array(v) => {
-                    let mut template_dirs = Vec::new();
-                    for e in v {
-                        if let Yaml::String(s) = e {
-                            template_dirs.push(PathBuf::from(s));
+        let input = input.as_ref();
+        let len = input.len();
+        let mut iter = input.chars().enumerate();
+        let mut result = String::new();
+
+        let mut found = false;
+        let mut env_var = String::new();
+
+        while let Some((i, c)) = iter.next() {
+            match c {
+                '$' if found == false => {
+                    if let Some((_, cc)) = iter.next() {
+                        if cc == '{' {
+                            found = true;
                         } else {
-                            return Err(Self::error(
-                                "Arguments of `template_dir` must be string",
-                            ));
+                            // We didn't find a trailing { after the $
+                            // so we push the chars read onto the result
+                            result.push(c);
+                            result.push(cc);
                         }
                     }
-                    self.template_dirs = Some(template_dirs);
+                }
+                '}' if found => {
+                    let val = env::var(&env_var).map_err(|e| match e {
+                        env::VarError::NotPresent => {
+                            error(format!("Environment variable ({}) not set", env_var))
+                        }
+                        env::VarError::NotUnicode(_) => error(format!(
+                            "Environment variable ({}) contents not valid unicode",
+                            env_var
+                        )),
+                    })?;
+                    result.push_str(&val);
+
+                    env_var.clear();
+                    found = false;
                 }
                 _ => {
-                    return Err(Self::error(
-                        "Arguments of `template_dir` must be string",
-                    ));
-                }
-            }
+                    if found {
+                        env_var.push(c);
 
-            Ok(())
-        }
-
-        fn visit_delimiter(&mut self, value: Yaml) -> Result<(), Error> {
-            if self.delimiter.is_some() {
-                return Err(Self::error("Duplicate key (delimiter)"));
-            }
-
-            if let Yaml::String(s) = value {
-                if s.chars().count() == 1 {
-                    self.delimiter = Some(s.chars().next().unwrap());
-                    Ok(())
-                } else {
-                    Err(Self::error("`escape` must be single character"))
-                }
-            } else {
-                Err(Self::error("`escape` must be single character"))
-            }
-        }
-
-        fn visit_escape(&mut self, value: Yaml) -> Result<(), Error> {
-            if self.escape.is_some() {
-                return Err(Self::error("Duplicate key (escape)"));
-            }
-
-            if let Yaml::Boolean(b) = value {
-                self.escape = Some(b);
-                Ok(())
-            } else {
-                Err(Self::error("`escape` must be boolean"))
-            }
-        }
-
-        fn visit_optimization(&mut self, entry: Yaml) -> Result<(), Error> {
-            let hash = entry.into_hash().ok_or_else(|| {
-                ErrorKind::ConfigError("Invalid configuration format".to_owned())
-            })?;
-
-            for (k, v) in hash {
-                match k {
-                    Yaml::String(ref s) => match &**s {
-                        "rm_whitespace" => self.visit_rm_whitespace(v)?,
-                        _ => {
-                            return Err(Self::error(format!(
-                                "Unknown key (optimization.{})",
-                                s
-                            )));
+                        // Check if we're at the end with an unclosed environment variable:
+                        // ${MYVAR instead of ${MYVAR}
+                        // If so, push it back onto the string as some systems allows the $ { characters in paths.
+                        if i == len - 1 {
+                            result.push_str("${");
+                            result.push_str(&env_var);
                         }
-                    },
-                    _ => {
-                        return Err(Self::error("Invalid configuration format"));
+                    } else {
+                        result.push(c);
                     }
                 }
             }
-
-            Ok(())
         }
 
-        fn visit_rm_whitespace(&mut self, value: Yaml) -> Result<(), Error> {
-            if self.rm_whitespace.is_some() {
-                return Err(Self::error("Duplicate key (rm_whitespace)"));
-            }
+        Ok(result)
+    }
 
-            if let Yaml::Boolean(b) = value {
-                self.rm_whitespace = Some(b);
-                Ok(())
-            } else {
-                Err(Self::error("`rm_whitespace` must be boolean"))
-            }
+    fn error<T: Into<String>>(msg: T) -> Error {
+        make_error!(ErrorKind::ConfigError(msg.into()))
+    }
+
+    #[cfg(test)]
+    mod tests {
+        use crate::config::imp::expand_env_vars;
+        use std::env;
+
+        #[test]
+        fn expands_env_vars() {
+            env::set_var("TESTVAR", "/a/path");
+            let input = "/path/to/${TESTVAR}Templates";
+            let output = expand_env_vars(input).unwrap();
+            assert_eq!(output, "/path/to//a/pathTemplates");
         }
 
-        fn error<T: Into<String>>(msg: T) -> Error {
-            make_error!(ErrorKind::ConfigError(msg.into()))
+        #[test]
+        fn retains_case_sensitivity() {
+            env::set_var("tEstVar", "/a/path");
+            let input = "/path/${tEstVar}";
+            let output = expand_env_vars(input).unwrap();
+            assert_eq!(output, "/path//a/path");
+        }
+
+        #[test]
+        fn retains_unclosed_env_var() {
+            let input = "/path/to/${UNCLOSED";
+            let output = expand_env_vars(input).unwrap();
+            assert_eq!(output, input);
+        }
+
+        #[test]
+        fn ingores_markers() {
+            let input = "path/{$/$}/${/to/{";
+            let output = expand_env_vars(input).unwrap();
+            assert_eq!(output, input);
+        }
+
+        #[test]
+        fn errors_on_unset_env_var() {
+            let input = "/path/to/${UNSET}";
+            let output = expand_env_vars(input);
+            assert!(output.is_err());
         }
     }
 }

sailfish-tests/integration-tests/config/sailfish.toml

@@ -0,0 +1,6 @@
+template_dirs = ["../templates"]
+escape = true
+delimiter = "%"
+
+[optimizations]
+rm_whitespace = false

sailfish-tests/integration-tests/config/sailfish.yml

@@ -1,6 +0,0 @@
-template_dir: "../templates"
-escape: true
-delimiter: "%"
-
-optimization:
-    rm_whitespace: false