Make HTML renderer more customizable

[jgm]

It would be nice if renderers for individual elements (e.g. links) could be customized by setting properties of the renderer object.

[EmptyStar]

If it's of any interest, I've implemented changes in my own fork of commonmark.js that allows for customizing the output of the HTML renderer. Please let me know if the API is a good fit for this feature and if there's any desire to eventually adopt it into the main commonmark.js repository. Note that I haven't been part of the CommonMark community, so my changes are completely independent of any discussions or recommendations on how this feature should or should not be implemented.

[jgm]

@EmptyStar, can you briefly describe the approach you're taking?

[EmptyStar]

@jgm, with pleasure. The API doc I linked explains it best, but basically, it's an event-based implementation. Think of it similar to SAX parsing. You start by creating an object that implements a set of methods which are called when certain elements are encountered during rendering. That object is then passed as the policy option in the HtmlRenderer constructor.

As a quick example, say you wanted to uppercase all text. You would create an object like the following:

var myPolicy = {
    text: function(str) { // Called when text is found
        return str.toUpperCase(); // Return value is appended to output
    }
    // Other methods omitted for brevity
};

Then you'd pass it into your HtmlRenderer like so:

var html = new commonmark.HtmlRenderer({
    policy: myPolicy
});

The HtmlRenderer will pass any plain text it encounters into the text method of your rendering policy, and that method's return value gets appended to the output (except when coerced to boolean false). The same is true for the tag method and other policy methods. The HtmlRenderer sends all text and tags through the policy, including both raw HTML and Markdown-to-HTML output.

[jgm]

So, if I understand correctly: instead of producing text and tags directly, the HtmlRenderer calls functions from the policy which produce these? And these functions can do things like omit certain tags, add or remove attributes, uppercase text, etc.

My idea of customization was a bit different, and more simpleminded. The thought was to provide a dictionary of functions that get called for rendering each kind of node. (A few shared state variables would also be needed.) These would correspond to the cases in the switch statement at html.js line 71ff. This would make it easy, for example, to replace the code block renderer with a custom one.

What do you think are the advantages and disadvantages of this, with respect to your approach?

[EmptyStar]

Yes, it's actually the policy object in my implementation that produces the output. HtmlRenderer simply transforms the AST nodes into HTML symbols and delegates rendering to the policy object.

Your approach seems like it would give users more direct access to the raw data than my API. This would give users more freedom in how to handle raw HTML. It also aligns with the CommonMark spec which would give users a good reference for what the function for each node represents.

However, I think your API would have to change whenever the CommonMark spec changes. If a new feature becomes standard, or if an existing feature changes, then the API would have to change to accommodate. This would affect API consumers and necessitate versioning the API along with the spec.

My approach attempts to reduce all input to a limited set of symbols (text, tag, declaration, instruction, comment, cdata). In the case of new features, the HtmlRenderer would simply translate the new features into HTML. This seems like a more uniform and future-proof interface that wouldn't need to change as CommonMark evolves. The exception is if the spec recognizes a new HTML block type or a non-HTML feature.

However, preserving raw HTML blocks is impossible with my API since the HtmlRenderer parses them into their component parts. The API also requires all methods to be implemented even if only minor changes to the output are desired (though this can be mitigated with abstract or default implementations). I think it might also be possible to translate from your API to mine in which case our approaches are not mutually exclusive.

Sorry for the long comment. What are your thoughts?

[jgm]

+++ EmptyStar [Mar 31 15 19:00 ]:

However, I think your API would have to change whenever the CommonMark spec changes. If a new feature becomes standard, or if an existing feature changes, then the API would have to change to accommodate. This would affect API consumers and necessitate versioning the API along with the spec.

This is a good point. On the other hand, my method extends more naturally to non-HTML output formats.

I suppose the API will have to change anyway when new elements are added to the spec, at least if the list of allowable values for the "type" field are considered part of the API. People who do custom AST manipulations or write custom renderers will still need to change their code in response to such changes. But you are right that your method would give a stabler experience for most users.

[EmptyStar]

I agree with you 100%. After giving it some more thought, I understand the merits of having an API like you described. It's more true the spec—which commonmark.js is an implementation of—and it could easily accommodate other output formats. The spec and commonmark.js are married by nature, so changes to the spec will necessarily affect the API. This is a simple fact of the matter, and as a reference implementation, so it should be.

What I intend to do at this point is create a branch within my commonmark.js fork and experiment with an API like you've described. I think it may work well as a more generic "renderer" API which other renderer subclasses can build upon. My own "HTML policy" API could be one such subclass that I could split off into its own project as a sort of plugin.

I'm new to GitHub, but if there's a way to continue the discussion over there, it might be a more appropriate place to discuss and design a solution to this issue. If not, then I'll be glad to collaborate in whatever way is best.

Lastly, thanks for hearing me out, and thanks for all of your hard work on this project!

[tmpfs]

@jgm

I would like to chime in with my thoughts on this if I may. Having extended marked to render to man, txt and round-trip to markdown I am now moving to Commonmark as it makes so much sense (thanks by the way, a spec is long overdue).

The first thing I am trying to do is round-trip to markdown so am adding a MarkdownRenderer, thankfully your design is much nicer than marked (I ended up having to fork the entire code base rather than implement one class) and should be much easier to create my renderer (wip).

However, whatever is decided by this discussion would clearly have an impact upon my work. I suggest that you implement a Renderer that performs the walk of the tree and then either:

1. Dispatch named events for each node type
2. Call a member method for each node type

I don't really mind which to be honest; I see that with 1) there would be some difficulties moving to the browser by relying on events.EventEmitter but it would be less maintenance (just emit by type and pass the event object) whereas with 2) you would be declaring an abstract member method when a new type is defined.

Using events would allow multiple formats to listen on the same walk of the tree which would be useful when outputting multiple formats simultaneously.

What are your thoughts on this approach? I am happy to do some work on this if you would like to advise on the direction you prefer to take.

[jgm]

+++ muji [Jan 27 16 20:24 ]:

[1]@jgm

I would like to chime in with my thoughts on this if I may. Having
extended marked to render to man, txt and round-trip to markdown I am
now moving to Commonmark as it makes so much sense (thanks by the way,
a spec is long overdue).

The first thing I am trying to do is round-trip to markdown so am
adding a MarkdownRenderer, thankfully your design is much nicer than
marked (I ended up having to fork the entire code base rather than
implement one class) and should be much easier to create my renderer
(wip).

However, whatever is decided by this discussion would clearly have an
impact upon my work. I suggest that you implement a Renderer that
performs the walk of the tree and then either:

1. Dispatch named events for each node type

2. Call a member method for each node type

   I don't really mind which to be honest; I see that with 1) there would
   be some difficulties moving to the browser by relying on
   events.EventEmitter but it would be less maintenance (just emit by type
   and pass the event object) whereas with 2) you would be declaring an
   abstract member method when a new type is defined.

   Using events would allow multiple formats to listen on the same walk of
   the tree which would be useful when outputting multiple formats
   simultaneously.

   What are your thoughts on this approach? I am happy to do some work on
   this if you would like to advise on the direction you prefer to take.

Both approaches sound reasonable to me.

Note that we already have an iterator interface
(node.walker()) that allows you to walk the tree and emit
events with minimal fuss. That makes me wonder whether
there'd be much added value in implementing the kind of
generic event-based renderer you're describing. It's
already pretty easy to implement a custom writer using a
"walker."

[tmpfs]

My thinking was that commonmark would just supply a Renderer and then HtmlRenderer and XmlRenderer (and all the other renderers) would be in separate packages. This would make everything much leaner (for the browser). If you just need the HtmlRenderer there is no need to include the XmlRenderer code for example.

The Renderer defines the contract (interface) either with events or methods.

[jgm]

+++ muji [Jan 27 16 21:40 ]:

My thinking was that commonmark would just supply a Renderer and then
HtmlRenderer and XmlRenderer (and all the other renderers) would be in
separate packages. This would make everything much leaner (for the
browser). If you just need the HtmlRenderer there is no need to include
the XmlRenderer code for example.

I see the point. Well, I'm open to ideas about what a
generic Renderer module might look like.

If it's going to be something that generates events, then it
might not be much beyond the walker, which we already have.
IF you want to sketch something up I could take a look.

[jgm]

Further note on this. If you look at
src/man.c or src/commonmark.c in jgm/cmark, you'll see the
generic interface I desigend in cmark for writing renderers.

It includes macros CR (to ensure that the next content
starts at the beginning of a line, if it isn't already),
BLANKLINE (to make sure the next content is separated by
exactly one blank line from what came before), LIT (for
literal unescaped content), and OUT (for possibly escaped
and possibly wrapped content). This interface also allows
you to add line prefixes to a stack (e.g. > or
indentation) and handles hard-wrapping text to a fixed
width. (This is tricky because when you're generating
commonmark you have to escape certain characters at the
beginning of a line, but not elsewhere.)

Creating a generic interface that makes it easy to write
renderers for formats like Markdown is a lot harder than
creating one that is only going to target xml/html.

[tmpfs]

IF you want to sketch something up I could take a look.

Thanks for being open to this idea, I sketched something yesterday:

https://github.com/tmpfs/mk3/blob/e485e6c4e0ba166244657398357a1fd6fafb4065/lib/render/renderer.js

Decided to use member methods rather than events as it has less dependencies. Whilst both the implementations are very much work in progress you can see the difference, the original (walker() and switch style):

https://github.com/tmpfs/mk3/blob/e485e6c4e0ba166244657398357a1fd6fafb4065/lib/render/markdown-commonmark-style.js

And the class that inherits from Renderer:

https://github.com/tmpfs/mk3/blob/e485e6c4e0ba166244657398357a1fd6fafb4065/lib/render/markdown.js

Whilst this is really just a stylistic thing, if I am going to write quite a few renderers (I hope to implement man, pdf and txt as well as commonmark markdown) I personally find the inheritance approach cleaner to work with and maintain than the raw walker() style.

Apart from allowing separation of the concrete renderers from the main commonmark package it also allows for inheritance writing renderers, encouraging code re-use where possible.

Thanks for the pointers in your additional comment.

It includes macros CR (to ensure that the next content starts at the beginning of a line, if it isn't already),

This I have not done yet, but will implement (it was on my mind), I'll take a look at the C implementation.

BLANKLINE (to make sure the next content is separated by exactly one blank line from what came before)

So far more or less good on this, it means the document always ends with a blank line at the moment but this could be fixed, I am taking the position that all block level elements add this single blank line which seems to work ok, but I'll take a look at the logic in the reference implementation.

The LIT and OUT macros could be abstract and provided by Renderer so implementation methods instead of returning a string (to be concatenated on to the buffer) could call out() or lit() and they would concat on to this.buffer and the render() method would return this.buffer rather than the local variable. I think that would fulfill the need for differentiating between literal and potentially escaped content. And to be honest I prefer it to returning a string and testing against undefined in render().

Creating a generic interface that makes it easy to write renderers for formats like Markdown is a lot harder than creating one that is only going to target xml/html.

Quite possibly, the main difficulty I run into with this is handling nested lists correctly and especially ordered lists and the index when other nested lists are between ordered list items. I might ask (haven't looked into the code) if the _listData object could maintain an index property for this purpose. First I'll see if it's easy to implement outside of the existing code.

[tmpfs]

An updated sketch is here:

https://github.com/tmpfs/mk3/blob/3714d3b2ce4a5ca11e70073d8b7ff5b00cddb99b/lib/render/renderer.js
https://github.com/tmpfs/mk3/blob/3714d3b2ce4a5ca11e70073d8b7ff5b00cddb99b/lib/render/markdown.js

Which adds the out() and lit() member methods and does not use the function return values but requires them to call out() or lit().

[jgm]

+++ muji [Jan 28 16 17:39 ]:

BLANKLINE (to make sure the next content is separated by exactly one
blank line from what came before)

So far more of less good on this, it means the document always ends
with a blank line at the moment but this could be fixed, I am taking
the position that all block level elements add this single blank line
which seems to work ok, but I'll take a look at the logic in the
reference implementation.

The reference implementation avoids any extraneous blank
lines, e.g. at the end of the document or the end of a block
quote. You don't want

> foo
>
> bar
>

next paragraph