search

realworldocaml / mdx

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

`Normal` -> `Markdown` rename #412

Closed Leonidas-from-XIV closed 1 year ago

Leonidas-from-XIV commented 1 year ago

Normal is a very strange name for the markdown syntax and nobody calls it "normal syntax". It's just a supported syntax of MDX, just like Cram and MLI are (and there's nothing un-normal about thous), although it did come first.

This PR renames the constructor name. It might be user-facing if users use the MDX library, hence it has a changelog entry.

I've also removed the aliases of the syntax type. This can cause some breakage if users expect Mdx.Normal/Mdx.Cram to exist, so I've added it as a separate commit so if we decide against shipping it the commit can be removed from the PR.

panglesd commented 1 year ago

If you want to turn this PR into: "Introduce equality between syntaxes", you can also change the cli, where the description of the binary is:

NAME
       ocaml-mdx - Execute markdown files.
[...]

and

NAME
       ocaml-mdx-test - Test markdown files.
[...]

and similarly for the package description!

Leonidas-from-XIV commented 1 year ago

in the of_string function, I guess we do not want to remove it for compatibility reasons.

Exactly. I tried to keep the function of the binary at least equivalent, so users calling mdx should not observe regressions.

Good point about the API. I am not entirely sure who uses MDX as a library besides Dune, but in my research Dune didn't seem to use Normal either way (amusingly, it does have a Normal constructor too, but used for something else), so this should be safe enough. And the earlier we fix this, the less breakage will happen afterwards, I would argue.

I agree with the documentation changes, these should be updated so I pushed changes to clarify the meaning of the binaries. Generally it would be good to look though the documentation and adjust wherever it only talks about Markdown (and misses out on Cram, MLI or soon MLD support), but this can happen any time without triggering any issues (unlike this PR which at least has a low but nonzero likelihood of breakage).