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

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JulianGmp
Copy link

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.

@rustbot rustbot added the S-waiting-on-review Status: waiting on a review label Jun 26, 2024
@danieleades
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: waiting on a review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants