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

Allow to set the collapsible property for each directive #174

Merged
merged 1 commit into from
May 19, 2024

Conversation

yannickseurin
Copy link
Contributor

Allows to set the collapsible property independently for each directive (overwriting the global default) as suggested in issue #169 .

  • for custom directives, this is simply done by adding a collapsible subfield in the directive declaration;
  • for builtin directives, it uses a HashMap in AdmonitionDefaults.

When resolving a block, the value of collapsible is set to

  • the value of collapsible for the block (if present),
  • else, the default value of collapsible for the directive (if set),
  • else, the global default value for collapsible.

Note that the updated documentation specifies that the table for builtin directives must be declared as

[preprocessor.admonish.default.builtins_collapsible]

which assumes that PR 173 is merged (otherwise it should be builtins-collapsible).

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.

Very happy with the end functionality, I'd just like to update the configuration format so it's extensible with more options for builtin directives in future.

book/src/reference.md Outdated Show resolved Hide resolved
@tommilligan
Copy link
Owner

This ended up turning into a big rewrite, but I'm happy with the outcome. The upshot is that we can now configure in a uniform style like:

[preprocessor.admonish.directive.builtin.warning]
collapsible = true

[preprocessor.admonish.directive.custom.expensive]
icon = "./money-bag.svg"
color = "#24ab38"
collapsible = true
aliases = ["money", "cash", "budget"]

The old style configuration for custom is supported backwards compatibly. This leaves room for further PRs to add overrides for builtin directives aliases, icons etc. if people want that in future.

@tommilligan tommilligan merged commit 80cce84 into tommilligan:main May 19, 2024
6 checks passed
@yannickseurin
Copy link
Contributor Author

Very neat, thanks for this! And sorry for having prompted such a big rewrite :)

@yannickseurin yannickseurin deleted the custom-collapsible branch May 21, 2024 09:29
@tommilligan
Copy link
Owner

Very neat, thanks for this! And sorry for having prompted such a big rewrite :)

Not a problem! It was in my mind for a while and it worked to get it out here. Also prompted by problems with the existing format and the old version of toml that mdbook uses internally - it has a bug in it such that the table-of-tables approach I was using literally broke when used 😞

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.

2 participants