Skip to content

disincentivize usage of functions that expose toml::Table in Config #2407

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions examples/nop-preprocessor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ fn handle_supports(pre: &dyn Preprocessor, sub_args: &ArgMatches) -> ! {
/// in your main `lib.rs` file.
mod nop_lib {
use super::*;
use serde::Deserialize;

/// A no-op preprocessor.
pub struct Nop;
Expand All @@ -87,10 +88,21 @@ mod nop_lib {
}

fn run(&self, ctx: &PreprocessorContext, book: Book) -> Result<Book, Error> {
// In testing we want to tell the preprocessor to blow up by setting a
// The config options our preprocessor uses, deserialized from the book.toml that
// mdbook uses
#[derive(Deserialize)]
struct Config {
/// Indicate whether or not the preprocessor should return an error.
#[serde(default)]
// tell serde to use std::default::Default (false) as the default
blow_up: bool,
}

// In testing we can tell the preprocessor to blow up by setting a
// particular config value
if let Some(nop_cfg) = ctx.config.get_preprocessor(self.name()) {
if nop_cfg.contains_key("blow-up") {
let nop_cfg: Option<Config> = ctx.config.get_preprocessor_deserialized(self.name())?;
if let Some(nop_cfg) = nop_cfg {
if nop_cfg.blow_up {
anyhow::bail!("Boom!!1!");
}
}
Expand Down
35 changes: 24 additions & 11 deletions src/book/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -706,7 +706,11 @@ mod tests {
let cfg = Config::from_str(cfg_str).unwrap();

// make sure the `preprocessor.random` table exists
assert!(cfg.get_preprocessor("random").is_some());
#[derive(serde::Deserialize)]
struct Random {}
let random: Result<Option<Random>> = cfg.get_preprocessor_deserialized("random");
assert!(random.is_ok());
assert!(random.unwrap().is_some());

let got = determine_preprocessors(&cfg).unwrap();

Expand All @@ -722,11 +726,17 @@ mod tests {

let cfg = Config::from_str(cfg_str).unwrap();

// make sure the `preprocessor.random` table exists
let random = cfg.get_preprocessor("random").unwrap();
let random = get_custom_preprocessor_cmd("random", &Value::Table(random.clone()));
// Deserialize the preproessor.random config section into a struct
#[derive(serde::Deserialize)]
struct Random {
command: String,
}
let random: Random = cfg
.get_preprocessor_deserialized("random")
.unwrap()
.unwrap();

assert_eq!(random, "python random.py");
assert_eq!(random.command, "python random.py");
}

#[test]
Expand Down Expand Up @@ -851,13 +861,16 @@ mod tests {
let cfg = Config::from_str(cfg_str).unwrap();

// double-check that we can access preprocessor.links.renderers[0]
#[derive(serde::Deserialize)]
struct Links {
renderers: Vec<String>,
}
let html = cfg
.get_preprocessor("links")
.and_then(|links| links.get("renderers"))
.and_then(Value::as_array)
.and_then(|renderers| renderers.get(0))
.and_then(Value::as_str)
.unwrap();
.get_preprocessor_deserialized::<Links, _>("links")
.unwrap()
.unwrap()
.renderers
.remove(0);
assert_eq!(html, "html");
let html_renderer = HtmlHandlebars;
let pre = LinkPreprocessor::new();
Expand Down
22 changes: 22 additions & 0 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -240,17 +240,39 @@ impl Config {
}

/// Get the table associated with a particular renderer.
#[deprecated = "prefer get_renderer_deserialized over get_renderer"]
pub fn get_renderer<I: AsRef<str>>(&self, index: I) -> Option<&Table> {
let key = format!("output.{}", index.as_ref());
self.get(&key).and_then(Value::as_table)
}

/// Convenience function to fetch the renderer section from the config and deserialize it
/// into some arbitrary type.
pub fn get_renderer_deserialized<'de, T: Deserialize<'de>, I: AsRef<str>>(
&self,
index: I,
) -> Result<Option<T>> {
let key = format!("output.{}", index.as_ref());
self.get_deserialized_opt(key)
}

/// Get the table associated with a particular preprocessor.
#[deprecated = "prefer get_preprocessor_deserialized over get_preprocessor"]
pub fn get_preprocessor<I: AsRef<str>>(&self, index: I) -> Option<&Table> {
let key = format!("preprocessor.{}", index.as_ref());
self.get(&key).and_then(Value::as_table)
}

/// Convenience function to fetch the preprocessor section from the config and deserialize it
/// into some arbitrary type.
pub fn get_preprocessor_deserialized<'de, T: Deserialize<'de>, I: AsRef<str>>(
&self,
index: I,
) -> Result<Option<T>> {
let key = format!("preprocessor.{}", index.as_ref());
self.get_deserialized_opt(key)
}

fn from_legacy(mut table: Value) -> Config {
let mut cfg = Config::default();

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
[preprocessor.nop-preprocessor]
command = "cargo run --quiet --example nop-preprocessor --"
blow-up = true
blow_up = true