search

mity / md4c

C Markdown parser. Fast. SAX-like interface. Compliant to CommonMark specification.
MIT License
776 stars 146 forks source link

Move HTML renderer from md2html.c to a separate source file #8

Closed craigbarnes closed 7 years ago

craigbarnes commented 7 years ago

I'm currently working on porting an app from Discount to md4c and would like to reuse the HTML rendering code from md2html.c. However, copying the sources into my project repo currently requires manually stripping out code for option parsing, clock() calls, the main() function, etc. and makes it a harder to track future upstream changes.

Would a pull request to move just the rendering code to a separate source file be accepted? If so, is it likely to conflict with any changes you are working on for issue #5?

P.S. Thanks for sharing this great library. I've been wanting to upgrade a few projects to a CommonMark compatible parser for a long time, but couldn't use cmark for lack of tables support. The feature set of md4c really hits the sweet spot I was looking for.

mity commented 7 years ago

Would a pull request to move just the rendering code to a separate source file be accepted?

It would be welcome.

If so, is it likely to conflict with any changes you are working on for issue #5?

Working on #5 will likely take a day or two before I even start touching the rendering side. And even then, changes on the renderer side should be fairly simple and well localized so even if there is a conflict, it should be easy to resolve it. So feel free to go ahead.

mity commented 7 years ago

@craigbarnes I just want to sync with you. I am about to start the work on the renderer side for #5. So if you haven't yet started, then perhaps rather wait a day or two, and if you are close to makeing the pull request, then let me know, and I'll wait until your changes are done and upstreamed.

craigbarnes commented 7 years ago

I started working on it and I think I have a good idea what needs to be done now, but I got to the point where I wanted to rewrite the whole patch and maybe split it into a few separate commits to make it easier to review. So, since I will probably start from scratch anyway, I will wait until after you land the changes for issue #5.

mity commented 7 years ago

@craigbarnes Fix of #5 is committed so feel free to start. I will yet do some testing of it, but even if I encounter some bugs, it should be on the parser side. The changes in the renderer side are quite simple and some refactoring.

craigbarnes commented 7 years ago

I've been thinking about this some more. It seems like the globals in md2html.c need to be replaced with a state struct (that can be passed around as a parameter) before the renderer can be split out into a reusable part. The HTML utility functions are also somewhat difficult to reuse, since they implicitly depend on the membuf type. This is totally reasonable for a command line utility like md2html but I'm starting to think it'd be easier to write a new, reusable renderer from scratch.

I think what I will do for now is work on implementing a custom renderer for my app and when I have something that works maybe try to incorporate it back into the upstream md2html.c.

mity commented 7 years ago

Ok.

Maybe I will take a look and if I find an easy way, I'll try to do something with it.

mity commented 7 years ago

I think I have found quite a straightforward way so it is done. Take a look whether it is usable for you.

There is still some space for improvement, in particular the output callback could return an int which would be propagated through all the functions in render_html.c into parser so the renderer callback could abort prematurely, but I was too lazy to do that in a single step.