From 338a9b424ecb670b5ef47abad10657779f4b116e Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Mon, 18 Aug 2025 11:14:29 -0700 Subject: [PATCH] Change with_renderer/with_preprocessor to overwrite This changes `with_renderer` and `with_preprocessor` to replace any extensions of the same name instead of just appending to the list. This is necessary for rust-lang's build process, because we replace the preprocessors with local ones. Previously, mdbook would just print an error, but continue working. With the change that preprocessors are no longer optional by default, it is now required that we have a way to replace the existing entries. --- Cargo.lock | 1 + Cargo.toml | 1 + crates/mdbook-driver/Cargo.toml | 1 + crates/mdbook-driver/src/mdbook.rs | 52 ++++++++++++++---------- crates/mdbook-driver/src/mdbook/tests.rs | 29 ++++--------- tests/testsuite/preprocessor.rs | 15 +++---- tests/testsuite/renderer.rs | 15 ++----- 7 files changed, 51 insertions(+), 63 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fe63883f..2b1daada 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1335,6 +1335,7 @@ name = "mdbook-driver" version = "0.5.0-alpha.1" dependencies = [ "anyhow", + "indexmap", "log", "mdbook-core", "mdbook-html", diff --git a/Cargo.toml b/Cargo.toml index c085d699..dbdfb442 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -37,6 +37,7 @@ font-awesome-as-a-crate = "0.3.0" futures-util = "0.3.31" handlebars = "6.3.2" hex = "0.4.3" +indexmap = "2.10.0" ignore = "0.4.23" log = "0.4.27" mdbook-core = { path = "crates/mdbook-core" } diff --git a/crates/mdbook-driver/Cargo.toml b/crates/mdbook-driver/Cargo.toml index f7edc053..9f7936a4 100644 --- a/crates/mdbook-driver/Cargo.toml +++ b/crates/mdbook-driver/Cargo.toml @@ -9,6 +9,7 @@ rust-version.workspace = true [dependencies] anyhow.workspace = true +indexmap.workspace = true log.workspace = true mdbook-core.workspace = true mdbook-html.workspace = true diff --git a/crates/mdbook-driver/src/mdbook.rs b/crates/mdbook-driver/src/mdbook.rs index 4edc27c6..d173502b 100644 --- a/crates/mdbook-driver/src/mdbook.rs +++ b/crates/mdbook-driver/src/mdbook.rs @@ -5,6 +5,7 @@ use crate::builtin_renderers::{CmdRenderer, MarkdownRenderer}; use crate::init::BookBuilder; use crate::load::{load_book, load_book_from_disk}; use anyhow::{Context, Error, Result, bail}; +use indexmap::IndexMap; use log::{debug, error, info, log_enabled, trace, warn}; use mdbook_core::book::{Book, BookItem, BookItems}; use mdbook_core::config::{Config, RustEdition}; @@ -28,14 +29,18 @@ mod tests; pub struct MDBook { /// The book's root directory. pub root: PathBuf, + /// The configuration used to tweak now a book is built. pub config: Config, + /// A representation of the book's contents in memory. pub book: Book, - renderers: Vec>, - /// List of pre-processors to be run on the book. - preprocessors: Vec>, + /// Renderers to execute. + renderers: IndexMap>, + + /// Pre-processors to be run on the book. + preprocessors: IndexMap>, } impl MDBook { @@ -156,7 +161,7 @@ impl MDBook { pub fn build(&self) -> Result<()> { info!("Book building has started"); - for renderer in &self.renderers { + for renderer in self.renderers.values() { self.execute_build_process(&**renderer)?; } @@ -171,7 +176,7 @@ impl MDBook { renderer.name().to_string(), ); let mut preprocessed_book = self.book.clone(); - for preprocessor in &self.preprocessors { + for preprocessor in self.preprocessors.values() { if preprocessor_should_run(&**preprocessor, renderer, &self.config)? { debug!("Running the {} preprocessor.", preprocessor.name()); preprocessed_book = preprocessor.run(&preprocess_ctx, preprocessed_book)?; @@ -207,13 +212,15 @@ impl MDBook { /// The only requirement is that your renderer implement the [`Renderer`] /// trait. pub fn with_renderer(&mut self, renderer: R) -> &mut Self { - self.renderers.push(Box::new(renderer)); + self.renderers + .insert(renderer.name().to_string(), Box::new(renderer)); self } /// Register a [`Preprocessor`] to be used when rendering the book. pub fn with_preprocessor(&mut self, preprocessor: P) -> &mut Self { - self.preprocessors.push(Box::new(preprocessor)); + self.preprocessors + .insert(preprocessor.name().to_string(), Box::new(preprocessor)); self } @@ -258,10 +265,9 @@ impl MDBook { // Index Preprocessor is disabled so that chapter paths // continue to point to the actual markdown files. - self.preprocessors = determine_preprocessors(&self.config, &self.root)? - .into_iter() - .filter(|pre| pre.name() != IndexPreprocessor::NAME) - .collect(); + self.preprocessors = determine_preprocessors(&self.config, &self.root)?; + self.preprocessors + .shift_remove_entry(IndexPreprocessor::NAME); let (book, _) = self.preprocess_book(&TestRenderer)?; let color_output = std::io::stderr().is_terminal(); @@ -399,24 +405,25 @@ struct OutputConfig { } /// Look at the `Config` and try to figure out what renderers to use. -fn determine_renderers(config: &Config) -> Result>> { - let mut renderers = Vec::new(); +fn determine_renderers(config: &Config) -> Result>> { + let mut renderers = IndexMap::new(); let outputs = config.outputs::()?; renderers.extend(outputs.into_iter().map(|(key, table)| { - if key == "html" { + let renderer = if key == "html" { Box::new(HtmlHandlebars::new()) as Box } else if key == "markdown" { Box::new(MarkdownRenderer::new()) as Box } else { let command = table.command.unwrap_or_else(|| format!("mdbook-{key}")); - Box::new(CmdRenderer::new(key, command)) - } + Box::new(CmdRenderer::new(key.clone(), command)) + }; + (key, renderer) })); // if we couldn't find anything, add the HTML renderer as a default if renderers.is_empty() { - renderers.push(Box::new(HtmlHandlebars::new())); + renderers.insert("html".to_string(), Box::new(HtmlHandlebars::new())); } Ok(renderers) @@ -442,7 +449,10 @@ struct PreprocessorConfig { } /// Look at the `MDBook` and try to figure out what preprocessors to run. -fn determine_preprocessors(config: &Config, root: &Path) -> Result>> { +fn determine_preprocessors( + config: &Config, + root: &Path, +) -> Result>> { // Collect the names of all preprocessors intended to be run, and the order // in which they should be run. let mut preprocessor_names = TopologicalSort::::new(); @@ -490,7 +500,7 @@ fn determine_preprocessors(config: &Config, root: &Path) -> Result Result= index(after) { eprintln!("Preprocessor order:"); - for preprocessor in &preprocessors { - eprintln!(" {}", preprocessor.name()); + for preprocessor in preprocessors.keys() { + eprintln!(" {}", preprocessor); } panic!("{before} should come before {after}"); } @@ -193,11 +186,8 @@ fn dependencies_dont_register_undefined_preprocessors() { let preprocessors = determine_preprocessors(&cfg, Path::new("")).unwrap(); - assert!( - !preprocessors - .iter() - .any(|preprocessor| preprocessor.name() == "random") - ); + // Does not contain "random" + assert_eq!(preprocessors.keys().collect::>(), ["index", "links"]); } #[test] @@ -214,11 +204,8 @@ fn dependencies_dont_register_builtin_preprocessors_if_disabled() { let preprocessors = determine_preprocessors(&cfg, Path::new("")).unwrap(); - assert!( - !preprocessors - .iter() - .any(|preprocessor| preprocessor.name() == "links") - ); + // Does not contain "links" + assert_eq!(preprocessors.keys().collect::>(), ["random"]); } #[test] diff --git a/tests/testsuite/preprocessor.rs b/tests/testsuite/preprocessor.rs index 81b7b4e7..de3e5efe 100644 --- a/tests/testsuite/preprocessor.rs +++ b/tests/testsuite/preprocessor.rs @@ -193,14 +193,9 @@ fn with_preprocessor_same_name() { let spy: Arc> = Default::default(); let mut book = test.load_book(); book.with_preprocessor(Spy(Arc::clone(&spy))); - let err = book.build().unwrap_err(); - test.assert.eq( - format!("{err:?}"), - str![[r#" -Unable to run the preprocessor `dummy` - -Caused by: - [NOT_FOUND] -"#]], - ); + // Unfortunately this is unable to capture the output when using the API. + book.build().unwrap(); + let inner = spy.lock().unwrap(); + assert_eq!(inner.run_count, 1); + assert_eq!(inner.rendered_with, ["html"]); } diff --git a/tests/testsuite/renderer.rs b/tests/testsuite/renderer.rs index 4f845297..456fcd7e 100644 --- a/tests/testsuite/renderer.rs +++ b/tests/testsuite/renderer.rs @@ -254,15 +254,8 @@ fn with_renderer_same_name() { let spy: Arc> = Default::default(); let mut book = test.load_book(); book.with_renderer(Spy(Arc::clone(&spy))); - let err = book.build().unwrap_err(); - test.assert.eq( - format!("{err:?}"), - str![[r#" -Rendering failed - -Caused by: - 0: Unable to run the backend `dummy` - 1: [NOT_FOUND] -"#]], - ); + // Unfortunately this is unable to capture the output when using the API. + book.build().unwrap(); + let inner = spy.lock().unwrap(); + assert_eq!(inner.run_count, 1); }