From ee740aceff6257964a6f517a815b70ab9c754eea Mon Sep 17 00:00:00 2001 From: Ruin0x11 Date: Fri, 28 Aug 2020 12:26:08 -0700 Subject: [PATCH] Remove 'default' property on languages, use book.language instead --- src/book/book.rs | 2 +- src/book/init.rs | 17 ++++---- src/cmd/build.rs | 2 +- src/cmd/clean.rs | 2 +- src/cmd/serve.rs | 4 +- src/cmd/test.rs | 2 +- src/cmd/watch.rs | 2 +- src/config.rs | 100 +++++++++++++++++++++++++++-------------------- tests/init.rs | 18 +++++++-- 9 files changed, 89 insertions(+), 60 deletions(-) diff --git a/src/book/book.rs b/src/book/book.rs index af23e785..fa65fcb5 100644 --- a/src/book/book.rs +++ b/src/book/book.rs @@ -15,7 +15,7 @@ pub fn load_book>( cfg: &Config, build_opts: &BuildOpts, ) -> Result { - if cfg.language.has_localized_dir_structure() { + if cfg.has_localized_dir_structure() { match build_opts.language_ident { // Build a single book's translation. Some(_) => Ok(LoadedBook::Single(load_single_book_translation( diff --git a/src/book/init.rs b/src/book/init.rs index 3f489cdd..26eb6d7d 100644 --- a/src/book/init.rs +++ b/src/book/init.rs @@ -14,18 +14,18 @@ pub struct BookBuilder { create_gitignore: bool, config: Config, copy_theme: bool, - language_ident: String + language_ident: String, } fn add_default_language(cfg: &mut Config, language_ident: String) { let language = Language { name: String::from("English"), - default: true, title: None, authors: None, description: None, }; - cfg.language.0.insert(language_ident, language); + cfg.language.0.insert(language_ident.clone(), language); + cfg.book.language = Some(language_ident); } impl BookBuilder { @@ -41,13 +41,16 @@ impl BookBuilder { create_gitignore: false, config: cfg, copy_theme: false, - language_ident: language_ident + language_ident: language_ident, } } /// Get the output source directory of the builder. pub fn source_dir(&self) -> PathBuf { - let src = self.config.get_localized_src_path(Some(&self.language_ident)).unwrap(); + let src = self + .config + .get_localized_src_path(Some(&self.language_ident)) + .unwrap(); self.root.join(src) } @@ -125,8 +128,8 @@ impl BookBuilder { File::create(book_toml) .with_context(|| "Couldn't create book.toml")? - .write_all(&cfg) - .with_context(|| "Unable to write config to book.toml")?; + .write_all(&cfg) + .with_context(|| "Unable to write config to book.toml")?; Ok(()) } diff --git a/src/cmd/build.rs b/src/cmd/build.rs index 29ddacc9..25116cb9 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -19,7 +19,7 @@ pub fn make_subcommand<'a, 'b>() -> App<'a, 'b> { .arg_from_usage("-o, --open 'Opens the compiled book in a web browser'") .arg_from_usage( "-l, --language=[language] 'Language to render the compiled book in.{n}\ - Only valid if the [languages] table in the config is not empty.{n}\ + Only valid if the [language] table in the config is not empty.{n}\ If omitted, builds all translations and provides a menu in the generated output for switching between them.'", ) } diff --git a/src/cmd/clean.rs b/src/cmd/clean.rs index 98fdb505..4884146b 100644 --- a/src/cmd/clean.rs +++ b/src/cmd/clean.rs @@ -20,7 +20,7 @@ pub fn make_subcommand<'a, 'b>() -> App<'a, 'b> { ) .arg_from_usage( "-l, --language=[language] 'Language to render the compiled book in.{n}\ - Only valid if the [languages] table in the config is not empty.{n}\ + Only valid if the [language] table in the config is not empty.{n}\ If omitted, builds all translations and provides a menu in the generated output for switching between them.'", ) } diff --git a/src/cmd/serve.rs b/src/cmd/serve.rs index d768cadb..58f18acc 100644 --- a/src/cmd/serve.rs +++ b/src/cmd/serve.rs @@ -52,7 +52,7 @@ pub fn make_subcommand<'a, 'b>() -> App<'a, 'b> { .arg_from_usage("-o, --open 'Opens the book server in a web browser'") .arg_from_usage( "-l, --language=[language] 'Language to render the compiled book in.{n}\ - Only valid if the [languages] table in the config is not empty.{n}\ + Only valid if the [language] table in the config is not empty.{n}\ If omitted, builds all translations and provides a menu in the generated output for switching between them.'", ) } @@ -86,7 +86,7 @@ pub fn execute(args: &ArgMatches) -> Result<()> { let language: Option = match build_opts.language_ident { // index.html will be at the root directory. Some(_) => None, - None => match book.config.language.default_language() { + None => match book.config.default_language() { // If book has translations, index.html will be under src/en/ or // similar. Some(lang_ident) => Some(lang_ident.clone()), diff --git a/src/cmd/test.rs b/src/cmd/test.rs index 0295547f..642b4d20 100644 --- a/src/cmd/test.rs +++ b/src/cmd/test.rs @@ -26,7 +26,7 @@ pub fn make_subcommand<'a, 'b>() -> App<'a, 'b> { .empty_values(false) .help("A comma-separated list of directories to add to {n}the crate search path when building tests")) .arg_from_usage("-l, --language=[language] 'Language to render the compiled book in.{n}\ - Only valid if the [languages] table in the config is not empty.{n}\ + Only valid if the [language] table in the config is not empty.{n}\ If omitted, builds all translations and provides a menu in the generated output for switching between them.'") } diff --git a/src/cmd/watch.rs b/src/cmd/watch.rs index d7841311..aa2f0c65 100644 --- a/src/cmd/watch.rs +++ b/src/cmd/watch.rs @@ -25,7 +25,7 @@ pub fn make_subcommand<'a, 'b>() -> App<'a, 'b> { .arg_from_usage("-o, --open 'Open the compiled book in a web browser'") .arg_from_usage( "-l, --language=[language] 'Language to render the compiled book in.{n}\ - Only valid if the [languages] table in the config is not empty.{n}\ + Only valid if the [language] table in the config is not empty.{n}\ If omitted, builds all translations and provides a menu in the generated output for switching between them.'", ) } diff --git a/src/config.rs b/src/config.rs index 2341cc06..d1a4f68c 100644 --- a/src/config.rs +++ b/src/config.rs @@ -255,7 +255,7 @@ impl Config { /// Gets the language configured for a book. pub fn get_language>(&self, index: Option) -> Result> { - match self.language.default_language() { + match self.default_language() { // Languages have been specified, assume directory structure with // language subfolders. Some(ref default) => match index { @@ -342,7 +342,7 @@ impl Config { /// missing in a localization, any links to them will gracefully degrade to /// the files that exist in this directory. pub fn get_fallback_src_path(&self) -> PathBuf { - match self.language.default_language() { + match self.default_language() { // Languages have been specified, assume directory structure with // language subfolders. Some(default) => { @@ -358,6 +358,31 @@ impl Config { } } + /// If true, mdBook should assume there are subdirectories under src/ + /// corresponding to the localizations in the config. If false, src/ is a + /// single directory containing the summary file and the rest. + pub fn has_localized_dir_structure(&self) -> bool { + !self.language.0.is_empty() + } + + /// Obtains the default language for this config. + pub fn default_language(&self) -> Option { + if self.has_localized_dir_structure() { + let language_ident = self + .book + .language + .clone() + .expect("Config has [language] table, but `book.language` not was declared"); + self.language.0.get(&language_ident).expect(&format!( + "Expected [language.{}] to be declared in book.toml", + language_ident + )); + Some(language_ident) + } else { + None + } + } + fn from_legacy(mut table: Value) -> Config { let mut cfg = Config::default(); @@ -455,12 +480,28 @@ impl<'de> Deserialize<'de> for Config { if !language.0.is_empty() { let default_languages = language.0.iter().filter(|(_, lang)| lang.default).count(); - - if default_languages != 1 { + if default_languages != 1 { return Err(D::Error::custom( - "If [languages] table is specified, exactly one entry must be set as 'default'", + "If the [language] table is specified, then `book.language` must be declared", )); } + let language_ident = book.language.clone().unwrap(); + if language.0.get(&language_ident).is_none() { + use serde::de::Error; + return Err(D::Error::custom(format!( + "Expected [language.{}] to be declared in book.toml", + language_ident + ))); + } + for (ident, language) in language.0.iter() { + if language.name.is_empty() { + use serde::de::Error; + return Err(D::Error::custom(format!( + "`name` property for [language.{}] must be non-empty", + ident + ))); + } + } } Ok(Config { @@ -492,7 +533,8 @@ impl Serialize for Config { } if !self.language.0.is_empty() { - let language_config = Value::try_from(&self.language).expect("should always be serializable"); + let language_config = + Value::try_from(&self.language).expect("should always be serializable"); table.insert("language", language_config); } @@ -543,9 +585,7 @@ pub struct BookConfig { pub description: Option, /// Location of the book source relative to the book's root directory. pub src: PathBuf, - /// Does this book support more than one language? (Deprecated.) - pub multilingual: bool, - /// The main language of the book. (Deprecated.) + /// The main language of the book. pub language: Option, } @@ -556,7 +596,6 @@ impl Default for BookConfig { authors: Vec::new(), description: None, src: PathBuf::from("src"), - multilingual: true, language: Some(String::from("en")), } } @@ -830,9 +869,6 @@ pub struct LanguageConfig(pub HashMap); pub struct Language { /// Human-readable name of the language. pub name: String, - /// If true, this language is the default. There must be exactly one default - /// language in the config. - pub default: bool, /// Localized title of the book. pub title: Option, /// The authors of the translation. @@ -841,22 +877,7 @@ pub struct Language { pub description: Option, } -impl LanguageConfig { - /// If true, mdBook should assume there are subdirectories under src/ - /// corresponding to the localizations in the config. If false, src/ is a - /// single directory containing the summary file and the rest. - pub fn has_localized_dir_structure(&self) -> bool { - self.default_language().is_some() - } - - /// Returns the default language specified in the config. - pub fn default_language(&self) -> Option<&String> { - self.0 - .iter() - .find(|(_, lang)| lang.default) - .map(|(lang_ident, _)| lang_ident) - } -} +impl LanguageConfig {} /// Allows you to "update" any arbitrary field in a struct by round-tripping via /// a `toml::Value`. @@ -923,7 +944,6 @@ mod tests { [language.en] name = "English" - default = true [language.ja] name = "日本語" @@ -940,7 +960,6 @@ mod tests { title: Some(String::from("Some Book")), authors: vec![String::from("Michael-F-Bryan ")], description: Some(String::from("A completely useless book")), - multilingual: true, src: PathBuf::from("source"), language: Some(String::from("ja")), }; @@ -981,7 +1000,6 @@ mod tests { String::from("en"), Language { name: String::from("English"), - default: true, title: None, description: None, authors: None, @@ -991,7 +1009,6 @@ mod tests { String::from("ja"), Language { name: String::from("日本語"), - default: false, title: Some(String::from("なんかの本")), description: Some(String::from("何の役にも立たない本")), authors: Some(vec![String::from("Ruin0x11")]), @@ -1354,10 +1371,10 @@ mod tests { #[test] #[should_panic(expected = "Invalid configuration file")] - fn validate_default_language() { + fn validate_config_default_language_must_exist_in_languages_table() { let src = r#" - [language.en] - name = "English" + [language.ja] + name = "日本語" "#; Config::from_str(src).unwrap(); @@ -1365,15 +1382,12 @@ mod tests { #[test] #[should_panic(expected = "Invalid configuration file")] - fn validate_default_language_2() { + fn validate_language_config_must_have_name() { let src = r#" - [language.en] - name = "English" - default = true + [book] + language = "en" - [language.fr] - name = "Français" - default = true + [language.en] "#; Config::from_str(src).unwrap(); diff --git a/tests/init.rs b/tests/init.rs index 454f3bf0..c1b3f9ea 100644 --- a/tests/init.rs +++ b/tests/init.rs @@ -10,7 +10,13 @@ use tempfile::Builder as TempFileBuilder; /// are created. #[test] fn base_mdbook_init_should_create_default_content() { - let created_files = vec!["book", "src", "src/en", "src/en/SUMMARY.md", "src/en/chapter_1.md"]; + let created_files = vec![ + "book", + "src", + "src/en", + "src/en/SUMMARY.md", + "src/en/chapter_1.md", + ]; let temp = TempFileBuilder::new().prefix("mdbook").tempdir().unwrap(); for file in &created_files { @@ -28,7 +34,7 @@ fn base_mdbook_init_should_create_default_content() { let contents = fs::read_to_string(temp.path().join("book.toml")).unwrap(); assert_eq!( contents, - "[book]\nauthors = []\nlanguage = \"en\"\nmultilingual = true\nsrc = \"src\"\n[language.en]\ndefault = true\nname = \"English\"\n" + "[book]\nauthors = []\nlanguage = \"en\"\nsrc = \"src\"\n[language.en]\nname = \"English\"\n" ); } @@ -66,7 +72,13 @@ fn run_mdbook_init_should_create_content_from_summary() { /// files, then call `mdbook init`. #[test] fn run_mdbook_init_with_custom_book_and_src_locations() { - let created_files = vec!["out", "in", "in/en", "in/en/SUMMARY.md", "in/en/chapter_1.md"]; + let created_files = vec![ + "out", + "in", + "in/en", + "in/en/SUMMARY.md", + "in/en/chapter_1.md", + ]; let temp = TempFileBuilder::new().prefix("mdbook").tempdir().unwrap(); for file in &created_files {