tommilligan / mdbook-admonish

A preprocessor for mdbook to add Material Design admonishments.
MIT License
175 stars 20 forks source link

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

Open meator opened 7 months ago

meator commented 7 months ago

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 commented 7 months ago

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:

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 :disappointed: )

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 commented 7 months ago

Thank you for the explanation!