Allow references in Config (#429)

For #251, I want to use the `Language<'_>` type inside `Config`. The languages will either refer to values read from the config file, or to static strings from `get_languages_from_env`, so just using `Language<'static>` is not an option.

Instead, some input for the `Config` needs to be persisted in the main function for the duration of the program so that the config can reference it. At first I was hoping that this input would be the `contents` string from `RawConfig::load`, but as it turns out you cannot (in general) deserialize strings from toml without having to alter them, for example when they contain escapes like `\n`. Thus, the toml parser seemingly doesn't even try and just throws an error when deserializing into a borrowed string (even if it could in theory just return the correct substring from the input). Given that `RawConfig` should stay static then, the raw config itself is the next best thing to keep alive and have the config reference into.

While this change might seem a bit drastic for little benefit, I am actually pretty happy with it because I want to unify the configuration anyways at some point so that the CLI arguments, environment variables, and the config file are merged at the beginning of the program and then only a single config is used for the everything (no more `enable_styles` everywhere!). At this time, the `Config` would have references into `Cli` anyways, and having the `ConfigLoader` as an entity for this merging also seems natural.

src/config.rs

@@ -1,7 +1,7 @@
 use std::{
     env, fmt,
     fs::{self, File},
-    io::{ErrorKind, Read, Write},
+    io::{ErrorKind, Write},
     path::{Path, PathBuf},
     time::Duration,
 };
@@ -138,8 +138,8 @@ struct RawStyleConfig {
     pub example_variable: RawStyle,
 }
 
-impl From<RawStyleConfig> for StyleConfig {
+impl From<&RawStyleConfig> for StyleConfig {
-    fn from(raw_style_config: RawStyleConfig) -> Self {
+    fn from(raw_style_config: &RawStyleConfig) -> Self {
         Self {
             command_name: raw_style_config.command_name.into(),
             description: raw_style_config.description.into(),
@@ -158,8 +158,8 @@ struct RawDisplayConfig {
     pub use_pager: bool,
 }
 
-impl From<RawDisplayConfig> for DisplayConfig {
+impl From<&RawDisplayConfig> for DisplayConfig {
-    fn from(raw_display_config: RawDisplayConfig) -> Self {
+    fn from(raw_display_config: &RawDisplayConfig) -> Self {
         Self {
             compact: raw_display_config.compact,
             use_pager: raw_display_config.use_pager,
@@ -202,10 +202,10 @@ impl Default for RawUpdatesConfig {
     }
 }
 
-impl TryFrom<RawUpdatesConfig> for UpdatesConfig {
+impl<'a> TryFrom<&'a RawUpdatesConfig> for UpdatesConfig<'a> {
     type Error = anyhow::Error;
 
-    fn try_from(raw_updates_config: RawUpdatesConfig) -> Result<Self> {
+    fn try_from(raw_updates_config: &'a RawUpdatesConfig) -> Result<Self> {
         let tls_backend = match raw_updates_config.tls_backend {
             #[cfg(feature = "native-tls")]
             RawTlsBackend::NativeTls => TlsBackend::NativeTls,
@@ -227,7 +227,7 @@ impl TryFrom<RawUpdatesConfig> for UpdatesConfig {
             auto_update_interval: Duration::from_secs(
                 raw_updates_config.auto_update_interval_hours * 3600,
             ),
-            archive_source: raw_updates_config.archive_source,
+            archive_source: &raw_updates_config.archive_source,
             tls_backend,
         })
     }
@@ -250,20 +250,6 @@ struct RawConfig {
     directories: RawDirectoriesConfig,
 }
 
-impl RawConfig {
-    fn new() -> Self {
-        Self::default()
-    }
-
-    fn load(mut config: impl Read) -> Result<RawConfig> {
-        let mut content = String::new();
-        config
-            .read_to_string(&mut content)
-            .context("Failed to read from config file")?;
-        toml::from_str(&content).context("Failed to parse TOML config file")
-    }
-}
-
 impl Default for RawConfig {
     fn default() -> Self {
         let mut raw_config = RawConfig {
@@ -300,10 +286,10 @@ pub struct DisplayConfig {
 }
 
 #[derive(Clone, Debug, PartialEq, Eq)]
-pub struct UpdatesConfig {
+pub struct UpdatesConfig<'a> {
     pub auto_update: bool,
     pub auto_update_interval: Duration,
-    pub archive_source: String,
+    pub archive_source: &'a str,
     pub tls_backend: TlsBackend,
 }
 
@@ -369,23 +355,23 @@ pub enum TlsBackend {
 }
 
 #[derive(Clone, Debug, PartialEq, Eq)]
-pub struct Config {
+pub struct Config<'a> {
     pub style: StyleConfig,
     pub display: DisplayConfig,
-    pub updates: UpdatesConfig,
+    pub updates: UpdatesConfig<'a>,
     pub directories: DirectoriesConfig,
     pub file_path: PathWithSource,
 }
 
-impl Config {
+impl<'a> Config<'a> {
     /// Convert a `RawConfig` to a high-level `Config`.
     ///
     /// For this, some values need to be converted to other types and some
     /// defaults need to be set (sometimes based on env variables).
-    fn from_raw(raw_config: RawConfig, config_file_path: PathWithSource) -> Result<Self> {
+    fn from_raw(raw_config: &'a RawConfig, config_file_path: PathWithSource) -> Result<Self> {
-        let style = raw_config.style.into();
+        let style = (&raw_config.style).into();
-        let display = raw_config.display.into();
+        let display = (&raw_config.display).into();
-        let updates = raw_config.updates.try_into()?;
+        let updates = (&raw_config.updates).try_into()?;
         let relative_path_root = config_file_path
             .path()
             .parent()
@@ -404,7 +390,7 @@ impl Config {
                 path: PathBuf::from(env_var),
                 source: PathSource::EnvVar,
             }
-        } else if let Some(config_value) = raw_config.directories.cache_dir {
+        } else if let Some(config_value) = &raw_config.directories.cache_dir {
             // If the user explicitly configured a cache directory, use that.
             PathWithSource {
                 // Resolve possible relative path. It would be nicer to clean up the path, but Rust stdlib
@@ -425,6 +411,7 @@ impl Config {
         let custom_pages_dir = raw_config
             .directories
             .custom_pages_dir
+            .as_ref()
             .map(|path| PathWithSource {
                 // Resolve possible relative path.
                 path: relative_path_root.join(path),
@@ -454,47 +441,64 @@ impl Config {
             file_path: config_file_path,
         })
     }
+}
 
-    /// Load and read the config file from the given path into
+/// The [`ConfigLoader`] is used to load a [`Config`] from a file.
-    /// a [Config] and return it.
+///
-    ///
+/// Since the rich [`Config`] keeps references to [`RawConfig`], the raw config needs to be kept alive outside of the
-    /// path: The path to the config file.
+/// [`Config`]. The [`ConfigLoader`] thus offers the following flow:
-    pub fn load(path: &Path) -> Result<Self> {
+/// 1. Read a raw config using [`ConfigLoader::read`] or [`ConfigLoader::read_default_path`].
-        let raw_config = RawConfig::load(File::open(path)?)?;
+/// 2. Validate the contents to a [`Config`] that borrows the [`ConfigLoader`].
+pub struct ConfigLoader {
+    raw: RawConfig,
+    path: PathWithSource,
+}
 
-        let config = Self::from_raw(
+impl ConfigLoader {
-            raw_config,
+    fn read_internal(path: PathWithSource, allow_not_found: bool) -> Result<Self> {
-            PathWithSource {
+        match fs::read_to_string(&path.path) {
-                path: path.into(),
+            Ok(content) => Ok(Self {
-                source: PathSource::Cli,
+                raw: toml::from_str(&content).with_context(|| {
-            },
+                    format!(
-        )
+                        "Could not parse config file contents as toml from {}.",
-        .context("Could not process raw config")?;
+                        path.path.display()
-
+                    )
-        Ok(config)
+                })?,
+                path,
+            }),
+            Err(e) if allow_not_found && e.kind() == ErrorKind::NotFound => Ok(Self {
+                raw: RawConfig::default(),
+                path,
+            }),
+            Err(e) => Err(e).context(format!(
+                "Could not read config file contents from {}.",
+                path.path().display()
+            )),
+        }
     }
 
-    /// Load and read the config file from the default path into
+    /// Create a loader that uses the config at `path`.
-    /// a [Config] and return it.
+    pub fn read(path: PathBuf) -> Result<Self> {
-    pub fn load_default_path() -> Result<Self> {
+        Self::read_internal(
-        // Determine path
+            PathWithSource {
-        let config_file_path =
+                path,
-            get_default_config_path().context("Could not determine config path")?;
+                source: PathSource::Cli,
+            },
+            false,
+        )
+    }
 
-        let raw_config = match File::open(config_file_path.path()) {
+    /// Create a loader that uses the default config file location. If no file is present at the default location, the
-            Ok(file) => RawConfig::load(file)?,
+    /// default configuration is used.
-            Err(e) if e.kind() == ErrorKind::NotFound => RawConfig::default(),
+    pub fn read_default_path() -> Result<Self> {
-            Err(e) => {
+        let path = get_default_config_path().context("Could not determine default config path.")?;
-                return Err(e).context(format!(
+        Self::read_internal(path, true)
-                    "Failed to open config file at {}",
+    }
-                    config_file_path.path().display()
-                ));
-            }
-        };
-        let config =
-            Self::from_raw(raw_config, config_file_path).context("Could not process raw config")?;
 
-        Ok(config)
+    /// Parse the read [`RawConfig`] into a [`Config`].
+    pub fn load(&self) -> Result<Config<'_>> {
+        Config::from_raw(&self.raw, self.path.clone())
+            .context("Could not process raw config into rich config")
     }
 }
 
@@ -563,7 +567,7 @@ pub fn make_default_config(path: Option<&Path>) -> Result<PathBuf> {
 
     // Create default config
     let serialized_config =
-        toml::to_string(&RawConfig::new()).context("Failed to serialize default config")?;
+        toml::to_string(&RawConfig::default()).context("Failed to serialize default config")?;
 
     // Write default config
     let mut config_file =
@@ -577,7 +581,7 @@ pub fn make_default_config(path: Option<&Path>) -> Result<PathBuf> {
 
 #[test]
 fn test_serialize_deserialize() {
-    let raw_config = RawConfig::new();
+    let raw_config = RawConfig::default();
     let serialized = toml::to_string(&raw_config).unwrap();
     let deserialized: RawConfig = toml::from_str(&serialized).unwrap();
     assert_eq!(raw_config, deserialized);
@@ -585,12 +589,12 @@ fn test_serialize_deserialize() {
 
 #[test]
 fn test_relative_path_resolution() {
-    let mut raw_config = RawConfig::new();
+    let mut raw_config = RawConfig::default();
     raw_config.directories.cache_dir = Some("../cache".into());
     raw_config.directories.custom_pages_dir = Some("../custom_pages".into());
 
     let config = Config::from_raw(
-        raw_config,
+        &raw_config,
         PathWithSource {
             path: PathBuf::from("/path/to/config/config.toml"),
             source: PathSource::OsConvention,

src/main.rs

@@ -38,7 +38,7 @@ use anyhow::{anyhow, Context, Result};
 use app_dirs::AppInfo;
 use cache::{CacheConfig, Language, TLDR_OLD_PAGES_DIR};
 use clap::Parser;
-use config::{StyleConfig, TlsBackend};
+use config::{ConfigLoader, StyleConfig, TlsBackend};
 use log::debug;
 
 mod cache;
@@ -233,12 +233,15 @@ fn main() -> ExitCode {
 fn try_main(args: Cli, enable_styles: bool) -> Result<ExitCode> {
     // Look up config file, if none is found fall back to default config.
     debug!("Loading config");
-    let mut config = match &args.config_path {
+    let config_loader = match &args.config_path {
         Some(path) if !args.seed_config => {
-            Config::load(path).context("Could not load config from given path")?
+            ConfigLoader::read(path.clone()).context("Could not read config from given path")?
+        }
+        _ => {
+            ConfigLoader::read_default_path().context("Could not read config from default path")?
         }
-        _ => Config::load_default_path().context("Could not load config from default path")?,
     };
+    let mut config = config_loader.load()?;
 
     // Override styles if needed
     if !enable_styles {
@@ -327,7 +330,7 @@ fn try_main(args: Cli, enable_styles: bool) -> Result<ExitCode> {
         if was_created || args.update || cache.age()? >= config.updates.auto_update_interval {
             update_cache(
                 &mut cache,
-                &config.updates.archive_source,
+                config.updates.archive_source,
                 config.updates.tls_backend,
                 args.quiet,
             )?;