Skip to content
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

Introduce Preprocessors #530

Closed
Michael-F-Bryan opened this issue Jan 7, 2018 · 4 comments
Closed

Introduce Preprocessors #530

Michael-F-Bryan opened this issue Jan 7, 2018 · 4 comments
Labels
A-Internal-representation Area: Internal Representation C-enhancement Category: Enhancement or feature request E-Help-wanted Experience: Help Needed E-Medium Experience: Medium
Milestone

Comments

@Michael-F-Bryan
Copy link
Contributor

Michael-F-Bryan commented Jan 7, 2018

As part of the overall effort to make mdbook more modular, it'd be nice to formalise the concept of a "preprocessor". This is a bit of code that's run immediately after a Book is loaded from disk and before it gets rendered, allowing you to apply manipulations/transformations to the original source code.

Use cases are:

  • The current {{#playpen file.rs}} and {{#include file.md}} "macros"
  • Linting for known issues to give the user better feedback (e.g. Files with spaces are not found #527)
  • Replacing a $$ ... $$ snippet with its mathjax equivalent

Rough guideline of what is involved:

  1. Introduce a Preprocessor trait
  2. Make MDBook hold a list of preprocessors (Vec<Box<Preprocessor>>) and add a with_preprocessor() method for adding more
  3. Extract {{#playpen ...}} and {{#include ...}} (from here) into their own preprocessor types instead of being run as part of the HTML renderer (here)
  4. In the MDBook::build() process, execute each preprocessor on a copy of the original Book
  5. Update MDBook::load_with_config() so we can discover which preprocessors should be run by inspecting the book.toml config

Hint: most of this can actually be stolen from #507 because it'll be identical to how we discover which alternate backends to run. Look for the determine_renderers() function.

  1. Document how preprocessors work in the user guide

It looks like a big job, but that's mainly because there are several little steps which need to be tweaked. Steps 1 and 3 are actually just a dozen lines, with step 5 being the most complex (and even then you can steal 90% of the work from existing code). I'd estimate this to be a couple hundred lines of code at most, with a large proportion of that being taken up by relatively straight-forward tests and documentation.

I'm also more than happy to help mentor through the entire process 😀

@Michael-F-Bryan Michael-F-Bryan added this to the Plug-ins milestone Jan 7, 2018
@Michael-F-Bryan Michael-F-Bryan added A-Internal-representation Area: Internal Representation E-Help-wanted Experience: Help Needed E-Medium Experience: Medium C-enhancement Category: Enhancement or feature request C-refactor Category: Code refactoring and removed C-refactor Category: Code refactoring labels Jan 7, 2018
@JaimeValdemoros
Copy link
Contributor

Hi! I saw your post on /r/rust - I've been meaning to start contributing to more open source projects and this sounds like a good place to get started. I don't really know how the whole mentor thing works but the steps you've laid out seem pretty clear, so I think I'll start an attempt and report back?

@Michael-F-Bryan
Copy link
Contributor Author

I don't really know how the whole mentor thing works but the steps you've laid out seem pretty clear, so I think I'll start an attempt and report back?

Me either! What I usually do is outline the steps I think we'll need to do to implement the feature, then we can see where we go from there. Sometimes the most difficult bit is just learning where different things are in the codebase so if you've got someone you can ask questions or bounce ideas off of, that can be a big help.

@JaimeValdemoros, if you open up a new PR titled something like "WIP implementing preprocessors" then we'll have somewhere to do review and discussion.

@JaimeValdemoros
Copy link
Contributor

Sorry, took me longer than I expected to get my dev environment set up again - pull request is here

@Michael-F-Bryan
Copy link
Contributor Author

Looks like we forgot to close this when #532 was merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Internal-representation Area: Internal Representation C-enhancement Category: Enhancement or feature request E-Help-wanted Experience: Help Needed E-Medium Experience: Medium
Projects
None yet
Development

No branches or pull requests

2 participants