yogthos / markdown-clj

Markdown parser in Clojure
Eclipse Public License 1.0
540 stars 120 forks source link

parsing of references and footnodes more functional #202

Closed habruening closed 3 months ago

habruening commented 3 months ago

I changed the implementation of parse-references and parse-footnotes. It is now more functional style.

Functionally I changed nothing. All tests still pass. But it looks like the tests do not cover the cljs version. So in case you accept this pull request, please have an extra look to my changes in core.cljs.

I don't know if you like that PR. Feel free to ignore it, as it does not change or fix anything functionally.

Regarding the performance, I don't know if my change makes a difference. I guess so, because the atoms add extra code for the synchronisation, which is not really needed here. If you want the best performance, you could try to use map (which then could be optimised a lot by the compiler) and then only use reduce for the construction of the result. But without knowing if that makes a difference, it would be premature optimisation.

What probably will make a performance difference, if you parse the references, footnotes and metadata all together.

My motivation was originally to make markdown-clj a parser that emits data structures and not just a text transformation. But I don't know yet if I will do that. it looks like this was never the intention for markdown-clj.

yogthos commented 3 months ago

Thanks, I like the change, it's a cleaner way to do this. I don't think performance is a huge concern here either.