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 default title option #84

Merged
merged 2 commits into from
Apr 20, 2023

Conversation

ShaunSHamilton
Copy link
Contributor

Adds:

[preprocessor.admonish.default]
title = "..."

Priorities:

explicit > default > directive

What this means is any parse-able title given in the codeblock is taken as the desired. Then, any default given in the book.toml. Then, the directive is used as the fallback.

My code is messier than I expected. So, feel free to be nit-picky about it

Specifically, I am not sure how useful the return Result<()> from Admonition::attach_defaults is.

Closes #82

Copy link
Owner

@tommilligan tommilligan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the initial pass on this, it's a really good starting point.

I ended up doing a little bit of a wider refactor to make the distinction between AdmonitionInfo (resolved values for rendering) and AdmonitionInfoRaw (raw user configuration) clearer. Probably some new names would help as well.

I also added support for collapsible at the same time, to check the approach was extensible.

I've pushed a commit with my review changes in it - have a look and see if anything sticks out to you. Otherwise I'll push the changes in sometime this week.

src/config/mod.rs Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Show resolved Hide resolved
@ShaunSHamilton
Copy link
Contributor Author

In testing this looks good to me 👍

@tommilligan tommilligan merged commit e8813eb into tommilligan:main Apr 20, 2023
@ShaunSHamilton ShaunSHamilton deleted the feat_default-title branch April 20, 2023 15:29
@tommilligan
Copy link
Owner

Now released in v1.9.0

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.

setting to default to empty title
2 participants