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

feat: add support for test renderer, running doctests #109

Merged
merged 8 commits into from
Jul 23, 2023
Merged

Conversation

tommilligan
Copy link
Owner

@tommilligan tommilligan commented Jul 21, 2023

Closes #108

Adds support for the test renderer used by mdbook test to run doctests.

This required implementing a new method to simply strip all admonish syntax after relevant blocks are identified and parsed, leaving behind the content (basically in an "unwrapped" form)

This should not be a breaking change, as all block content is subject to further rendering from the markdown engine anyway in the standard html render pipeline. However, this will definitely be a minor/feature release. It could technically be breaking if tests that were previously undetected now fail, but I'd argue that's a bugfix.

@stevecheckoway open for comments on if this fits your use case, and if you think there are any other details I've missed

@stevecheckoway
Copy link

I did a quick test. I cloned the repo, checked out the commit in this PR, and then updated my book.toml to point the command to the newly build mdbook-admonish. Running mdbook test seemed to correctly test my code (and I now get a bunch of expected errors due to missing crates in my examples).

I think this might be a breaking change with respect to semvar, but I'm hardly an expert. You could introduce a configuration switch like this.

[preprocessor.admonish]
renderers = ["html", "test"]

I copied that example from the configuration I see for mdbook-katex.

But I'm happy with this as is.

@tommilligan
Copy link
Owner Author

That's an excellent idea. I'll add a renderer option in the next minor version, and add a note to enable it by default for next major version

@stevecheckoway
Copy link

Thanks for looking into this. I really appreciate it (and I really appreciate mdbook-admonish).

@tommilligan
Copy link
Owner Author

I ended up restructuring a bit to make the approach more extensible and testable - lots of refactoring that's only interesting to me, but the upshot is you can now add:

[preprocessor.admonish.renderer.test]
render_mode = "strip"

And it will enable the admonish syntax stripping as described above. Otherwise, it'll do nothing.

You can test this by simply enabling the same setting for the html renderer, if you like:

[preprocessor.admonish.renderer.html]
render_mode = "strip"

I have a few more bits of documentation to tidy up, but I'll put out a minor release with this in asap

@tommilligan tommilligan merged commit 0742c6c into main Jul 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Admonish doesn't work with mdbook test
2 participants