From 53456bd052f0a55cdc8ed80a773f567c8023959a Mon Sep 17 00:00:00 2001 From: southerntofu <52931252+southerntofu@users.noreply.github.com> Date: Thu, 4 Feb 2021 08:54:12 +0000 Subject: [PATCH] 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 --- components/config/src/config/mod.rs | 18 ++++++++---------- components/config/src/lib.rs | 12 +++--------- components/config/src/theme.rs | 12 ++++-------- components/site/src/lib.rs | 4 ++-- components/utils/src/fs.rs | 15 +-------------- 5 files changed, 18 insertions(+), 43 deletions(-) diff --git a/components/config/src/config/mod.rs b/components/config/src/config/mod.rs index 415f215b..a8118c78 100644 --- a/components/config/src/config/mod.rs +++ b/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>(path: P) -> Result { 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) } diff --git a/components/config/src/lib.rs b/components/config/src/lib.rs index 529a8b4a..f431f232 100644 --- a/components/config/src/lib.rs +++ b/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::from_file(filename) } diff --git a/components/config/src/theme.rs b/components/config/src/theme.rs index 6c68401a..5022319b 100644 --- a/components/config/src/theme.rs +++ b/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 { - 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 { + let content = read_file(path) + .map_err(|e| errors::Error::chain(format!("Failed to load theme {}", theme_name), e))?; Theme::parse(&content) } } diff --git a/components/site/src/lib.rs b/components/site/src/lib.rs index edeaedbe..48932742 100644 --- a/components/site/src/lib.rs +++ b/components/site/src/lib.rs @@ -72,12 +72,12 @@ impl Site { pub fn new, P2: AsRef>(path: P, config_file: P2) -> Result { 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)?; diff --git a/components/utils/src/fs.rs b/components/utils/src/fs.rs index a46f6d94..afb7a04e 100644 --- a/components/utils/src/fs.rs +++ b/components/utils/src/fs.rs @@ -49,7 +49,7 @@ pub fn create_directory(path: &Path) -> Result<()> { pub fn read_file(path: &Path) -> Result { 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 { 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 { - 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 {