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

Why does mdbook-admonish have such a strange install process? #171

Open
meator opened this issue Mar 19, 2024 · 2 comments
Open

Why does mdbook-admonish have such a strange install process? #171

meator opened this issue Mar 19, 2024 · 2 comments

Comments

@meator
Copy link
Contributor

meator commented Mar 19, 2024

Hey. I have some question about the whole mdbook-admonish install process.

I know no other mdBook addon (but I don't know a lot of addons) that would have such artificial versioning. It is nice to make sure that breaking changes aren't introduced, but it makes it pretty difficult to use mdbook-admonish "rolling release".

The most problems are caused by the mdbook-admonish.css file.

Why is the mdbook-admonish.css included within the released mdbook-admonish executable? Including large data files in executables is strange.

I use admonish on my website: https://meator.github.io/xbps-src-tutorials/. As I describe in its README, I have created a wrapper script whose purpose is to "undo" the efforts of admonish to be versioned. This script is pretty hacky.

admonish provides no way to output the raw mdbook-admonish.css file. This is why I'm asking why is the file hardcoded into the executable. If it were a freestanding file, I could just copy it in the script. But the only way I know of to convince the mdbook-admonish executable to let go of this file is to install it to a book. Because of this, I have to create a fake stub book in the script, install admonish to it and then use the generated mdbook-admonish.css file in the build process of the real book.

Am I expected to include the mdbook-admonish.css file in my git repository? Or should I instruct the user to run mdbook-admonish install before building my book? (I already have a script mentioned earlier that handles that, but I'm asking generally.) If it should be included in the repository, I am reluctant to include arbitrary data files which aren't directly related to the content of the website to the repository. This could be documented better either on the website or in this project's README.

@tommilligan
Copy link
Owner

These are all really fair points. In terms of explicit implementation, this crate was initially heavily templated from mdbook-mermaid.

In terms of why this complex process, it mostly boils down to limitations in the mdbook system that mean the plugin cannot emit additional assets at runtime.

Specifically, what this plugin does is:

  • Generate some HTML
  • Apply some CSS to that HTML to make it look nice

The problem with this being a two component system, is if your components get out of step, things break (and because it's "only styling", they break in a bad-looking user-facing way 😞 )

There are a few options to try and get around this that I thought of (open to more ideas)

  1. Just emit all the CSS inline, as style directives
    • Good: simple (in some sense)
    • Bad: larger HTML, can't use mdbook theme system
  2. Have a separately managed CSS file, which applies to HTML emitted by mdbook-admonish
    • Good: CSS gets reused, is separate resource file for tracking/serving
    • Bad: If mdbook-admonish updates how it emits HTML, could cause mismatch with CSS and break styling
  3. Emit a new CSS file at runtime, and add it to the book output

Current status:

Based on the above, we do a hybrid of 2 and 3. We're not allowed to emit CSS at runtime, but we can put in a check that we already have the right CSS installed (pre-emitted, if you like). If this check fails, we can do something (log to user, raise an error, whatever).

So yes - if you like mdbook-admonish install is really like mdbook-admonish emit-latest-css-file, but tied into the book config and location.


To go through your questions point by point:

I know no other mdBook addon (but I don't know a lot of addons) that would have such artificial versioning. It is nice to make sure that breaking changes aren't introduced, but it makes it pretty difficult to use mdbook-admonish "rolling release".

Basically agreed. The current system requires CSS be updated/installed outside the book runtime - this check exists to ensure that has happened. If we can remove this extra step, we can dump the versioning system for sure. Or if you have a nicer idea of how to avoid breakage, very up for it.

Why is the mdbook-admonish.css included within the released mdbook-admonish executable? Including large data files in executables is strange.

Agreed, but what else do you suggest? We could of course provide it by CDN or some other external source, but this introduces other problems. Most users want to install a single binary package (from crates.io or otherwise) and have all functionality included there.

For reference, the CSS file can just be downloaded from the git repo if you need it.

I have created a wrapper script whose purpose is to "undo" the efforts of admonish to be versioned.

I'm not sure I understand why you need such a complex process to "unversion" here - it should be enough just to add the assets_version = "3.0.1" line, and use a fixed version of mdbook-admonish, which will always pass this check. Maybe I'm not understanding what you're trying to achieve.

admonish provides no way to output the raw mdbook-admonish.css file

This is fair, adding a new command to do that would be easy and I think generally useful.

Am I expected to include the mdbook-admonish.css file in my git repository? Or should I instruct the user to run mdbook-admonish install before building my book?

The latter. Whoever is building the book should run mdbook-admonish install before building, to emit the CSS assets for the book. This is already documented in the readme here, but if you think the wording could still be clearer feel free to suggest something.

You can also see this is what the mdbook-admonish book does itself

@meator
Copy link
Contributor Author

meator commented Mar 24, 2024

Thank you for the explanation!

tvlbot pushed a commit to tvlfyi/tvix that referenced this issue Jun 14, 2024
This provides a plugin for callouts.

It needs to have additional CSS file added to `book.toml`,
which can be (re-)generated by `mdbook-admonish install`.

See tommilligan/mdbook-admonish#171 (comment)
for more context.

Use it by adding a warning one to the architecture document.

Change-Id: I75c9a33d00acb603c6da10d3f9ce3485731c1672
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11805
Tested-by: BuildkiteCI
Autosubmit: flokli <[email protected]>
Reviewed-by: yuka <[email protected]>
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

No branches or pull requests

2 participants