Skip to content
Merged
Show file tree
Hide file tree
Changes from 12 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
124 changes: 116 additions & 8 deletions src/book/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use toml::Value;

use utils;
use renderer::{CmdRenderer, HtmlHandlebars, RenderContext, Renderer};
use preprocess;
use preprocess::{Preprocessor, LinkPreprocessor, PreprocessorContext};
use errors::*;

use config::Config;
Expand All @@ -40,6 +40,9 @@ pub struct MDBook {

/// The URL used for live reloading when serving up the book.
pub livereload: Option<String>,

/// List of pre-processors to be run on the book
preprocessors: Vec<Box<Preprocessor>>
}

impl MDBook {
Expand Down Expand Up @@ -85,13 +88,15 @@ impl MDBook {
let livereload = None;

let renderers = determine_renderers(&config);
let preprocessors = determine_preprocessors(&config)?;

Ok(MDBook {
root,
config,
book,
renderers,
livereload,
preprocessors,
})
}

Expand Down Expand Up @@ -151,14 +156,23 @@ impl MDBook {
pub fn build(&self) -> Result<()> {
debug!("[fn]: build");

let mut preprocessed_book = self.book.clone();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that we're not mutating the original book here and instead preprocessing its clone 👍

let preprocess_ctx = PreprocessorContext {
src_dir: self.source_dir(),
};

for preprocessor in &self.preprocessors {
preprocessor.run(&preprocess_ctx, &mut preprocessed_book)?;
}

for renderer in &self.renderers {
self.run_renderer(renderer.as_ref())?;
self.run_renderer(&preprocessed_book, renderer.as_ref())?;
}

Ok(())
}

fn run_renderer(&self, renderer: &Renderer) -> Result<()> {
fn run_renderer(&self, preprocessed_book: &Book, renderer: &Renderer) -> Result<()> {
let name = renderer.name();
let build_dir = self.build_dir_for(name);
if build_dir.exists() {
Expand All @@ -174,7 +188,7 @@ impl MDBook {

let render_context = RenderContext::new(
self.root.clone(),
self.book.clone(),
preprocessed_book.clone(),
self.config.clone(),
build_dir,
);
Expand All @@ -192,6 +206,13 @@ impl MDBook {
self
}

/// You can add a new preprocessor by using this method.
/// The only requirement is for your renderer to implement the Preprocessor trait.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doc-comment should probably be reworded in the "imperative" tense to describe what it does. So something like "Register a [Preprocessor] to be used when rendering the book", with the word "Preprocessor" being a link to the relevant trait so people can see the required methods and what types already implement Preprocessor.

pub fn with_preprecessor<P: Preprocessor + 'static>(&mut self, preprocessor: P) -> &mut Self {
self.preprocessors.push(Box::new(preprocessor));
self
}

/// Run `rustdoc` tests on the book, linking against the provided libraries.
pub fn test(&mut self, library_paths: Vec<&str>) -> Result<()> {
let library_args: Vec<&str> = (0..library_paths.len())
Expand All @@ -202,15 +223,18 @@ impl MDBook {

let temp_dir = TempDir::new("mdbook")?;

let src_dir = self.source_dir();
let preprocess_context = PreprocessorContext {
src_dir
};

LinkPreprocessor::new().run(&preprocess_context, &mut self.book)?;

for item in self.iter() {
if let BookItem::Chapter(ref ch) = *item {
if !ch.path.as_os_str().is_empty() {
let path = self.source_dir().join(&ch.path);
let base = path.parent()
.ok_or_else(|| String::from("Invalid bookitem path!"))?;
let content = utils::fs::file_to_string(&path)?;
// Parse and expand links
let content = preprocess::links::replace_all(&content, base)?;
println!("[*]: Testing file: {:?}", path);

// write preprocessed file to tempdir
Expand Down Expand Up @@ -309,6 +333,29 @@ fn determine_renderers(config: &Config) -> Vec<Box<Renderer>> {
renderers
}

/// Look at the `MDBook` and try to figure out what preprocessors to run.
fn determine_preprocessors(config: &Config) -> Result<Vec<Box<Preprocessor>>> {

let preprocess_list = match config.build.preprocess {
Some(ref p) => p,
// If no preprocessor field is set, default to the LinkPreprocessor. This allows you
// to disable the LinkPreprocessor by setting "preprocess" to an empty list.
None => return Ok(vec![Box::new(LinkPreprocessor::new())])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reckon we may want to pull this bit out into its own default_preprocessors() function, where we immediately return vec![Box::new(LinkPreprocessor::new())]. I imagine that'll make things easier to update in the future, and it's immediately obvious what the default preprocessors are.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done - you're right, makes it a bit cleaner

};

let mut preprocessors: Vec<Box<Preprocessor>> = Vec::new();

for key in preprocess_list {
if key == "links" {
preprocessors.push(Box::new(LinkPreprocessor::new()))
} else {
bail!("{:?} is not a recognised preprocessor", key);
}
}

Ok(preprocessors)
}

fn interpret_custom_renderer(key: &str, table: &Value) -> Box<Renderer> {
// look for the `command` field, falling back to using the key
// prepended by "mdbook-"
Expand Down Expand Up @@ -364,4 +411,65 @@ mod tests {
assert_eq!(got.len(), 1);
assert_eq!(got[0].name(), "random");
}

#[test]
fn config_defaults_to_link_preprocessor_if_not_set() {
let cfg = Config::default();

// make sure we haven't got anything in the `output` table
assert!(cfg.build.preprocess.is_none());

let got = determine_preprocessors(&cfg);

assert!(got.is_ok());
assert_eq!(got.as_ref().unwrap().len(), 1);
assert_eq!(got.as_ref().unwrap()[0].name(), "links");
}

#[test]
fn config_doesnt_default_if_empty() {
let cfg_str: &'static str = r#"
[book]
title = "Some Book"

[build]
build-dir = "outputs"
create-missing = false
preprocess = []
"#;


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

// make sure we have something in the `output` table
assert!(cfg.build.preprocess.is_some());

let got = determine_preprocessors(&cfg);

assert!(got.is_ok());
assert!(got.unwrap().is_empty());
}

#[test]
fn config_complains_if_unimplemented_preprocessor() {
let cfg_str: &'static str = r#"
[book]
title = "Some Book"

[build]
build-dir = "outputs"
create-missing = false
preprocess = ["random"]
"#;


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

// make sure we have something in the `output` table
assert!(cfg.build.preprocess.is_some());

let got = determine_preprocessors(&cfg);

assert!(got.is_err());
}
}
8 changes: 8 additions & 0 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -329,13 +329,17 @@ pub struct BuildConfig {
/// Should non-existent markdown files specified in `SETTINGS.md` be created
/// if they don't exist?
pub create_missing: bool,
/// Which preprocessors should be applied
pub preprocess: Option<Vec<String>>,

}

impl Default for BuildConfig {
fn default() -> BuildConfig {
BuildConfig {
build_dir: PathBuf::from("book"),
create_missing: true,
preprocess: None,
}
}
}
Expand Down Expand Up @@ -422,6 +426,7 @@ mod tests {
[build]
build-dir = "outputs"
create-missing = false
preprocess = ["first_preprocessor", "second_preprocessor"]

[output.html]
theme = "./themedir"
Expand Down Expand Up @@ -449,6 +454,8 @@ mod tests {
let build_should_be = BuildConfig {
build_dir: PathBuf::from("outputs"),
create_missing: false,
preprocess: Some(vec!["first_preprocessor".to_string(),
"second_preprocessor".to_string()]),
};
let playpen_should_be = Playpen {
editable: true,
Expand Down Expand Up @@ -550,6 +557,7 @@ mod tests {
let build_should_be = BuildConfig {
build_dir: PathBuf::from("my-book"),
create_missing: true,
preprocess: None,
};

let html_should_be = HtmlConfig {
Expand Down
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ extern crate toml_query;
#[macro_use]
extern crate pretty_assertions;

mod preprocess;
pub mod preprocess;
pub mod book;
pub mod config;
pub mod renderer;
Expand Down
36 changes: 35 additions & 1 deletion src/preprocess/links.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,43 @@ use utils::fs::file_to_string;
use utils::take_lines;
use errors::*;

use super::{Preprocessor, PreprocessorContext};
use book::{Book, BookItem};

const ESCAPE_CHAR: char = '\\';

pub fn replace_all<P: AsRef<Path>>(s: &str, path: P) -> Result<String> {
pub struct LinkPreprocessor;

impl LinkPreprocessor {
pub fn new() -> Self {
LinkPreprocessor
}
}

impl Preprocessor for LinkPreprocessor {
fn name(&self) -> &str {
"links"
}

fn run(&self, ctx: &PreprocessorContext, book: &mut Book) -> Result<()> {
for section in &mut book.sections {
match *section {
BookItem::Chapter(ref mut ch) => {
let base = ch.path.parent()
.map(|dir| ctx.src_dir.join(dir))
.ok_or_else(|| String::from("Invalid bookitem path!"))?;
let content = replace_all(&ch.content, base)?;
ch.content = content
}
_ => {}
}
}

Ok(())
}
}

fn replace_all<P: AsRef<Path>>(s: &str, path: P) -> Result<String> {
// When replacing one thing in a string by something with a different length,
// the indices after that will not correspond,
// we therefore have to store the difference to correct this
Expand Down
18 changes: 17 additions & 1 deletion src/preprocess/mod.rs
Original file line number Diff line number Diff line change
@@ -1 +1,17 @@
pub mod links;
pub use self::links::LinkPreprocessor;

mod links;

use book::Book;
use errors::*;

use std::path::PathBuf;

pub struct PreprocessorContext {
pub src_dir: PathBuf
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably better for PreprocessorContext to contain a reference (or clone, if you don't want to deal with lifetimes) to the Config and the book's root. That way a preprocessor can hook into book.toml for configuration, and the src_dir can be discovered by combining config.book.src and the book root.

So something like this:

pub struct PreprocessorContext {
  pub config: Config,
  pub root: PathBuf,
  // maybe also include the `Book` here... I dunno
}

Copy link
Contributor Author

@JaimeValdemoros JaimeValdemoros Jan 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense, given the other fields of the MDBook shouldn't be needed by the preprocessor. I've made it a clone for now since a borrow would be problematic here:

https://github.com/rust-lang-nursery/mdBook/blob/bf093e2f5f0e8c7f455b6e021cb72adb974836d4/src/book/mod.rs#L205

since we'd have a reference to an inner field of self in the Context and additionally trying to take a reference to itself to pass into iter.

}

pub trait Preprocessor {
fn name(&self) -> &str;
fn run(&self, ctx: &PreprocessorContext, book: &mut Book) -> Result<()>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm tossing up whether to include the Book as part of the PreprocessorContext. Do you think the book to be processed should be part of the context or should we keep it as a second argument?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's helpful to distinguish the fact that you only expect the Book itself to be mutated, and that the PreprocessorContext is additional information it can use to help in that mutation. Having a single parameter &mut PreprocessorContext obscures that slightly and has fewer guarantees about what the Preprocessor is allowed to do to the context.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah that's a good point. I completely forgot the Book is &mut while we pass an immutable reference for the PreprocessorContext.

}
9 changes: 0 additions & 9 deletions src/renderer/html_handlebars/hbs_renderer.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use renderer::html_handlebars::helpers;
use preprocess;
use renderer::{RenderContext, Renderer};
use book::{Book, BookItem, Chapter};
use config::{Config, HtmlConfig, Playpen};
Expand Down Expand Up @@ -50,12 +49,6 @@ impl HtmlHandlebars {
match *item {
BookItem::Chapter(ref ch) => {
let content = ch.content.clone();
let base = ch.path.parent()
.map(|dir| ctx.src_dir.join(dir))
.expect("All chapters must have a parent directory");

// Parse and expand links
let content = preprocess::links::replace_all(&content, base)?;
let content = utils::render_markdown(&content, ctx.html_config.curly_quotes);
print_content.push_str(&content);

Expand Down Expand Up @@ -322,7 +315,6 @@ impl Renderer for HtmlHandlebars {
let ctx = RenderItemContext {
handlebars: &handlebars,
destination: destination.to_path_buf(),
src_dir: src_dir.clone(),
data: data.clone(),
is_index: i == 0,
html_config: html_config.clone(),
Expand Down Expand Up @@ -634,7 +626,6 @@ fn partition_source(s: &str) -> (String, String) {
struct RenderItemContext<'a> {
handlebars: &'a Handlebars,
destination: PathBuf,
src_dir: PathBuf,
data: serde_json::Map<String, serde_json::Value>,
is_index: bool,
html_config: HtmlConfig,
Expand Down
Loading