Configurable preprocessor

[Michael-F-Bryan]

This adds the ability to configure a Preprocessor and specify which renderers it gets run for.

It effectively changes the build step to be something like:

for renderer in &self.renderers {
  let mut book = self.book.clone();

  for preprocessor in &self.preprocessors() {
    if should_run(preprocessor, renderer.name()) {
      book = preprocessor.run(book)?;
    }
  }

  renderer.render(book)?;
}

Where the should_run() function inspects the book.toml to first ensure we're allowed to run this preprocessor with a specific renderer, falling back to a new Preprocessor::supports_renderer() method if the user doesn't say anything. This setup means the user always has the final say, via their book.toml.

We'll need to update the book.toml format to have a build.use-default-preprocessors flag now preprocessors have their own [preprocessor.*] set of tables.

I'll update the Configuration section in the user guide, but for now this is how I imagine the relevant sections in book.toml will look.

[build]
use-default-preprocessors = true  # enables the "links" preprocessor

[preprocessor.mathjax]
renderers = ["html"]  # mathjax only makes sense with the HTML renderer

cc: #626

[Michael-F-Bryan]

@dvberkel and @Byron, can you have a quick look at what I'm planning to do in this PR and let me know what you think?

[dvberkel]

@Michael-F-Bryan To me this looks good.

It allows a user fine control on which preprocessors should run when, without to much of configuration.

👍

[mattico]

This LGTM as well. I'm looking into the netbsd failure. We're probably going to have to disable that target for now.

[Byron]

Looks good to me!
Will plugin preprocessors also come in with this PR?

[Byron]

Can it be this function is never called? (I am using the webview to see all changed files at once, and this one doesn’t show up anywhere else)

[Byron]

Ah, maybe it’s still a WIP, and it’s more about comments on the general direction :). Never mind.

[Michael-F-Bryan]

Yeah this PR is still a work in progress. Eventually I want to extract the logic out into a should_run() function which will check the config if the user explicitly overrides what preprocessors are run, then fall back to asking a preprocessor directly whether it supports a specific renderer.

[Michael-F-Bryan]

Will plugin preprocessors also come in with this PR?

I was thinking that can be a second PR. This one mainly deals with the issue of making sure certain preprocessors only get run for the renderers they support. That's the big thing blocking #626 at the moment.

[rbuckland]

Is this change going to be merged ? or abandoned?

I am keen to work on #631 as I have quite a lot of little "pluggable" preprocessor candidates.
Seems like this was a precursor to #631

[Michael-F-Bryan]

Is this change going to be merged ? or abandoned?

I got a full time job and ran out of time :( That said, this PR is quite small so I'll see what I can do tonight to un-bitrot it.

[Michael-F-Bryan]

@rbuckland I still need to update the user guide to reflect the changes, but in a nutshell this PR will:

• Updated the Preprocessor trait to be more like a fn(Book) -> Book than fn(&mut Book)
• Create a copy of the book for each renderer, passing the copy through each preprocessor if preprocessor_should_run() determines that the preprocessor should be run
• A Preprocessor can now provide a hint, letting mdbook know when it's not compatible with a particular renderer (Preprocessor::supports_renderer()). This defaults to true and can be overridden by the user using the config options from the next point
• Users can explicitly specify that a preprocessor should run for a renderer with something like this:

  [preprocessor.mathjax]
  renderers = ["html"]  # mathjax only makes sense with the HTML renderer

• By default the default preprocessors (index and links) will be enabled for any renderers they support (currently all of them). This can be disabled via the build.use-default-preprocessors option
• Preprocessors now get their own table which can be used for passing in config options. This means that
  this:

  [build]
  preprocessors = ["foo"]

  for enabling the foo preprocessor, will now be written as this:

  [preprocessor.foo]

  This is technically a breaking change. Although I'm not sure the previous syntax was ever used because it's currently impossible to create your own preprocessors and the two default preprocessors don't accept any config options.

This should hopefully pave the way for creating custom preprocessors, using the Preprocessor equivalent of CmdRenderer, it also gives the user a way to configure specific preprocessors.

---

@mattico or @rbuckland would one of you be able to review this and let me know what you think? All the tests pass and I think it behaves as I would like, but I literally put it together while lying in bed and it'll need a second set of eyes before it's safe to merge.

[rbuckland]

That’s excellent @Michael-F-Bryan I will review over today.

[rbuckland]

Hi @Michael-F-Bryan, I have some good news for you.

I looked over the code, ✅, this will unblock me to add some of the features I and others are calling out for,
And yes agree - we need to update the user guide.

Regarding if it "breaks" anything (ignoring the breaking change you called out on config)

I took the liberty to script up a force check of mdbook (2x versions) against multiple "other" book.toml repos.

And the result is that

  :: https://github.com/rust-lang-nursery/mdBook
  :: https://github.com/rust-lang/rust-by-example
  :: https://github.com/rust-lang-nursery/rustc-guide
  :: https://github.com/rust-lang-nursery/api-guidelines
  :: https://github.com/rust-lang-nursery/rust-cookbook

All generated identically with the current "master" version and this "smart-preprocessor" version.

I created a PR on your branch that adds those scripts (as they are some what re-usable).

[rbuckland]

I am moving these test scripts over to a separate branch https://github.com/rbuckland/mdBook/tree/feature/other-books-test-scripts

[Michael-F-Bryan]

@mattico and @rbuckland, I think this PR is pretty much good to go. Can you see any points we forgot to address or fix up?

[rbuckland]

I think it looks good @Michael-F-Bryan.
I have been working off this branch for my changes so can vouch that it is good.