Rewrite configuration providing in runtime

Before the configuration object (the raw TOML object) was provided via a
wrapper object `Configuration`. This was ugly and not very nice to use.

Now, we only have the `toml::Value` object we lend out from
`Runtime::config()`.

The changes included libimagrt internal rewrites, which are not visible
to the user. Anyways, this change changes the API for config-fetching
from the runtime, so fixes for all other crates may follow.

The changes also removed the support for reading the "editor" setting
from the configuration file, which was not used anyways (in the example
imagrc.toml file).
The CLI-reading and ENV-reading are still supported, though.

5 files changed, 126 insertions, 242 deletions

diff --git a/doc/src/09020-changelog.md b/doc/src/09020-changelog.md
index 763c4763..baa9e0d6 100644
--- a/doc/src/09020-changelog.md
+++ b/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`
diff --git a/lib/core/libimagrt/src/configuration.rs b/lib/core/libimagrt/src/configuration.rs
index 985c9446..88ed6512 100644
--- a/lib/core/libimagrt/src/configuration.rs
+++ b/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! {
-    types {
-        ConfigError, ConfigErrorKind, ResultExt, Result;
-    }
-
-    errors {
-        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")
-        }
+use error::RuntimeError as RE;
+use error::RuntimeErrorKind as REK;
+use error::Result;
+use error::ResultExt;
 
-        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
+/// Get a new configuration object.
 ///
-/// Holds all config variables which are globally available plus the configuration object from the
-/// config parser, which can be accessed.
-#[derive(Debug)]
-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.
-///
-/// 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
+/// 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.
 ///
-/// Tests several variants for the config file path and uses the first one which works.
-fn fetch_config(searchpath: &PathBuf) -> Result<Value> {
+/// If that doesn't work either, an error is returned.
+pub 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> {}
+
diff --git a/lib/core/libimagrt/src/error.rs b/lib/core/libimagrt/src/error.rs
index a0e32d1b..4537c8a6 100644
--- a/lib/core/libimagrt/src/error.rs
+++ b/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")
+        }
+
     }
 }
 
diff --git a/lib/core/libimagrt/src/logger.rs b/lib/core/libimagrt/src/logger.rs
index 241ff73c..60f50d6d 100644
--- a/lib/core/libimagrt/src/logger.rs
+++ b/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 {
diff --git a/lib/core/libimagrt/src/runtime.rs b/lib/core/libimagrt/src/runtime.rs
index 985ff990..8e805723 100644
--- a/lib/core/libimagrt/src/runtime.rs
+++ b/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) {
-            Err(e) => if !is_match!(e.kind(), &ConfigErrorKind::NoConfigFileFound) {
+        let config = match fetch_config(&configpath) {
+            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)
     }