rust-lang / mdBook

Create book from markdown files. Like Gitbook but implemented in Rust
https://rust-lang.github.io/mdBook/
Mozilla Public License 2.0
17.84k stars 1.62k forks source link

disincentivize usage of functions that expose toml::Table in Config #2407

Open JulianGmp opened 3 months ago

JulianGmp commented 3 months ago

hi there,

firstly, thank you for your continued work on mdbook, it's a great tool :)

This is related to #2037, which I encountered while trying to deserialize the exposed Map/Table into a user defined struct in a preprocessor I'm writing.

Using get_deserialized_opt seems much more preferable to get_preprocessor (and get_renderer for that matter). However, I merely happend to stumble across that function while looking through config.rs.
I don't think I'm alone with this, I did a brief search over the preprocessors listed in the third party plugins page and the ones that do have options use get_preprocessor.

The one caveat for get_deserialized_opt is that you have to pass "preprocessor.my_thing" instead of "my_thing", so I made get_renderer_deserialized and get_preprocessor_deserialized for convenience. I added that to the example to incentivize people writing new preprocessors to use it.

I also marked get_renderer and get_preprocessor as deprecated. However this might be bold. There are probably valid use cases for these functions, unless mdbook were to remove the exposed toml type(s) like #2037 describes (which would significantly change mdbook::config::Config's interface).

Note that this also results in 3 deprecation warnings in the tests.
I haven't addresses these, because I think the decision whether to deprecate them at all should be done by the maintainers, I'm just a user of the library/tool.

danieleades commented 1 month ago

i think deprecating the methods that expose the toml type and pointing users towards get_deserialized_opt is the right way to go