search

webpro / reveal-md

reveal.js on steroids! Get beautiful reveal.js presentations from any Markdown file
MIT License
3.74k stars 416 forks source link

Remove markdown plugin #471

Closed MartenBE closed 10 months ago

MartenBE commented 11 months ago

As Reveal.js actually already parses slidified markdown, there is no need for us to call the markdown plugin. This is a PoC and a WIP (not ready for merging) to explore this direction, which if it works, would be interesting to merge in v6.

I am struggling with comprehending how all the options are handled (default, yamlfrontend, cli, ...), it could be I'll slightly refactor to make this more clear.

MartenBE commented 10 months ago

@webpro I think this is actually something usable that can be merged. I can run my slides with this PR without a difference from main. This PR makes sure we don't use the markdown plugin anymore, but let Reveal.js take care of all the slidification. This means our codebase gets simpler and we don't introduce additional errors for something that Reveal.js already does. For example, the slidify tests aren't necessary anymore because we don't do that anymore, we let Reveal.js do it all for us. I even tested it with the featured slide option and that also seems to work.

The only thing still unclear for me is the precedence of configs, as it seems to differ for mergedConfig, yamlOptions, slideOptions, ... . But perhaps it is best to keep this for another PR (the last for 6.0.0 so we can release that to the public)?

What do you think?

webpro commented 10 months ago

This PR makes sure we don't use the markdown plugin anymore, but let Reveal.js take care of all the slidification. This means our codebase gets simpler and we don't introduce additional errors for something that Reveal.js already does. For example, the slidify tests aren't necessary anymore because we don't do that anymore, we let Reveal.js do it all for us. I even tested it with the featured slide option and that also seems to work.

Totally agree, this is so much better. Thanks so much for doing this!

The only thing still unclear for me is the precedence of configs, as it seems to differ for mergedConfig, yamlOptions, slideOptions, ... .

This config setup is a beast. Over the time (reveal-md started 9 years ago), new features have been added, things have been refactored a few times. Another related issue is the diff in local previews vs static generation, hosting that, and diff in relative vs abs paths, etc. Never got it right, guess this should be re-built from the ground up (it's not that much really).

But perhaps it is best to keep this for another PR (the last for 6.0.0 so we can release that to the public)?

Yeah, let's do it. I'll keep an eye on issues that might come in. Gonna test it a bit more and probably merge tomorrow if nothing major/blocking.

What do you think?

Sometimes I forget this little tool has a little group of wonderful users (npm downloads are still ) and they are all worth serving ❤️ That's what I'm thinking :)