Skip to content

Commit 5090c9e

Browse files
KeatsKeats
committed
Use macro for LanguageOptions::merge
1 parent 2d44fe8 commit 5090c9e

File tree

2 files changed

+65
-90
lines changed

2 files changed

+65
-90
lines changed

‎components/config/src/config/languages.rs

Lines changed: 58 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -31,85 +31,66 @@ pub struct LanguageOptions {
3131
}
3232

3333
impl LanguageOptions {
34-
/// Merges self with another LanguageOptions, panicking if 2 equivalent fields are not None,
35-
/// empty or of default value.
34+
/// Merges self with another LanguageOptions, erroring if 2 equivalent fields are not None,
35+
/// empty or the default value.
3636
pub fn merge(&mut self, other: &LanguageOptions) -> Result<()> {
37-
match &self.title {
38-
None => self.title = other.title.clone(),
39-
Some(self_title) => match &other.title {
40-
Some(other_title) => bail!(
41-
"`title` for default language is specified twice, as {:?} and {:?}.",
42-
self_title,
43-
other_title
44-
),
45-
None => (),
46-
},
47-
};
48-
49-
match &self.description {
50-
None => self.description = other.description.clone(),
51-
Some(self_description) => match &other.description {
52-
Some(other_description) => bail!(
53-
"`description` for default language is specified twice, as {:?} and {:?}.",
54-
self_description,
55-
other_description
56-
),
57-
None => (),
58-
},
59-
};
37+
macro_rules! merge_field {
38+
($orig_field:expr,$other_field:expr,$name:expr) => {
39+
match &$orig_field {
40+
None => $orig_field = $other_field.clone(),
41+
Some(cur_value) => {
42+
if let Some(other_field_value) = &$other_field {
43+
bail!(
44+
"`{}` for default language is specified twice, as {:?} and {:?}.",
45+
$name,
46+
cur_value,
47+
other_field_value
48+
);
49+
}
50+
}
51+
};
52+
};
53+
($cond:expr,$orig_field:expr,$other_field:expr,$name:expr) => {
54+
if $cond {
55+
$orig_field = $other_field.clone();
56+
} else if !$other_field.is_empty() {
57+
bail!(
58+
"`{}` for default language is specified twice, as {:?} and {:?}.",
59+
$name,
60+
$orig_field,
61+
$other_field
62+
)
63+
}
64+
};
65+
}
66+
merge_field!(self.title, other.title, "title");
67+
merge_field!(self.description, other.description, "description");
68+
merge_field!(
69+
self.feed_filename == "atom.xml",
70+
self.feed_filename,
71+
other.feed_filename,
72+
"feed_filename"
73+
);
74+
merge_field!(self.taxonomies.is_empty(), self.taxonomies, other.taxonomies, "taxonomies");
75+
merge_field!(
76+
self.translations.is_empty(),
77+
self.translations,
78+
other.translations,
79+
"translations"
80+
);
6081

6182
self.generate_feed = self.generate_feed || other.generate_feed;
62-
63-
match &self.feed_filename == "atom.xml" {
64-
// "atom.xml" is default value.
65-
true => self.feed_filename = other.feed_filename.clone(),
66-
false => match &other.feed_filename.is_empty() {
67-
false => bail!(
68-
"`feed filename` for default language is specifiec twice, as {:?} and {:?}.",
69-
self.feed_filename,
70-
other.feed_filename
71-
),
72-
true => (),
73-
},
74-
};
75-
76-
match &self.taxonomies.is_empty() {
77-
true => self.taxonomies = other.taxonomies.clone(),
78-
false => match &other.taxonomies.is_empty() {
79-
false => bail!(
80-
"`taxonomies` for default language is specifiec twice, as {:?} and {:?}.",
81-
self.taxonomies,
82-
other.taxonomies
83-
),
84-
true => (),
85-
},
86-
};
87-
8883
self.build_search_index = self.build_search_index || other.build_search_index;
8984

90-
match self.search == search::Search::default() {
91-
true => self.search = other.search.clone(),
92-
false => match self.search == other.search {
93-
false => bail!(
94-
"`search` for default language is specified twice, as {:?} and {:?}.",
95-
self.search,
96-
other.search
97-
),
98-
true => (),
99-
},
100-
};
101-
102-
match &self.translations.is_empty() {
103-
true => self.translations = other.translations.clone(),
104-
false => match &other.translations.is_empty() {
105-
false => bail!(
106-
"`translations` for default language is specified twice, as {:?} and {:?}.",
107-
self.translations,
108-
other.translations
109-
),
110-
true => (),
111-
},
112-
};
85+
if self.search == search::Search::default() {
86+
self.search = other.search.clone();
87+
} else if self.search != other.search {
88+
bail!(
89+
"`search` for default language is specified twice, as {:?} and {:?}.",
90+
self.search,
91+
other.search
92+
);
93+
}
11394

11495
Ok(())
11596
}
@@ -156,7 +137,6 @@ mod tests {
156137
}
157138

158139
#[test]
159-
#[should_panic]
160140
fn merge_with_conflict() {
161141
let mut base_default_language_options = LanguageOptions {
162142
title: Some("Site's title".to_string()),
@@ -180,8 +160,8 @@ mod tests {
180160
translations: HashMap::new(),
181161
};
182162

183-
base_default_language_options
184-
.merge(&section_default_language_options)
185-
.expect("This should lead to panic");
163+
let res =
164+
base_default_language_options.merge(&section_default_language_options).unwrap_err();
165+
assert!(res.to_string().contains("`description` for default language is specified twice"));
186166
}
187167
}

‎components/config/src/config/mod.rs

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -210,11 +210,7 @@ impl Config {
210210
/// If section for the same language also exists, the options at this section and base are merged and then adds it
211211
/// to list.
212212
pub fn add_default_language(&mut self) -> Result<()> {
213-
// We automatically insert a language option for the default language *if* it isn't present
214-
// TODO: what to do if there is like an empty dict for the lang? merge it or use the language
215-
// TODO: as source of truth?
216-
217-
let mut base_default_language_options = languages::LanguageOptions {
213+
let mut base_language_options = languages::LanguageOptions {
218214
title: self.title.clone(),
219215
description: self.description.clone(),
220216
generate_feed: self.generate_feed,
@@ -225,16 +221,15 @@ impl Config {
225221
translations: self.translations.clone(),
226222
};
227223

228-
if let Some(section_default_language_options) = self.languages.get(&self.default_language) {
229-
if base_default_language_options != languages::LanguageOptions::default() {
230-
println!("Warning: config.toml contains both default language specific information at base and under section `[languages.{}]`, \
231-
which may cause merge conflicts. Please use only one to specify language specific information", self.default_language);
232-
base_default_language_options.merge(section_default_language_options)?;
233-
} else {
224+
if let Some(section_language_options) = self.languages.get(&self.default_language) {
225+
if base_language_options == languages::LanguageOptions::default() {
234226
return Ok(());
235227
}
228+
println!("Warning: config.toml contains both default language specific information at base and under section `[languages.{}]`, \
229+
which may cause merge conflicts. Please use only one to specify language specific information", self.default_language);
230+
base_language_options.merge(section_language_options)?;
236231
}
237-
self.languages.insert(self.default_language.clone(), base_default_language_options);
232+
self.languages.insert(self.default_language.clone(), base_language_options);
238233

239234
Ok(())
240235
}

0 commit comments

Comments
 (0)