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.

Author: ehuss

Cargo.lock

@@ -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" }

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

crates/mdbook-driver/Cargo.toml

@@ -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<Box<dyn Renderer>>,
 
-    /// List of pre-processors to be run on the book.
-    preprocessors: Vec<Box<dyn Preprocessor>>,
+    /// Renderers to execute.
+    renderers: IndexMap<String, Box<dyn Renderer>>,
+
+    /// Pre-processors to be run on the book.
+    preprocessors: IndexMap<String, Box<dyn Preprocessor>>,
 }
 
 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<R: Renderer + 'static>(&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<P: Preprocessor + 'static>(&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<Vec<Box<dyn Renderer>>> {
-    let mut renderers = Vec::new();
+fn determine_renderers(config: &Config) -> Result<IndexMap<String, Box<dyn Renderer>>> {
+    let mut renderers = IndexMap::new();
 
     let outputs = config.outputs::<OutputConfig>()?;
     renderers.extend(outputs.into_iter().map(|(key, table)| {
-        if key == "html" {
+        let renderer = if key == "html" {
             Box::new(HtmlHandlebars::new()) as Box<dyn Renderer>
         } else if key == "markdown" {
             Box::new(MarkdownRenderer::new()) as Box<dyn Renderer>
         } 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<Vec<Box<dyn Preprocessor>>> {
+fn determine_preprocessors(
+    config: &Config,
+    root: &Path,
+) -> Result<IndexMap<String, Box<dyn Preprocessor>>> {
     // Collect the names of all preprocessors intended to be run, and the order
     // in which they should be run.
     let mut preprocessor_names = TopologicalSort::<String>::new();
@@ -490,7 +500,7 @@ fn determine_preprocessors(config: &Config, root: &Path) -> Result<Vec<Box<dyn P
     }
 
     // Now that all links have been established, queue preprocessors in a suitable order
-    let mut preprocessors = Vec::with_capacity(preprocessor_names.len());
+    let mut preprocessors = IndexMap::with_capacity(preprocessor_names.len());
     // `pop_all()` returns an empty vector when no more items are not being depended upon
     for mut names in std::iter::repeat_with(|| preprocessor_names.pop_all())
         .take_while(|names| !names.is_empty())
@@ -516,14 +526,14 @@ fn determine_preprocessors(config: &Config, root: &Path) -> Result<Vec<Box<dyn P
                         .to_owned()
                         .unwrap_or_else(|| format!("mdbook-{name}"));
                     Box::new(CmdPreprocessor::new(
-                        name,
+                        name.clone(),
                         command,
                         root.to_owned(),
                         table.optional,
                     ))
                 }
             };
-            preprocessors.push(preprocessor);
+            preprocessors.insert(name, preprocessor);
         }
     }
 

crates/mdbook-driver/src/mdbook.rs

@@ -85,7 +85,7 @@ fn can_determine_third_party_preprocessors() {
 
     let got = determine_preprocessors(&cfg, Path::new("")).unwrap();
 
-    assert!(got.into_iter().any(|p| p.name() == "random"));
+    assert!(got.contains_key("random"));
 }
 
 #[test]
@@ -143,19 +143,12 @@ fn preprocessor_order_is_honored() {
     let cfg = Config::from_str(cfg_str).unwrap();
 
     let preprocessors = determine_preprocessors(&cfg, Path::new("")).unwrap();
-    let index = |name| {
-        preprocessors
-            .iter()
-            .enumerate()
-            .find(|(_, preprocessor)| preprocessor.name() == name)
-            .unwrap()
-            .0
-    };
+    let index = |name| preprocessors.get_index_of(name).unwrap();
     let assert_before = |before, after| {
         if index(before) >= 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::<Vec<_>>(), ["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::<Vec<_>>(), ["random"]);
 }
 
 #[test]

crates/mdbook-driver/src/mdbook/tests.rs

@@ -193,14 +193,9 @@ fn with_preprocessor_same_name() {
     let spy: Arc<Mutex<Inner>> = 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"]);
 }

tests/testsuite/preprocessor.rs

@@ -254,15 +254,8 @@ fn with_renderer_same_name() {
     let spy: Arc<Mutex<Inner>> = 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);
 }