tommilligan / mdbook-admonish

A preprocessor for mdbook to add Material Design admonishments.
MIT License
171 stars 18 forks source link

Admonish breaks if `on_failure = "bail"` is set and `[preprocessor.admonish.default]` exists in `book.toml` #196

Closed DianaNites closed 2 months ago

DianaNites commented 3 months ago

Admonish will fail and break the build entirely if on_failure = "bail" is set and [preprocessor.admonish.default] exists, for example as documented at https://tommilligan.github.io/mdbook-admonish/reference.html#booktoml-configuration.

# book.toml
[preprocessor.admonish]
assets_version = "3.0.2" # do not edit: managed by `mdbook-admonish install`
command = "mdbook-admonish"
on_failure = "bail"

[preprocessor.admonish.default]
# collapsible = true # With or without this or any other value.

[preprocessor.admonish.renderer.test] # With or without this.
render_mode = "strip"

results in

[2024-06-18T01:41:58Z ERROR mdbook_admonish] Fatal error: values must be emitted before tables
[2024-06-18T01:41:58Z ERROR mdbook_admonish]   - values must be emitted before tables
2024-06-18 01:41:58 [ERROR] (mdbook::utils): Error: The "admonish" preprocessor exited unsuccessfully with exit status: 1 status

commenting out on_failure successfully completes with no warnings or errors, and values take effect with no obvious issue. There is no error message I can find in the rendered output or anywhere else, as the documentation says there should be when there is an error: "If rendering to HTML, an error message will be displayed in the book output."

It makes no difference whether any admonish blocks actually even exist in the mdbook.

This is the only configuration item for which I have observed this. It's a pretty annoying problem that having any defaults and reporting invalid blocks are mutually exclusive.

tommilligan commented 3 months ago

Hi, thanks for the detailed report. Can I also ask which version of mdbook and mdbook-admonish you're using?

sigh I've seen this error before in development, and thought I'd worked around it. It's an old bug in the toml library, but because mdbook can't upgrade it's toml dependency, we still have to care about it.

If I recall correctly, this was triggered by defining new tables in some sequence after inline values. It's possible you could work around this by changing the structure of your config to use more inline tables, like

on_failure = "bail"
default = { collapsible = true }

but I haven't tested this.

I tried pretty hard to properly solve this at the time and gave up, but I'll take another look.

DianaNites commented 3 months ago

I'd tried an inline table as well, it still fails.

I thought I mentioned the versions but it seems I lost them during editing, I tried (mdbook v0.4.37, admonish v1.15.0) (the versions I happened to already have installed when I last worked on the project where I first encountered this, but did not at the time investigate), (mdbook v0.4.40, admonish v1.15.0), and (mdbook v0.4.40, admonish v1.17.1).

tommilligan commented 2 months ago

Okay, after poking around more I found I was making this needlessly complex for myself. Thanks for making me fix it!

DianaNites commented 2 months ago

Thank you for the fix! Just updated and it works fine now!