From 53c3a92285bc930a2cda10d6d6ac0eb7f38665ee Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Mon, 17 Feb 2025 08:20:16 -0800 Subject: [PATCH 1/2] Add test for a chapter with no source path --- tests/rendered_output.rs | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/tests/rendered_output.rs b/tests/rendered_output.rs index 31606220a6..a44b6e147b 100644 --- a/tests/rendered_output.rs +++ b/tests/rendered_output.rs @@ -3,10 +3,11 @@ mod dummy_book; use crate::dummy_book::{assert_contains_strings, assert_doesnt_contain_strings, DummyBook}; use anyhow::Context; +use mdbook::book::Chapter; use mdbook::config::Config; use mdbook::errors::*; use mdbook::utils::fs::write_file; -use mdbook::MDBook; +use mdbook::{BookItem, MDBook}; use pretty_assertions::assert_eq; use select::document::Document; use select::predicate::{Attr, Class, Name, Predicate}; @@ -1031,3 +1032,22 @@ fn custom_header_attributes() { ]; assert_contains_strings(&contents, summary_strings); } + +#[test] +#[should_panic] +fn with_no_source_path() { + // Test for a regression where search would fail if source_path is None. + let temp = DummyBook::new().build().unwrap(); + let mut md = MDBook::load(temp.path()).unwrap(); + let chapter = Chapter { + name: "Sample chapter".to_string(), + content: "".to_string(), + number: None, + sub_items: Vec::new(), + path: Some(PathBuf::from("sample.html")), + source_path: None, + parent_names: Vec::new(), + }; + md.book.sections.push(BookItem::Chapter(chapter)); + md.build().unwrap(); +} From 5777a0edc4af9cd4617ab795c82298913ae685bd Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Mon, 17 Feb 2025 09:41:52 -0800 Subject: [PATCH 2/2] Fix issue with None source_path This fixes an issue where mdbook would panic if a non-draft chapter has a None source_path when generating the search index. The code was assuming that only draft chapters would have that behavior. However, API users can inject synthetic chapters that have no path on disk. This updates it to fall back to the path, or skip if neither is set. --- src/book/book.rs | 3 ++- src/renderer/html_handlebars/search.rs | 20 +++++++++++--------- tests/rendered_output.rs | 1 - 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/book/book.rs b/src/book/book.rs index 5bb4480e7b..c715097284 100644 --- a/src/book/book.rs +++ b/src/book/book.rs @@ -173,7 +173,8 @@ pub struct Chapter { /// `index.md` via the [`Chapter::path`] field. The `source_path` field /// exists if you need access to the true file path. /// - /// This is `None` for a draft chapter. + /// This is `None` for a draft chapter, or a synthetically generated + /// chapter that has no file on disk. pub source_path: Option, /// An ordered list of the names of each chapter above this one in the hierarchy. pub parent_names: Vec, diff --git a/src/renderer/html_handlebars/search.rs b/src/renderer/html_handlebars/search.rs index 9715ce15c1..d21ff6b101 100644 --- a/src/renderer/html_handlebars/search.rs +++ b/src/renderer/html_handlebars/search.rs @@ -43,10 +43,11 @@ pub fn create_files(search_config: &Search, destination: &Path, book: &Book) -> BookItem::Chapter(ch) if !ch.is_draft_chapter() => ch, _ => continue, }; - let chapter_settings = - get_chapter_settings(&chapter_configs, chapter.source_path.as_ref().unwrap()); - if !chapter_settings.enable.unwrap_or(true) { - continue; + if let Some(path) = settings_path(chapter) { + let chapter_settings = get_chapter_settings(&chapter_configs, path); + if !chapter_settings.enable.unwrap_or(true) { + continue; + } } render_item(&mut index, search_config, &mut doc_urls, chapter)?; } @@ -321,6 +322,10 @@ fn clean_html(html: &str) -> String { AMMONIA.clean(html).to_string() } +fn settings_path(ch: &Chapter) -> Option<&Path> { + ch.source_path.as_deref().or_else(|| ch.path.as_deref()) +} + fn validate_chapter_config( chapter_configs: &[(PathBuf, SearchChapterSettings)], book: &Book, @@ -329,13 +334,10 @@ fn validate_chapter_config( let found = book .iter() .filter_map(|item| match item { - BookItem::Chapter(ch) if !ch.is_draft_chapter() => Some(ch), + BookItem::Chapter(ch) if !ch.is_draft_chapter() => settings_path(ch), _ => None, }) - .any(|chapter| { - let ch_path = chapter.source_path.as_ref().unwrap(); - ch_path.starts_with(path) - }); + .any(|source_path| source_path.starts_with(path)); if !found { bail!( "[output.html.search.chapter] key `{}` does not match any chapter paths", diff --git a/tests/rendered_output.rs b/tests/rendered_output.rs index a44b6e147b..c2725c531c 100644 --- a/tests/rendered_output.rs +++ b/tests/rendered_output.rs @@ -1034,7 +1034,6 @@ fn custom_header_attributes() { } #[test] -#[should_panic] fn with_no_source_path() { // Test for a regression where search would fail if source_path is None. let temp = DummyBook::new().build().unwrap();