commonmark.js icon indicating copy to clipboard operation
commonmark.js copied to clipboard

Feature: Commonmark compliant markdown renderer

Open tmpfs opened this issue 9 years ago • 22 comments

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.

tmpfs avatar Jan 30 '16 06:01 tmpfs

Issue #82 should be resolved first in my opinion.

tmpfs avatar Jan 30 '16 06:01 tmpfs

I've created a commonmark complaint renderer here: https://github.com/nicojs/html2commonmark

Example:

var renderer = new require('html2commonmark').Renderer();
var reader = new require('commonmark').Parser();
var aPieceOfCake = renderer.render(reader.parse('a `piece` of _cake_'));

nicojs avatar Feb 02 '16 09:02 nicojs

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

tmpfs avatar Feb 02 '16 11:02 tmpfs

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

nicojs avatar Feb 02 '16 13:02 nicojs

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 avatar Feb 02 '16 13:02 tmpfs

@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?

nicojs avatar Feb 16 '16 13:02 nicojs

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.

tmpfs avatar Feb 16 '16 14:02 tmpfs

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.

mattyclarkson avatar Oct 03 '18 08:10 mattyclarkson

If you're interested in it, there's interest!

jgm avatar Oct 03 '18 16:10 jgm

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 render/commonmark.js? I can always rebase/refactor into another project later if needed.

mattyclarkson avatar Oct 04 '18 10:10 mattyclarkson

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.

jgm avatar Oct 04 '18 16:10 jgm

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.

mattyclarkson avatar Oct 04 '18 16:10 mattyclarkson

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:

  • commonmark-html
  • commonmark-xml

I also have more renderers here which we should also move to separate packages:

  • https://github.com/mkdoc/mkout/blob/master/lib/text.js
  • https://github.com/mkdoc/mkout/blob/master/lib/yaml.js (useful for debugging the AST)
  • https://github.com/mkdoc/mkout/blob/master/lib/man.js (maybe rename to commonmark-troff)

Happy to work on this some more as I think it is a useful and logical separation of concerns.

tmpfs avatar Oct 05 '18 01:10 tmpfs

Is there any plan to support this feature (based on @tmpfs 's work or other) in a future release?

jeromesimeon avatar Jul 10 '19 14:07 jeromesimeon

I'm afraid I probably dropped the ball on this. @tmpfs what is the status of this? Do you have a commonmark renderer that could just be dropped into this project, for now? We could then revisit the issue of separating out the renderers, in a separate issue.

jgm avatar Jul 11 '19 01:07 jgm

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.

tmpfs avatar Jul 17 '19 00:07 tmpfs

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.

tmpfs avatar Jul 17 '19 00:07 tmpfs

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).

tmpfs avatar Jul 17 '19 01:07 tmpfs

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?

jgm avatar Jul 17 '19 05:07 jgm

@jgm, indeed it is not needed for the markdown renderer. I can strip it out but then I have to manage two versions of the renderer one here and one in mkdoc that relies on JSON ser/deser :(

I will do a pass on extracting it out and issue a PR to see what you think. I think we can close #82 now too!

tmpfs avatar Jul 17 '19 09:07 tmpfs

Great, I'll await a PR.

jgm avatar Jul 17 '19 16:07 jgm

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.

Wilfred avatar Sep 13 '22 22:09 Wilfred