From c9c5f7a1fd6dcb7d4d791f3ebffe61073c23ff80 Mon Sep 17 00:00:00 2001 From: vcrn <79985743+vcrn@users.noreply.github.com> Date: Sun, 20 Nov 2022 21:28:16 +0100 Subject: [PATCH 1/4] Merge language data in config.toml --- components/config/src/config/languages.rs | 167 ++++++++++++++++++++++ components/config/src/config/markup.rs | 14 +- components/config/src/config/mod.rs | 136 ++++++++++++++++-- 3 files changed, 298 insertions(+), 19 deletions(-) diff --git a/components/config/src/config/languages.rs b/components/config/src/config/languages.rs index f7b33288fa..d247047257 100644 --- a/components/config/src/config/languages.rs +++ b/components/config/src/config/languages.rs @@ -30,6 +30,113 @@ pub struct LanguageOptions { pub translations: HashMap, } +impl LanguageOptions { + /// Merges self with another LanguageOptions, panicking if 2 equivalent fields are not None, + /// empty or of default value. + pub fn merge(&mut self, other: &LanguageOptions) { + match &self.title { + None => self.title = other.title.clone(), + Some(self_title) => match &other.title { + Some(other_title) => panic!( + "Error: `title` for default language is specified twice, as {:?} and {:?}.", + self_title, other_title + ), + None => (), + }, + }; + + match &self.description { + None => self.description = other.description.clone(), + Some(self_description) => match &other.description { + Some(other_description) => panic!("Error: `description` for default language is specified twice, as {:?} and {:?}.", self_description, other_description), + None => (), + }, + }; + + self.generate_feed = self.generate_feed || other.generate_feed; + + match &self.feed_filename == "atom.xml" { + // "atom.xml" is default value. + true => self.feed_filename = other.feed_filename.clone(), + false => match &other.feed_filename.is_empty() { + false => panic!("Error: `feed filename` for default language is specifiec twice, as {:?} and {:?}.", self.feed_filename, other.feed_filename), + true => (), + }, + }; + + match &self.taxonomies.is_empty() { + true => self.taxonomies = other.taxonomies.clone(), + false => match &other.taxonomies.is_empty() { + false => panic!("Error: `taxonomies` for default language is specifiec twice, as {:?} and {:?}.", self.taxonomies, other.taxonomies), + true => (), + }, + }; + + self.build_search_index = self.build_search_index || other.build_search_index; + + match self.search == search::Search::default() { + true => self.search = other.search.clone(), + false => match self.search == other.search { + true => (), + false => panic!( + "Error: `search` for default language is specified twice, as {:?} and {:?}.", + self.search, other.search + ), + }, + }; + + match &self.translations.is_empty() { + true => self.translations = other.translations.clone(), + false => match &other.translations.is_empty() { + true => (), + false => panic!("Error: `translations` for default language is specified twice, as {:?} and {:?}.", self.translations, other.translations), + }, + }; + } + + /// Checks whether self is of default value. + pub fn is_default(&self) -> bool { + match &self.title { + Some(_) => return false, + None => (), + }; + + match &self.description { + Some(_) => return false, + None => (), + }; + + if self.generate_feed { + return false; + } + + if &self.feed_filename == "atom.xml" { + return false; + } + + match &self.taxonomies.is_empty() { + false => return false, + true => (), + }; + + if self.build_search_index { + return false; + } + + match self.search == search::Search::default() { + false => return false, + true => (), + }; + + match &self.translations.is_empty() { + false => return false, + true => (), + }; + + true + } +} + /// We want to ensure the language codes are valid ones pub fn validate_code(code: &str) -> Result<()> { if LanguageIdentifier::from_bytes(code.as_bytes()).is_err() { @@ -38,3 +145,63 @@ pub fn validate_code(code: &str) -> Result<()> { Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn merge_without_conflict() { + let mut base_default_language_options = LanguageOptions { + title: Some("Site's title".to_string()), + description: None, + generate_feed: true, + feed_filename: "atom.xml".to_string(), + taxonomies: vec![], + build_search_index: true, + search: search::Search::default(), + translations: HashMap::new(), + }; + + let section_default_language_options = LanguageOptions { + title: None, + description: Some("Site's description".to_string()), + generate_feed: false, + feed_filename: "rss.xml".to_string(), + taxonomies: vec![], + build_search_index: true, + search: search::Search::default(), + translations: HashMap::new(), + }; + + base_default_language_options.merge(§ion_default_language_options); + } + + #[test] + #[should_panic] + fn merge_with_conflict() { + let mut base_default_language_options = LanguageOptions { + title: Some("Site's title".to_string()), + description: Some("Duplicate site description".to_string()), + generate_feed: true, + feed_filename: "".to_string(), + taxonomies: vec![], + build_search_index: true, + search: search::Search::default(), + translations: HashMap::new(), + }; + + let section_default_language_options = LanguageOptions { + title: None, + description: Some("Site's description".to_string()), + generate_feed: false, + feed_filename: "Some feed_filename".to_string(), + taxonomies: vec![], + build_search_index: true, + search: search::Search::default(), + translations: HashMap::new(), + }; + + base_default_language_options.merge(§ion_default_language_options); + } +} diff --git a/components/config/src/config/markup.rs b/components/config/src/config/markup.rs index 9b80ed8935..7cd03520c8 100644 --- a/components/config/src/config/markup.rs +++ b/components/config/src/config/markup.rs @@ -130,8 +130,11 @@ impl Markdown { ) } } else { - bail!("Highlight theme {} not available.\n\ - You can load custom themes by configuring `extra_syntaxes_and_themes` to include a list of folders containing '.tmTheme' files", self.highlight_theme) + bail!( + "Highlight theme {} not available.\n\ + You can load custom themes by configuring `extra_syntaxes_and_themes` to include a list of folders containing '.tmTheme' files", + self.highlight_theme + ) } } @@ -142,8 +145,11 @@ impl Markdown { // Check extra themes if let Some(extra) = &*self.extra_theme_set { if !extra.themes.contains_key(theme_name) { - bail!("Can't export highlight theme {}, as it does not exist.\n\ - Make sure it's spelled correctly, or your custom .tmTheme' is defined properly.", theme_name) + bail!( + "Can't export highlight theme {}, as it does not exist.\n\ + Make sure it's spelled correctly, or your custom .tmTheme' is defined properly.", + theme_name + ) } } } diff --git a/components/config/src/config/mod.rs b/components/config/src/config/mod.rs index b41ccc27fb..d2881980a7 100644 --- a/components/config/src/config/mod.rs +++ b/components/config/src/config/mod.rs @@ -203,25 +203,36 @@ impl Config { } } - /// Adds the default language to the list of languages if not present + /// Adds the default language to the list of languages if options for it are specified at base level of config.toml. + /// If section for the same language also exists, the options at this section and base are merged and then adds it + /// to list. pub fn add_default_language(&mut self) { // We automatically insert a language option for the default language *if* it isn't present // TODO: what to do if there is like an empty dict for the lang? merge it or use the language // TODO: as source of truth? - if !self.languages.contains_key(&self.default_language) { - self.languages.insert( - self.default_language.clone(), - languages::LanguageOptions { - title: self.title.clone(), - description: self.description.clone(), - generate_feed: self.generate_feed, - feed_filename: self.feed_filename.clone(), - build_search_index: self.build_search_index, - taxonomies: self.taxonomies.clone(), - search: self.search.clone(), - translations: self.translations.clone(), - }, - ); + + let mut base_default_language_options = languages::LanguageOptions { + title: self.title.clone(), + description: self.description.clone(), + generate_feed: self.generate_feed, + feed_filename: self.feed_filename.clone(), + build_search_index: self.build_search_index, + taxonomies: self.taxonomies.clone(), + search: self.search.clone(), + translations: self.translations.clone(), + }; + + if !base_default_language_options.is_default() { + if self.languages.contains_key(&self.default_language) { + println!("Warning: config.toml contains both default language specific information at base and under section `[languages.{}]`, \ + which may cause merge conflicts. Please use only one to specify language specific information", self.default_language); + + let section_default_language_options = + self.languages.get(&self.default_language).unwrap().clone(); + + base_default_language_options.merge(§ion_default_language_options); + } + self.languages.insert(self.default_language.clone(), base_default_language_options); } } @@ -385,6 +396,101 @@ mod tests { use super::*; use utils::slugs::SlugifyStrategy; + #[test] + fn add_default_language_skips_insert() { + let title = Some("A title".to_string()); + let description = Some("A description".to_string()); + + let mut config = Config::default(); + config.languages.insert( + config.default_language.clone(), + languages::LanguageOptions { + title: title.clone(), + description: description.clone(), + generate_feed: true, + feed_filename: config.feed_filename.clone(), + taxonomies: config.taxonomies.clone(), + build_search_index: false, + search: search::Search::default(), + translations: config.translations.clone(), + }, + ); + config.add_default_language(); + + let default_language_options = + config.languages.get(&config.default_language).unwrap().clone(); + assert_eq!(default_language_options.title, title); + assert_eq!(default_language_options.description, description); + } + + #[test] + #[should_panic] + fn add_default_language_causes_panic() { + let title = Some("A title".to_string()); + let title_2 = Some("Another title".to_string()); + + let mut config = Config::default(); + config.title = title.clone(); + config.languages.insert( + config.default_language.clone(), + languages::LanguageOptions { + title: title_2.clone(), + description: None, + generate_feed: true, + feed_filename: config.feed_filename.clone(), + taxonomies: config.taxonomies.clone(), + build_search_index: false, + search: search::Search::default(), + translations: config.translations.clone(), + }, + ); + config.add_default_language(); + } + + #[test] + fn can_add_default_language_with_merge() { + let title = Some("A title".to_string()); + let description = Some("A description".to_string()); + + let mut config = Config::default(); + config.title = title.clone(); + config.languages.insert( + config.default_language.clone(), + languages::LanguageOptions { + title: None, + description: description.clone(), + generate_feed: true, + feed_filename: config.feed_filename.clone(), + taxonomies: config.taxonomies.clone(), + build_search_index: false, + search: search::Search::default(), + translations: config.translations.clone(), + }, + ); + config.add_default_language(); + + let default_language_options = + config.languages.get(&config.default_language).unwrap().clone(); + assert_eq!(default_language_options.title, title); + assert_eq!(default_language_options.description, description); + } + + #[test] + fn can_add_default_language_without_merge() { + let title = Some("A title".to_string()); + let description = Some("A description".to_string()); + + let mut config = Config::default(); + config.title = title.clone(); + config.description = description.clone(); + config.add_default_language(); + + let default_language_options = + config.languages.get(&config.default_language).unwrap().clone(); + assert_eq!(default_language_options.title, title); + assert_eq!(default_language_options.description, description); + } + #[test] fn can_import_valid_config() { let config = r#" From 6e5a4e54862b5bcaa65ef547a60370447af2e81b Mon Sep 17 00:00:00 2001 From: vcrn <79985743+vcrn@users.noreply.github.com> Date: Sun, 4 Dec 2022 20:58:03 +0100 Subject: [PATCH 2/4] Fix feedback on merging default lang data --- components/config/src/config/languages.rs | 90 +++++++++-------------- components/config/src/config/mod.rs | 60 +++++---------- 2 files changed, 52 insertions(+), 98 deletions(-) diff --git a/components/config/src/config/languages.rs b/components/config/src/config/languages.rs index d247047257..3500d8dc18 100644 --- a/components/config/src/config/languages.rs +++ b/components/config/src/config/languages.rs @@ -33,13 +33,14 @@ pub struct LanguageOptions { impl LanguageOptions { /// Merges self with another LanguageOptions, panicking if 2 equivalent fields are not None, /// empty or of default value. - pub fn merge(&mut self, other: &LanguageOptions) { + pub fn merge(&mut self, other: &LanguageOptions) -> Result<()> { match &self.title { None => self.title = other.title.clone(), Some(self_title) => match &other.title { - Some(other_title) => panic!( - "Error: `title` for default language is specified twice, as {:?} and {:?}.", - self_title, other_title + Some(other_title) => bail!( + "`title` for default language is specified twice, as {:?} and {:?}.", + self_title, + other_title ), None => (), }, @@ -48,7 +49,11 @@ impl LanguageOptions { match &self.description { None => self.description = other.description.clone(), Some(self_description) => match &other.description { - Some(other_description) => panic!("Error: `description` for default language is specified twice, as {:?} and {:?}.", self_description, other_description), + Some(other_description) => bail!( + "`description` for default language is specified twice, as {:?} and {:?}.", + self_description, + other_description + ), None => (), }, }; @@ -59,7 +64,11 @@ impl LanguageOptions { // "atom.xml" is default value. true => self.feed_filename = other.feed_filename.clone(), false => match &other.feed_filename.is_empty() { - false => panic!("Error: `feed filename` for default language is specifiec twice, as {:?} and {:?}.", self.feed_filename, other.feed_filename), + false => bail!( + "`feed filename` for default language is specifiec twice, as {:?} and {:?}.", + self.feed_filename, + other.feed_filename + ), true => (), }, }; @@ -67,7 +76,11 @@ impl LanguageOptions { match &self.taxonomies.is_empty() { true => self.taxonomies = other.taxonomies.clone(), false => match &other.taxonomies.is_empty() { - false => panic!("Error: `taxonomies` for default language is specifiec twice, as {:?} and {:?}.", self.taxonomies, other.taxonomies), + false => bail!( + "`taxonomies` for default language is specifiec twice, as {:?} and {:?}.", + self.taxonomies, + other.taxonomies + ), true => (), }, }; @@ -77,63 +90,28 @@ impl LanguageOptions { match self.search == search::Search::default() { true => self.search = other.search.clone(), false => match self.search == other.search { - true => (), - false => panic!( - "Error: `search` for default language is specified twice, as {:?} and {:?}.", - self.search, other.search + false => bail!( + "`search` for default language is specified twice, as {:?} and {:?}.", + self.search, + other.search ), + true => (), }, }; match &self.translations.is_empty() { true => self.translations = other.translations.clone(), false => match &other.translations.is_empty() { + false => bail!( + "`translations` for default language is specified twice, as {:?} and {:?}.", + self.translations, + other.translations + ), true => (), - false => panic!("Error: `translations` for default language is specified twice, as {:?} and {:?}.", self.translations, other.translations), }, }; - } - - /// Checks whether self is of default value. - pub fn is_default(&self) -> bool { - match &self.title { - Some(_) => return false, - None => (), - }; - - match &self.description { - Some(_) => return false, - None => (), - }; - - if self.generate_feed { - return false; - } - - if &self.feed_filename == "atom.xml" { - return false; - } - - match &self.taxonomies.is_empty() { - false => return false, - true => (), - }; - - if self.build_search_index { - return false; - } - - match self.search == search::Search::default() { - false => return false, - true => (), - }; - - match &self.translations.is_empty() { - false => return false, - true => (), - }; - true + Ok(()) } } @@ -174,7 +152,7 @@ mod tests { translations: HashMap::new(), }; - base_default_language_options.merge(§ion_default_language_options); + base_default_language_options.merge(§ion_default_language_options).unwrap(); } #[test] @@ -202,6 +180,8 @@ mod tests { translations: HashMap::new(), }; - base_default_language_options.merge(§ion_default_language_options); + base_default_language_options + .merge(§ion_default_language_options) + .expect("This should lead to panic"); } } diff --git a/components/config/src/config/mod.rs b/components/config/src/config/mod.rs index 704a9250cb..2111ad9336 100644 --- a/components/config/src/config/mod.rs +++ b/components/config/src/config/mod.rs @@ -126,7 +126,7 @@ impl Config { languages::validate_code(code)?; } - config.add_default_language(); + config.add_default_language()?; config.slugify_taxonomies(); if !config.ignored_content.is_empty() { @@ -152,7 +152,7 @@ impl Config { pub fn default_for_test() -> Self { let mut config = Config::default(); - config.add_default_language(); + config.add_default_language().unwrap(); config.slugify_taxonomies(); config } @@ -208,7 +208,7 @@ impl Config { /// Adds the default language to the list of languages if options for it are specified at base level of config.toml. /// If section for the same language also exists, the options at this section and base are merged and then adds it /// to list. - pub fn add_default_language(&mut self) { + pub fn add_default_language(&mut self) -> Result<()> { // We automatically insert a language option for the default language *if* it isn't present // TODO: what to do if there is like an empty dict for the lang? merge it or use the language // TODO: as source of truth? @@ -224,18 +224,18 @@ impl Config { translations: self.translations.clone(), }; - if !base_default_language_options.is_default() { - if self.languages.contains_key(&self.default_language) { + if let Some(section_default_language_options) = self.languages.get(&self.default_language) { + if base_default_language_options != languages::LanguageOptions::default() { println!("Warning: config.toml contains both default language specific information at base and under section `[languages.{}]`, \ which may cause merge conflicts. Please use only one to specify language specific information", self.default_language); - - let section_default_language_options = - self.languages.get(&self.default_language).unwrap().clone(); - - base_default_language_options.merge(§ion_default_language_options); + base_default_language_options.merge(§ion_default_language_options)?; + } else { + return Ok(()); } - self.languages.insert(self.default_language.clone(), base_default_language_options); } + self.languages.insert(self.default_language.clone(), base_default_language_options); + + Ok(()) } /// Merges the extra data from the theme with the config extra data @@ -399,33 +399,6 @@ mod tests { use super::*; use utils::slugs::SlugifyStrategy; - #[test] - fn add_default_language_skips_insert() { - let title = Some("A title".to_string()); - let description = Some("A description".to_string()); - - let mut config = Config::default(); - config.languages.insert( - config.default_language.clone(), - languages::LanguageOptions { - title: title.clone(), - description: description.clone(), - generate_feed: true, - feed_filename: config.feed_filename.clone(), - taxonomies: config.taxonomies.clone(), - build_search_index: false, - search: search::Search::default(), - translations: config.translations.clone(), - }, - ); - config.add_default_language(); - - let default_language_options = - config.languages.get(&config.default_language).unwrap().clone(); - assert_eq!(default_language_options.title, title); - assert_eq!(default_language_options.description, description); - } - #[test] #[should_panic] fn add_default_language_causes_panic() { @@ -447,11 +420,12 @@ mod tests { translations: config.translations.clone(), }, ); - config.add_default_language(); + let result = config.add_default_language(); + result.expect("Should return an Err and panic."); } #[test] - fn can_add_default_language_with_merge() { + fn can_add_default_language_with_warning() { let title = Some("A title".to_string()); let description = Some("A description".to_string()); @@ -470,7 +444,7 @@ mod tests { translations: config.translations.clone(), }, ); - config.add_default_language(); + config.add_default_language().unwrap(); let default_language_options = config.languages.get(&config.default_language).unwrap().clone(); @@ -479,14 +453,14 @@ mod tests { } #[test] - fn can_add_default_language_without_merge() { + fn can_add_default_language_without_warning() { let title = Some("A title".to_string()); let description = Some("A description".to_string()); let mut config = Config::default(); config.title = title.clone(); config.description = description.clone(); - config.add_default_language(); + config.add_default_language().unwrap(); let default_language_options = config.languages.get(&config.default_language).unwrap().clone(); From d3f60090acd4a12ea094ab5cbcfcf035d68cb84d Mon Sep 17 00:00:00 2001 From: vcrn <79985743+vcrn@users.noreply.github.com> Date: Thu, 8 Dec 2022 21:25:16 +0100 Subject: [PATCH 3/4] Fix clippy warning --- components/config/src/config/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/config/src/config/mod.rs b/components/config/src/config/mod.rs index 2111ad9336..2aff4d71e2 100644 --- a/components/config/src/config/mod.rs +++ b/components/config/src/config/mod.rs @@ -228,7 +228,7 @@ impl Config { if base_default_language_options != languages::LanguageOptions::default() { println!("Warning: config.toml contains both default language specific information at base and under section `[languages.{}]`, \ which may cause merge conflicts. Please use only one to specify language specific information", self.default_language); - base_default_language_options.merge(§ion_default_language_options)?; + base_default_language_options.merge(section_default_language_options)?; } else { return Ok(()); } From 8f38b7710646308af5cd3e0179ee69469456513d Mon Sep 17 00:00:00 2001 From: vcrn <79985743+vcrn@users.noreply.github.com> Date: Sat, 14 Jan 2023 10:09:50 +0100 Subject: [PATCH 4/4] Update test names and test variables --- components/config/src/config/mod.rs | 73 ++++++++++++++--------------- 1 file changed, 36 insertions(+), 37 deletions(-) diff --git a/components/config/src/config/mod.rs b/components/config/src/config/mod.rs index 2aff4d71e2..2b31644f5e 100644 --- a/components/config/src/config/mod.rs +++ b/components/config/src/config/mod.rs @@ -400,18 +400,33 @@ mod tests { use utils::slugs::SlugifyStrategy; #[test] - #[should_panic] - fn add_default_language_causes_panic() { - let title = Some("A title".to_string()); - let title_2 = Some("Another title".to_string()); + fn can_add_default_language_with_data_only_at_base_section() { + let title_base = Some("Base section title".to_string()); + let description_base = Some("Base section description".to_string()); let mut config = Config::default(); - config.title = title.clone(); + config.title = title_base.clone(); + config.description = description_base.clone(); + config.add_default_language().unwrap(); + + let default_language_options = + config.languages.get(&config.default_language).unwrap().clone(); + assert_eq!(default_language_options.title, title_base); + assert_eq!(default_language_options.description, description_base); + } + + #[test] + fn can_add_default_language_with_data_at_base_and_language_section() { + let title_base = Some("Base section title".to_string()); + let description_lang_section = Some("Language section description".to_string()); + + let mut config = Config::default(); + config.title = title_base.clone(); config.languages.insert( config.default_language.clone(), languages::LanguageOptions { - title: title_2.clone(), - description: None, + title: None, + description: description_lang_section.clone(), generate_feed: true, feed_filename: config.feed_filename.clone(), taxonomies: config.taxonomies.clone(), @@ -420,22 +435,26 @@ mod tests { translations: config.translations.clone(), }, ); - let result = config.add_default_language(); - result.expect("Should return an Err and panic."); + config.add_default_language().unwrap(); + + let default_language_options = + config.languages.get(&config.default_language).unwrap().clone(); + assert_eq!(default_language_options.title, title_base); + assert_eq!(default_language_options.description, description_lang_section); } #[test] - fn can_add_default_language_with_warning() { - let title = Some("A title".to_string()); - let description = Some("A description".to_string()); + fn errors_when_same_field_present_at_base_and_language_section() { + let title_base = Some("Base section title".to_string()); + let title_lang_section = Some("Language section title".to_string()); let mut config = Config::default(); - config.title = title.clone(); + config.title = title_base.clone(); config.languages.insert( config.default_language.clone(), languages::LanguageOptions { - title: None, - description: description.clone(), + title: title_lang_section.clone(), + description: None, generate_feed: true, feed_filename: config.feed_filename.clone(), taxonomies: config.taxonomies.clone(), @@ -444,28 +463,8 @@ mod tests { translations: config.translations.clone(), }, ); - config.add_default_language().unwrap(); - - let default_language_options = - config.languages.get(&config.default_language).unwrap().clone(); - assert_eq!(default_language_options.title, title); - assert_eq!(default_language_options.description, description); - } - - #[test] - fn can_add_default_language_without_warning() { - let title = Some("A title".to_string()); - let description = Some("A description".to_string()); - - let mut config = Config::default(); - config.title = title.clone(); - config.description = description.clone(); - config.add_default_language().unwrap(); - - let default_language_options = - config.languages.get(&config.default_language).unwrap().clone(); - assert_eq!(default_language_options.title, title); - assert_eq!(default_language_options.description, description); + let result = config.add_default_language(); + assert!(result.is_err()); } #[test]