Don't panic for missing highlight theme (close #1299) (#1307)

* No panic when markdown.highlight_theme is missing

* Better error message for missing theme

* Better error messages

Co-authored-by: southerntofu <southerntofu@thunix.net>

components/config/src/config/mod.rs

@@ -16,7 +16,7 @@ use toml::Value as Toml;
 use crate::highlighting::THEME_SET;
 use crate::theme::Theme;
 use errors::{bail, Error, Result};
-use utils::fs::read_file_with_error;
+use utils::fs::read_file;
 
 // We want a default base url for tests
 static DEFAULT_BASE_URL: &str = "http://a-website.com";
@@ -124,8 +124,9 @@ impl Config {
             bail!("A base URL is required in config.toml with key `base_url`");
         }
 
-        if !THEME_SET.themes.contains_key(&config.highlight_theme) {
-            bail!("Highlight theme {} not available", config.highlight_theme)
+        let highlight_theme = config.highlight_theme();
+        if !THEME_SET.themes.contains_key(highlight_theme) {
+            bail!("Highlight theme {} defined in config does not exist.", highlight_theme);
         }
 
         if config.languages.iter().any(|l| l.code == config.default_language) {
@@ -169,11 +170,8 @@ impl Config {
     /// Parses a config file from the given path
     pub fn from_file<P: AsRef<Path>>(path: P) -> Result<Config> {
         let path = path.as_ref();
-        let file_name = path.file_name().unwrap();
-        let content = read_file_with_error(
-            path,
-            &format!("No `{:?}` file found. Are you in the right directory?", file_name),
-        )?;
+        let content = read_file(path)
+            .map_err(|e| errors::Error::chain("Failed to load config", e))?;
         Config::parse(&content)
     }
 
@@ -270,8 +268,8 @@ impl Config {
 
     /// Parse the theme.toml file and merges the extra data from the theme
     /// with the config extra data
-    pub fn merge_with_theme(&mut self, path: &PathBuf) -> Result<()> {
-        let theme = Theme::from_file(path)?;
+    pub fn merge_with_theme(&mut self, path: &PathBuf, theme_name: &str) -> Result<()> {
+        let theme = Theme::from_file(path, theme_name)?;
         self.add_theme_extra(&theme)
     }
 

components/config/src/lib.rs

@@ -4,18 +4,12 @@ mod theme;
 pub use crate::config::{
     languages::Language, link_checker::LinkChecker, slugify::Slugify, taxonomies::Taxonomy, Config,
 };
+use errors::Result;
 
 use std::path::Path;
 
 /// Get and parse the config.
 /// If it doesn't succeed, exit
-pub fn get_config(filename: &Path) -> Config {
-    match Config::from_file(filename) {
-        Ok(c) => c,
-        Err(e) => {
-            println!("Failed to load {}", filename.display());
-            println!("Error: {}", e);
-            ::std::process::exit(1);
-        }
-    }
+pub fn get_config(filename: &Path) -> Result<Config> {
+    Config::from_file(filename)
 }

components/config/src/theme.rs

@@ -5,7 +5,7 @@ use serde_derive::{Deserialize, Serialize};
 use toml::Value as Toml;
 
 use errors::{bail, Result};
-use utils::fs::read_file_with_error;
+use utils::fs::read_file;
 
 /// Holds the data from a `theme.toml` file.
 /// There are other fields than `extra` in it but Zola
@@ -39,13 +39,9 @@ impl Theme {
     }
 
     /// Parses a theme file from the given path
-    pub fn from_file(path: &PathBuf) -> Result<Theme> {
-        let content = read_file_with_error(
-            path,
-            "No `theme.toml` file found. \
-             Is the `theme` defined in your `config.toml` present in the `themes` directory \
-             and does it have a `theme.toml` inside?",
-        )?;
+    pub fn from_file(path: &PathBuf, theme_name: &str) -> Result<Theme> {
+        let content = read_file(path)
+            .map_err(|e| errors::Error::chain(format!("Failed to load theme {}", theme_name), e))?;
         Theme::parse(&content)
     }
 }

components/site/src/lib.rs

@@ -72,12 +72,12 @@ impl Site {
     pub fn new<P: AsRef<Path>, P2: AsRef<Path>>(path: P, config_file: P2) -> Result<Site> {
         let path = path.as_ref();
         let config_file = config_file.as_ref();
-        let mut config = get_config(config_file);
+        let mut config = get_config(config_file)?;
         config.load_extra_syntaxes(path)?;
 
         if let Some(theme) = config.theme.clone() {
             // Grab data from the extra section of the theme
-            config.merge_with_theme(&path.join("themes").join(&theme).join("theme.toml"))?;
+            config.merge_with_theme(&path.join("themes").join(&theme).join("theme.toml"), &theme)?;
         }
 
         let tera = tpls::load_tera(path, &config)?;

components/utils/src/fs.rs

@@ -49,7 +49,7 @@ pub fn create_directory(path: &Path) -> Result<()> {
 pub fn read_file(path: &Path) -> Result<String> {
     let mut content = String::new();
     File::open(path)
-        .map_err(|e| Error::chain(format!("Failed to open '{:?}'", path.display()), e))?
+        .map_err(|e| Error::chain(format!("Failed to open '{}'", path.display()), e))?
         .read_to_string(&mut content)?;
 
     // Remove utf-8 BOM if any.
@@ -60,19 +60,6 @@ pub fn read_file(path: &Path) -> Result<String> {
     Ok(content)
 }
 
-/// Return the content of a file, with error handling added.
-/// The default error message is overwritten by the message given.
-/// That means it is allocating 2 strings, oh well
-pub fn read_file_with_error(path: &Path, message: &str) -> Result<String> {
-    let res = read_file(&path);
-    if res.is_ok() {
-        return res;
-    }
-    let mut err = Error::msg(message);
-    err.source = res.unwrap_err().source;
-    Err(err)
-}
-
 /// Looks into the current folder for the path and see if there's anything that is not a .md
 /// file. Those will be copied next to the rendered .html file
 pub fn find_related_assets(path: &Path) -> Vec<PathBuf> {