realworldocaml / mdx

Execute code blocks inside your documentation
ISC License
265 stars 45 forks source link

Add mld support v2 #423

Closed jonludlam closed 1 year ago

jonludlam commented 1 year ago

On top of #422 - this one is another go at doing mld support.

Looks like quite a lot of tidying has been done since my last attempt, so this one is a much smaller diff! Very nice :-) Not all of the comments on the previous PR (#377) are still valid, but I think I've addressed those that are.

Leonidas-from-XIV commented 1 year ago

@jonludlam Could you rebase upon main now that #422 is merged and add a changelog entry?

jonludlam commented 1 year ago

Rebased.

tmattio commented 1 year ago

Very excited to see this, thanks for the PR @jonludlam!

How do you see this working with Dune? Do you think mdx should default to supporting .md and .mld or should it be opt-in? If the latter, it might be good to document it somewhere in the Dune documentation

Leonidas-from-XIV commented 1 year ago

@tmattio By default dune only checks for *.md files, but when manually passing .mld files, it works out-of-the-box (provided a version of MDX that has this PR merged): https://github.com/ocaml/dune/pull/7569

I think it would be useful to extend the default glob, either in an newer version of the mdx stanza or a newer version of the dune language. Will take this up, but for people wanting to use it now, it's already usable, and will work as soon as 2.3.0 hits opam-repository: https://github.com/ocaml/opam-repository/pull/23670