search

wooorm / markdown-rs

CommonMark compliant markdown parser in Rust with ASTs and extensions
https://docs.rs/markdown/1.0.0-alpha.21/markdown/
MIT License
906 stars 50 forks source link

Make `Options` (De)serialisable with serde #70

Open byhemechi opened 1 year ago

byhemechi commented 1 year ago

I've added basic serde attributes to the Config struct and its dependencies, only within the serde feature.

Please have a brief check that i've not missed any renames so that we stay in sync with micromark

Feel free to be rude if i've done anything stupid

All existing tests pass though I haven't written any new ones yet

byhemechi commented 1 year ago

please don't merge this yet, i'm yet to set up the defaults properly

it should be good now, dont merge without checking though of course

byhemechi commented 1 year ago

The check fail is on a file outside the scope of this PR so i'm not going to fix it here

wooorm commented 1 year ago

yeah don’t worry about that, that was my commit just now, will fix soon!

wooorm commented 1 year ago

Thanks! Code looks good, I do wonder if there should be some tests? And, should be documented in the features docs: https://docs.rs/markdown/1.0.0-alpha.9/markdown/#features

byhemechi commented 1 year ago

I've adjusted the docs, will work on tests very soon. Are you ok with pulling in serde_json as a dev dependency for said tests?

wooorm commented 1 year ago

Sure!

byhemechi commented 1 year ago

Alright, doing tests now. To be entirely honest i don't know if these test actually achieve anything as all they are really doing is testing serde behaviour, which is already in their tests and is obviously covered by semver

wooorm commented 1 year ago

I guess you can check if it works, and that there is a specific string being generated?

byhemechi commented 1 year ago

yeah, that's what i'm ending up doing :)

testing deserialisation is mildly annoying though because nothing is Eq

wooorm commented 1 year ago

last I remember you were still working on this, right? Or is it done?

byhemechi commented 1 year ago

Sorry, got busy with life and didn't quite finish it :)

I've written some of the tests but once again they aren't really needed for coverage, serde has tests on the macros and the structs themselves already have tests