-
-
Notifications
You must be signed in to change notification settings - Fork 226
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature: Commonmark compliant markdown renderer #83
Comments
Issue #82 should be resolved first in my opinion. |
I've created a commonmark complaint renderer here: Example: var renderer = new require('html2commonmark').Renderer();
var reader = new require('commonmark').Parser();
var aPieceOfCake = renderer.render(reader.parse('a `piece` of _cake_')); |
Thanks for the input Nico but this discussion is about this library and it's ability to support converting back to Commonmark from the AST generated. Your implementation here: https://github.com/nicojs/html2commonmark/blob/master/src/MarkdownRenderer.ts Is not in Javascript. We would like a Javascript version built on top of this Javascript reference implementation. Maybe you have some ideas about how the AST needs to be modified to support the currently missing data? Please see: http://talk.commonmark.org/t/issues-to-resolve-before-1-0-release/1287/25 |
My version is indeed written in TypeScript, which is transpiled to JavaScript before use. As you can see: git clone https://github.com/nicojs/html2commonmark
cd html2commonmark
npm install
npm install -g grunt-cli
grunt build
vim dist/server/MarkdownRenderer.js I did my work based on the cmark reference implementation (as was suggested to me in this comment by @jgm). I didn't know a markdown renderer was wanted, otherwise i would have created a pull request. EDIT: I don't checkin the dist folder as it is build output, however the dist folder is published in the npm package |
My bad Nico, maybe you can let us know if there is anything missing from the AST that would help to get the job done, I am personally keen to get all the relevant info in the AST so that implementing a Markdown renderer that reproduces a document as close to the original as possible is an easy task. |
@tmpfs ok, let's say i can find the time to do this. You want the list as a comment on this issue? Or somewhere else? |
Thanks @nicojs, I have added a little to this thread: http://talk.commonmark.org/t/issues-to-resolve-before-1-0-release/1287/26?u=tmpfs What I think we need to work out is all the things missing from the AST that at the moment prevent a clean round trip to commonmark, if you were able to add your experiences there I think it would be invaluable. |
I really would like a commonmark renderer so that I can create a commonmark formatter CLI. Is there still interest in a markdown renderer? Seems like #82 is moving forward well. |
If you're interested in it, there's interest!
|
Excellent. Would it make sense to wait for #82 splits the renders out into separate projects or shall I get a PR together to submit a |
I don't think there's been any progress on #82 for
over a year (though I do think it's a good idea).
Perhaps you could contact @tmpfs and see if they
want help on it?
One might have reservations about adding a commonmark
renderer as part of commonmark.js, since that adds to
the package size and it's a feature not everyone needs.
|
I would agree. I feel like all the renders should be an opt in from a package. I'll have a look at #82 to get up to speed. |
Hello folks, I have had it done and working (in production) for quite a while now, I was awaiting guidance from @jgm on how to proceed creating separate renderer npm packages. I have just merged upstream into my fork here (all tests continue to pass): https://github.com/tmpfs/commonmark.js and use these renderers in my markdown tools here: https://github.com//mkdoc/mkdoc. My feeling is that we should bundle the abstract renderer and then separate out into:
I also have more renderers here which we should also move to separate packages:
Happy to work on this some more as I think it is a useful and logical separation of concerns. |
Is there any plan to support this feature (based on @tmpfs 's work or other) in a future release? |
I'm afraid I probably dropped the ball on this. @tmpfs what is the status of this? |
I think it is ok to go, we verified performance is acceptable quite a while ago. I have been using the markdown and man renderers for my tooling in production for quite a while and the HTML renderer is a straight port of your original code. If they pass all the tests and benchmarks are stable i think we can merge into this repo and then make a plan to migrate to separate npm modules (this would be a breaking change and i think would warrant a major version bump). @jgm, Let me know if you want to try it and i will merge upstream and check everything is good, then issue a PR. |
Looking at the code here: https://github.com/mkdoc/mkout/blob/master/lib/markdown.js it does not look like it would take too much work to bring it into this repository. I do not recall if this code relies on the changes in #96 and #97, I would need to check. |
So the markdown renderer relies on some code I wrote to allow serializing and deserializing the AST to JSON which is here: https://github.com/mkdoc/mkast/blob/master/lib/node.js. I think I would need to remove that to bring in the markdown renderer or @jgm do you see any value in ser/des the AST to JSON (I was piping JSON between executables). |
I think serializing to JSON might be useful for some purposes, but why would that be needed for the markdown renderer, which has access to the unserialized AST? |
Great, I'll await a PR. |
In the meantime, this is possible with remark as a workaround. For example, https://github.com/remarkjs/remark-inline-links rewrite links in markdown source and converts back to markdown source. |
This issue represents the existing discussions on updating the AST to support a Commonmark compliant renderer.
Related forum discussions:
http://talk.commonmark.org/t/issues-to-resolve-before-1-0-release/1287
http://talk.commonmark.org/t/customizable-renderer-output-in-commonmark-js/1146/12
Originally discussed in #6.
The text was updated successfully, but these errors were encountered: