Merge pull request #2802 from ehuss/with-replace

Change with_renderer/with_preprocessor to overwrite

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

@@ -22,7 +22,8 @@ enum StatusCode {
 pub struct BookTest {
     /// The temp directory where the test should perform its work.
     pub dir: PathBuf,
-    assert: snapbox::Assert,
+    /// Snapshot assertion support.
+    pub assert: snapbox::Assert,
     /// This indicates whether or not the book has been built.
     built: bool,
 }

tests/testsuite/book_test.rs

@@ -180,3 +180,22 @@ fn missing_optional_not_fatal() {
 "#]]);
     });
 }
+
+// with_preprocessor of an existing name.
+#[test]
+fn with_preprocessor_same_name() {
+    let mut test = BookTest::init(|_| {});
+    test.change_file(
+        "book.toml",
+        "[preprocessor.dummy]\n\
+         command = 'mdbook-preprocessor-does-not-exist'\n",
+    );
+    let spy: Arc<Mutex<Inner>> = Default::default();
+    let mut book = test.load_book();
+    book.with_preprocessor(Spy(Arc::clone(&spy)));
+    // 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

@@ -241,3 +241,21 @@ fn relative_command_path() {
     })
     .check_file("book/output", "test");
 }
+
+// with_renderer of an existing name.
+#[test]
+fn with_renderer_same_name() {
+    let mut test = BookTest::init(|_| {});
+    test.change_file(
+        "book.toml",
+        "[output.dummy]\n\
+         command = 'mdbook-renderer-does-not-exist'\n",
+    );
+    let spy: Arc<Mutex<Inner>> = Default::default();
+    let mut book = test.load_book();
+    book.with_renderer(Spy(Arc::clone(&spy)));
+    // 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);
+}