Merge pull request #1155 from matthiasbeyer/libimagrt/config-refactoring

Rewrite configuration providing in runtime

bin/core/imag-view/src/main.rs

@@ -99,7 +99,7 @@ fn main() {
             .map_err_trace_exit_unwrap(1);
 
         let query = format!("view.viewers.{}", viewer);
-        match config.config().read(&query) {
+        match config.read(&query) {
             Err(e) => trace_error_exit(&e, 1),
             Ok(None) => {
                 error!("Cannot find '{}' in config", query);

bin/core/imag/src/main.rs

@@ -256,7 +256,6 @@ fn main() {
 fn fetch_aliases(rt: &Runtime) -> Result<BTreeMap<String, String>, String> {
     let cfg   = try!(rt.config().ok_or_else(|| String::from("No configuration found")));
     let value = cfg
-        .config()
         .read("imag.aliases")
         .map_err(|_| String::from("Reading from config failed"));
 

bin/domain/imag-bookmark/src/main.rs

@@ -169,7 +169,7 @@ fn get_collection_name(rt: &Runtime,
         .and_then(|scmd| scmd.value_of(collection_argument_name).map(String::from))
         .unwrap_or_else(|| {
             rt.config()
-                .map(|cfg| match cfg.config().read("bookmark.default_collection") {
+                .map(|cfg| match cfg.read("bookmark.default_collection") {
                     Err(e) => trace_error_exit(&e, 1),
                     Ok(Some(&Value::String(ref name))) => name.clone(),
                     Ok(None) => {

bin/domain/imag-diary/src/util.rs

@@ -53,7 +53,6 @@ pub fn get_diary_timed_config(rt: &Runtime, diary_name: &str) -> Result<Option<T
         None      => Ok(None),
         Some(cfg) => {
             let v = cfg
-                .config()
                 .read(&format!("diary.diaries.{}.timed", diary_name))
                 .chain_err(|| DiaryErrorKind::IOError);
 

doc/src/09020-changelog.md

@@ -30,6 +30,9 @@ This section contains the changelog from the last release to the next release.
     * `imag-store` can dump all storeids now
     * `imag-annotate` was introduced
     * `imag-diagnostics` was added
+    * The runtime does not read the config file for editor settings anymore.
+      Specifying an editor either via CLI or via the `$EDITOR` environment
+      variable still possible.
 
 * Minor changes
     * `libimagentryannotation` got a rewrite, is not based on `libimagnotes`

lib/core/libimagrt/src/configuration.rs

@@ -18,228 +18,23 @@
 //
 
 use std::path::PathBuf;
-use std::ops::Deref;
 
 use toml::Value;
 use clap::App;
 
-error_chain! {
+use error::RuntimeError as RE;
-    types {
+use error::RuntimeErrorKind as REK;
-        ConfigError, ConfigErrorKind, ResultExt, Result;
+use error::Result;
-    }
+use error::ResultExt;
 
-    errors {
+/// Get a new configuration object.
-        TOMLParserError {
-            description("TOML Parsing error")
-            display("TOML Parsing error")
-        }
-
-        NoConfigFileFound   {
-            description("No config file found")
-            display("No config file found")
-        }
-
-        ConfigOverrideError {
-            description("Config override error")
-            display("Config override error")
-        }
-
-        ConfigOverrideKeyNotAvailable {
-            description("Key not available")
-            display("Key not available")
-        }
-
-        ConfigOverrideTypeNotMatching {
-            description("Configuration Type not matching")
-            display("Configuration Type not matching")
-        }
-    }
-}
-use self::ConfigErrorKind as CEK;
-use self::ConfigError as CE;
-
-/// `Configuration` object
 ///
-/// Holds all config variables which are globally available plus the configuration object from the
+/// The passed runtimepath is used for searching the configuration file, whereas several file
-/// config parser, which can be accessed.
+/// names are tested. If that does not work, the home directory and the XDG basedir are tested
-#[derive(Debug)]
+/// with all variants.
-pub struct Configuration {
-
-    /// The plain configuration object for direct access if necessary
-    config: Value,
-
-    /// The verbosity the program should run with
-    verbosity: bool,
-
-    /// The editor which should be used
-    editor: Option<String>,
-
-    ///The options the editor should get when opening some file
-    editor_opts: String,
-}
-
-impl Configuration {
-
-    /// Get a new configuration object.
-    ///
-    /// The passed runtimepath is used for searching the configuration file, whereas several file
-    /// names are tested. If that does not work, the home directory and the XDG basedir are tested
-    /// with all variants.
-    ///
-    /// If that doesn't work either, an error is returned.
-    pub fn new(config_searchpath: &PathBuf) -> Result<Configuration> {
-        fetch_config(&config_searchpath).map(|cfg| {
-            let verbosity   = get_verbosity(&cfg);
-            let editor      = get_editor(&cfg);
-            let editor_opts = get_editor_opts(&cfg);
-
-            debug!("Building configuration");
-            debug!("  - verbosity  : {:?}", verbosity);
-            debug!("  - editor     : {:?}", editor);
-            debug!("  - editor-opts: {}", editor_opts);
-
-            Configuration {
-                config: cfg,
-                verbosity: verbosity,
-                editor: editor,
-                editor_opts: editor_opts,
-            }
-        })
-    }
-
-    /// Get a new configuration object built from the given toml value.
-    pub fn with_value(value: Value) -> Configuration {
-        Configuration{
-            verbosity: get_verbosity(&value),
-            editor: get_editor(&value),
-            editor_opts: get_editor_opts(&value),
-            config: value,
-        }
-    }
-
-    /// Get the Editor setting from the configuration
-    pub fn editor(&self) -> Option<&String> {
-        self.editor.as_ref()
-    }
-
-    /// Get the underlying configuration TOML object
-    pub fn config(&self) -> &Value {
-        &self.config
-    }
-
-    /// Get the configuration of the store, if any.
-    pub fn store_config(&self) -> Option<&Value> {
-        match self.config {
-            Value::Table(ref tabl) => tabl.get("store"),
-            _ => None,
-        }
-    }
-
-    /// Override the configuration.
-    /// The `v` parameter is expected to contain 'key=value' pairs where the key is a path in the
-    /// TOML tree, the value to be an appropriate value.
-    ///
-    /// The override fails if the configuration which is about to be overridden does not exist or
-    /// the `value` part cannot be converted to the type of the configuration value.
-    ///
-    /// If `v` is empty, this is considered to be a successful `override_config()` call.
-    pub fn override_config(&mut self, v: Vec<String>) -> Result<()> {
-        use libimagutil::key_value_split::*;
-        use toml_query::read::TomlValueReadExt;
-
-        let iter = v.into_iter()
-            .map(|s| { debug!("Trying to process '{}'", s); s })
-            .filter_map(|s| match s.into_kv() {
-                Some(kv) => Some(kv.into()),
-                None => {
-                    warn!("Could split at '=' - will be ignore override");
-                    None
-                }
-            })
-            .map(|(k, v)| self
-                 .config
-                 .read(&k[..])
-                 .chain_err(|| CEK::TOMLParserError)
-                 .map(|toml| match toml {
-                    Some(value) => match into_value(value, v) {
-                        Some(v) => {
-                            info!("Successfully overridden: {} = {}", k, v);
-                            Ok(())
-                        },
-                        None => Err(CE::from_kind(CEK::ConfigOverrideTypeNotMatching)),
-                    },
-                    None => Err(CE::from_kind(CEK::ConfigOverrideKeyNotAvailable)),
-                })
-            );
-
-        for elem in iter {
-            let _ = try!(elem.chain_err(|| CEK::ConfigOverrideError));
-        }
-
-        Ok(())
-    }
-}
-
-/// Tries to convert the String `s` into the same type as `value`.
 ///
-/// Returns None if string cannot be converted.
+/// If that doesn't work either, an error is returned.
-///
+pub fn fetch_config(searchpath: &PathBuf) -> Result<Value> {
-/// Arrays and Tables are not supported and will yield `None`.
-fn into_value(value: &Value, s: String) -> Option<Value> {
-    use std::str::FromStr;
-
-    match *value {
-        Value::String(_)  => Some(Value::String(s)),
-        Value::Integer(_) => FromStr::from_str(&s[..]).ok().map(Value::Integer),
-        Value::Float(_)   => FromStr::from_str(&s[..]).ok().map(Value::Float),
-        Value::Boolean(_) => {
-            if s == "true" { Some(Value::Boolean(true)) }
-            else if s == "false" { Some(Value::Boolean(false)) }
-            else { None }
-        }
-        Value::Datetime(_) => Value::try_from(s).ok(),
-        _ => None,
-    }
-}
-
-impl Deref for Configuration {
-    type Target = Value;
-
-    fn deref(&self) -> &Value {
-        &self.config
-    }
-
-}
-
-fn get_verbosity(v: &Value) -> bool {
-    match *v {
-        Value::Table(ref t) => t.get("verbose")
-                .map_or(false, |v| is_match!(v, &Value::Boolean(true))),
-        _ => false,
-    }
-}
-
-fn get_editor(v: &Value) -> Option<String> {
-    match *v {
-        Value::Table(ref t) => t.get("editor")
-                .and_then(|v| match *v { Value::String(ref s) => Some(s.clone()), _ => None, }),
-        _ => None,
-    }
-}
-
-fn get_editor_opts(v: &Value) -> String {
-    match *v {
-        Value::Table(ref t) => t.get("editor-opts")
-                .and_then(|v| match *v { Value::String(ref s) => Some(s.clone()), _ => None, })
-                .unwrap_or_default(),
-        _ => String::new(),
-    }
-}
-
-/// Helper to fetch the config file
-///
-/// Tests several variants for the config file path and uses the first one which works.
-fn fetch_config(searchpath: &PathBuf) -> Result<Value> {
     use std::env;
     use std::fs::File;
     use std::io::Read;
@@ -302,7 +97,72 @@ fn fetch_config(searchpath: &PathBuf) -> Result<Value> {
             }
         })
         .nth(0)
-        .ok_or(CE::from_kind(ConfigErrorKind::NoConfigFileFound))
+        .ok_or(RE::from_kind(REK::ConfigNoConfigFileFound))
+}
+
+/// Override the configuration.
+/// The `v` parameter is expected to contain 'key=value' pairs where the key is a path in the
+/// TOML tree, the value to be an appropriate value.
+///
+/// The override fails if the configuration which is about to be overridden does not exist or
+/// the `value` part cannot be converted to the type of the configuration value.
+///
+/// If `v` is empty, this is considered to be a successful `override_config()` call.
+pub fn override_config(val: &mut Value, v: Vec<String>) -> Result<()> {
+    use libimagutil::key_value_split::*;
+    use toml_query::read::TomlValueReadExt;
+
+    let iter = v.into_iter()
+        .map(|s| { debug!("Trying to process '{}'", s); s })
+        .filter_map(|s| match s.into_kv() {
+            Some(kv) => Some(kv.into()),
+            None => {
+                warn!("Could split at '=' - will be ignore override");
+                None
+            }
+        })
+        .map(|(k, v)| val
+             .read(&k[..])
+             .chain_err(|| REK::ConfigTOMLParserError)
+             .map(|toml| match toml {
+                Some(value) => match into_value(value, v) {
+                    Some(v) => {
+                        info!("Successfully overridden: {} = {}", k, v);
+                        Ok(())
+                    },
+                    None => Err(RE::from_kind(REK::ConfigOverrideTypeNotMatching)),
+                },
+                None => Err(RE::from_kind(REK::ConfigOverrideKeyNotAvailable)),
+            })
+        );
+
+    for elem in iter {
+        let _ = try!(elem.chain_err(|| REK::ConfigOverrideError));
+    }
+
+    Ok(())
+}
+
+/// Tries to convert the String `s` into the same type as `value`.
+///
+/// Returns None if string cannot be converted.
+///
+/// Arrays and Tables are not supported and will yield `None`.
+fn into_value(value: &Value, s: String) -> Option<Value> {
+    use std::str::FromStr;
+
+    match *value {
+        Value::String(_)  => Some(Value::String(s)),
+        Value::Integer(_) => FromStr::from_str(&s[..]).ok().map(Value::Integer),
+        Value::Float(_)   => FromStr::from_str(&s[..]).ok().map(Value::Float),
+        Value::Boolean(_) => {
+            if s == "true" { Some(Value::Boolean(true)) }
+            else if s == "false" { Some(Value::Boolean(false)) }
+            else { None }
+        }
+        Value::Datetime(_) => Value::try_from(s).ok(),
+        _ => None,
+    }
 }
 
 pub trait InternalConfiguration {
@@ -316,3 +176,4 @@ pub trait InternalConfiguration {
 }
 
 impl<'a> InternalConfiguration for App<'a, 'a> {}
+

lib/core/libimagrt/src/error.rs

@@ -93,6 +93,31 @@ error_chain! {
             display("Missing config for logging format for error logging")
         }
 
+        ConfigTOMLParserError {
+            description("Configuration: TOML Parsing error")
+            display("Configuration: TOML Parsing error")
+        }
+
+        ConfigNoConfigFileFound   {
+            description("Configuration: No config file found")
+            display("Configuration: No config file found")
+        }
+
+        ConfigOverrideError {
+            description("Configuration: Config override error")
+            display("Configuration: Config override error")
+        }
+
+        ConfigOverrideKeyNotAvailable {
+            description("Configuration: Key not available")
+            display("Configuration: Key not available")
+        }
+
+        ConfigOverrideTypeNotMatching {
+            description("Configuration: Configuration Type not matching")
+            display("Configuration: Configuration Type not matching")
+        }
+
     }
 }
 

lib/core/libimagrt/src/logger.rs

@@ -24,7 +24,6 @@ use std::sync::Arc;
 use std::sync::Mutex;
 use std::ops::Deref;
 
-use configuration::Configuration;
 use error::RuntimeErrorKind as EK;
 use error::RuntimeError as RE;
 use error::ResultExt;
@@ -77,7 +76,7 @@ pub struct ImagLogger {
 impl ImagLogger {
 
     /// Create a new ImagLogger object with a certain level
-    pub fn new(matches: &ArgMatches, config: Option<&Configuration>) -> Result<ImagLogger> {
+    pub fn new(matches: &ArgMatches, config: Option<&Value>) -> Result<ImagLogger> {
         let mut handlebars = Handlebars::new();
 
         handlebars.register_escape_fn(::handlebars::no_escape);
@@ -220,7 +219,7 @@ fn match_log_level_str(s: &str) -> Result<LogLevel> {
     }
 }
 
-fn aggregate_global_loglevel(matches: &ArgMatches, config: Option<&Configuration>)
+fn aggregate_global_loglevel(matches: &ArgMatches, config: Option<&Value>)
     -> Result<LogLevel>
 {
     fn get_arg_loglevel(matches: &ArgMatches) -> Result<Option<LogLevel>> {
@@ -300,7 +299,7 @@ fn translate_destinations(raw: &Vec<Value>) -> Result<Vec<LogDestination>> {
         })
 }
 
-fn aggregate_global_destinations(matches: &ArgMatches, config: Option<&Configuration>)
+fn aggregate_global_destinations(matches: &ArgMatches, config: Option<&Value>)
     -> Result<Vec<LogDestination>>
 {
 
@@ -344,7 +343,7 @@ macro_rules! aggregate_global_format {
     };
 }
 
-fn aggregate_global_format_trace(config: Option<&Configuration>)
+fn aggregate_global_format_trace(config: Option<&Value>)
     -> Result<String>
 {
     aggregate_global_format!("imag.logging.format.trace",
@@ -352,7 +351,7 @@ fn aggregate_global_format_trace(config: Option<&Configuration>)
                             config)
 }
 
-fn aggregate_global_format_debug(config: Option<&Configuration>)
+fn aggregate_global_format_debug(config: Option<&Value>)
     -> Result<String>
 {
     aggregate_global_format!("imag.logging.format.debug",
@@ -360,7 +359,7 @@ fn aggregate_global_format_debug(config: Option<&Configuration>)
                             config)
 }
 
-fn aggregate_global_format_info(config: Option<&Configuration>)
+fn aggregate_global_format_info(config: Option<&Value>)
     -> Result<String>
 {
     aggregate_global_format!("imag.logging.format.info",
@@ -368,7 +367,7 @@ fn aggregate_global_format_info(config: Option<&Configuration>)
                             config)
 }
 
-fn aggregate_global_format_warn(config: Option<&Configuration>)
+fn aggregate_global_format_warn(config: Option<&Value>)
     -> Result<String>
 {
     aggregate_global_format!("imag.logging.format.warn",
@@ -376,7 +375,7 @@ fn aggregate_global_format_warn(config: Option<&Configuration>)
                             config)
 }
 
-fn aggregate_global_format_error(config: Option<&Configuration>)
+fn aggregate_global_format_error(config: Option<&Value>)
     -> Result<String>
 {
     aggregate_global_format!("imag.logging.format.error",
@@ -384,7 +383,7 @@ fn aggregate_global_format_error(config: Option<&Configuration>)
                             config)
 }
 
-fn aggregate_module_settings(_matches: &ArgMatches, config: Option<&Configuration>)
+fn aggregate_module_settings(_matches: &ArgMatches, config: Option<&Value>)
     -> Result<BTreeMap<ModuleName, ModuleSettings>>
 {
     match config {

lib/core/libimagrt/src/runtime.rs

@@ -23,11 +23,13 @@ use std::env;
 use std::process::exit;
 
 pub use clap::App;
+use toml::Value;
+use toml_query::read::TomlValueReadExt;
 
 use clap::{Arg, ArgMatches};
 use log;
 
-use configuration::{Configuration, InternalConfiguration};
+use configuration::{fetch_config, override_config, InternalConfiguration};
 use error::RuntimeError;
 use error::RuntimeErrorKind;
 use error::ResultExt;
@@ -44,7 +46,7 @@ use spec::CliSpec;
 #[derive(Debug)]
 pub struct Runtime<'a> {
     rtp: PathBuf,
-    configuration: Option<Configuration>,
+    configuration: Option<Value>,
     cli_matches: ArgMatches<'a>,
     store: Store,
 }
@@ -61,8 +63,6 @@ impl<'a> Runtime<'a> {
     {
         use libimagerror::trace::trace_error;
 
-        use configuration::ConfigErrorKind;
-
         let matches = cli_app.clone().matches();
 
         let rtp = get_rtp_match(&matches);
@@ -72,8 +72,8 @@ impl<'a> Runtime<'a> {
 
         debug!("Config path = {:?}", configpath);
 
-        let config = match Configuration::new(&configpath) {
+        let config = match fetch_config(&configpath) {
-            Err(e) => if !is_match!(e.kind(), &ConfigErrorKind::NoConfigFileFound) {
+            Err(e) => if !is_match!(e.kind(), &RuntimeErrorKind::ConfigNoConfigFileFound) {
                 return Err(e).chain_err(|| RuntimeErrorKind::Instantiate);
             } else {
                 println!("No config file found.");
@@ -82,7 +82,7 @@ impl<'a> Runtime<'a> {
             },
 
             Ok(mut config) => {
-                if let Err(e) = config.override_config(get_override_specs(&matches)) {
+                if let Err(e) = override_config(&mut config, get_override_specs(&matches)) {
                     error!("Could not apply config overrides");
                     trace_error(&e);
 
@@ -97,7 +97,7 @@ impl<'a> Runtime<'a> {
     }
 
     /// Builds the Runtime object using the given `config`.
-    pub fn with_configuration<C>(cli_app: C, config: Option<Configuration>)
+    pub fn with_configuration<C>(cli_app: C, config: Option<Value>)
                                  -> Result<Runtime<'a>, RuntimeError>
         where C: Clone + CliSpec<'a> + InternalConfiguration
     {
@@ -105,7 +105,7 @@ impl<'a> Runtime<'a> {
         Runtime::_new(cli_app, matches, config)
     }
 
-    fn _new<C>(mut cli_app: C, matches: ArgMatches<'a>, config: Option<Configuration>)
+    fn _new<C>(mut cli_app: C, matches: ArgMatches<'a>, config: Option<Value>)
                -> Result<Runtime<'a>, RuntimeError>
     where C: Clone + CliSpec<'a> + InternalConfiguration
     {
@@ -139,7 +139,7 @@ impl<'a> Runtime<'a> {
         debug!("Store path  = {:?}", storepath);
 
         let store_config = match config {
-            Some(ref c) => c.store_config().cloned(),
+            Some(ref c) => c.read("store").chain_err(|| RuntimeErrorKind::Instantiate)?.cloned(),
             None        => None,
         };
 
@@ -345,7 +345,7 @@ impl<'a> Runtime<'a> {
     }
 
     /// Initialize the internal logger
-    fn init_logger(matches: &ArgMatches, config: Option<&Configuration>) {
+    fn init_logger(matches: &ArgMatches, config: Option<&Value>) {
         use std::env::var as env_var;
         use env_logger;
 
@@ -386,7 +386,7 @@ impl<'a> Runtime<'a> {
     }
 
     /// Get the configuration object
-    pub fn config(&self) -> Option<&Configuration> {
+    pub fn config(&self) -> Option<&Value> {
         self.configuration.as_ref()
     }
 
@@ -444,10 +444,6 @@ impl<'a> Runtime<'a> {
         self.cli()
             .value_of("editor")
             .map(String::from)
-            .or(match self.configuration {
-                Some(ref c) => c.editor().cloned(),
-                _ => None,
-            })
             .or(env::var("EDITOR").ok())
             .map(Command::new)
     }

lib/domain/libimagdiary/src/config.rs

@@ -34,10 +34,8 @@ pub fn get_default_diary_name(rt: &Runtime) -> Option<String> {
 }
 
 pub fn get_diary_config_section<'a>(rt: &'a Runtime) -> Option<&'a Value> {
-    rt.config()
+    rt.config().and_then(|config| match config.read(&String::from("diary")) {
-        .map(|config| config.config())
+        Ok(x)  => x,
-        .and_then(|config| match config.read(&String::from("diary")) {
+        Err(_) => None,
-            Ok(x)  => x,
+    })
-            Err(_) => None,
-        })
 }

lib/etc/libimaginteraction/src/readline.rs

@@ -34,9 +34,8 @@ pub struct Readline {
 impl Readline {
 
     pub fn new(rt: &Runtime) -> Result<Readline> {
-        let cfg = try!(rt.config().ok_or(IEK::NoConfigError));
+        let c = try!(rt.config().ok_or(IEK::NoConfigError));
 
-        let c = cfg.config();
         let histfile     = try!(c.lookup("ui.cli.readline_history_file").ok_or(IEK::ConfigError));
         let histsize     = try!(c.lookup("ui.cli.readline_history_size").ok_or(IEK::ConfigError));
         let histigndups  = try!(c.lookup("ui.cli.readline_history_ignore_dups").ok_or(IEK::ConfigError));

lib/etc/libimagutil/src/testing.rs

@@ -51,7 +51,8 @@ macro_rules! make_mock_app {
             use libimagrt::spec::CliSpec;
             use libimagrt::runtime::Runtime;
             use libimagrt::error::RuntimeError;
-            use libimagrt::configuration::{Configuration, InternalConfiguration};
+            use libimagrt::configuration::InternalConfiguration;
+            use toml::Value;
 
             #[derive(Clone)]
             struct MockLinkApp<'a> {
@@ -89,9 +90,8 @@ macro_rules! make_mock_app {
             }
 
             #[allow(unused)]
-            pub fn generate_minimal_test_config() -> Option<Configuration> { ::toml::de::from_str("[store]\nimplicit-create=true")
+            pub fn generate_minimal_test_config() -> Option<Value> {
-                            .map(Configuration::with_value)
+                ::toml::de::from_str("[store]\nimplicit-create=true").ok()
-                            .ok()
             }
 
             #[allow(unused)]