attributes-kit
attributes-kit copied to clipboard
Markdown Rendering Customization
I think it makes sense to ask whether we want to:
- Ignore Markdown rendering as a feature (probably bad)
- Render Markdown using a library of our choice
- Allow custom rendering of Markdown using whatever library a tool is already using
For some tools (e.g. Aglio) it would make a lot of sense to support the third option, since it supports some interesting custom CommonMark extensions. E.g:
The way that I can envision this working is by providing a function to the options parameter passed into the render function. That would take a string of Markdown to be rendered and return the rendered result. For example:
const md = new MarkdownIt();
AttributesKit.render(data, element, {
renderDescription: (input) => md.render(input)
}
What do you think?
@danielgtaylor @Baggz
Ignore the Markdown rendering is, of course, out of question. It would give an incomplete experience. On the same time, I agree that the markdown renderer perhaps should be pluggable.
As a simple test, I've prepared #124 , and the thing was quite easy. Another interesting thing is that the fixture diff is minimal (just a simple space). That's a good news.
By the way, I've also checked other markdown implementations, and more or less the function signature is always the same:
render(markdownCode)
So, I would say that your suggestion (a function into the options object) makes sense, and reverting with a default internal implementation eventually.
Thought?
//cc @Almad Who explicitly asked to be looped into the discussion.
Sorry for the later response guys.
@danielgtaylor @XVincentX I think that having DI for rendering definitely makes sense, even because of better testing, however I'd only encourage changing it for 3rd party users.
Internally, for whatever Blueprint/Apiary ecosystem is using, I'd agree on common renderer and common set of switches/options that are used as a default, and used from something like blueprint-markdown-render library.
Therefore, the optional API would be as you've said:
AttributesKit.render(data, element, {
renderDescription: (input) => md.render(input)
})
however, the "default" usage would be
AttributesKit.render(data, element)
and internally, rendering functions are encouraged to do
import render from 'blueprint-markdown-render';
export default class AttributesKit {
render(data, element, options) {
let mdRender = options.renderer;
if (!mdRender)
mdRender = render;
}
}
I'd move discussion around what features should we enable tooling-wide to that repo.
Does that make sense?
//cc @kylef and @zdne as well, as I think we should agree on this accross the board.
I agree with you. :+1:
@almad What's the status of blueprint-markdown-renderer? Do we have it already, are we crafting it?
No, I am proposing having it, and waiting for you guys to support the idea :)
I do not think there should be such a thing as blueprint-markdown-renderer – it is an engineering overkill that haven't be needed in the past few years. Furthermore it will put more burden on the parser. If we bake a C/C++ renderer into drafter and then create its emscripten version for it we are essentially loosing here. I do not see any benefits here.
Besides I have already said it many times but I want to pivot blueprint away from being a Markdown or GFM Markdown in the favor of plain text.
@zdne I think it have been, as it resulted in inconsistencies in our rendering toolchain.
I am not talking about baking it into drafter/C++ toolchain, I am talking about having agreed default on top of it.
Which means that the Markdown pivot is irrelevant, as I am talking about rendering Markdown blocks only, not the blueprint as a whole.
I agree that we should try and align the markdown parsers used in our own products as @Almad suggests. Settling on something like CommonMark which has a clearly defined specification would be the best case.
Third party tools should have the capability of changing the renderer. If a user would like to explicitly change the parser to add extensions I believe this should be allowed. However, only if the user explicitly enables these features. This allows the default experience of a blueprint to consistently render on all tools unless a user has explicitly changed this behaviour.
What do you think?
When it comes to CommonMark – we need to conduct a research on whereas it supports source maps before we can start thinking about using it in the Markdown Parser this research is long overdue and maybe we should schedule @kylef ...
IIRC hoedown has source maps support for CommonMark. But yes, I think we should schedule it soon.
I am lagging, because as I've looked into it deeper, I feel that I need to resolve few other issues:
- Escaping is different in CommonMark & markdown-it and different than what we currently have in toolchain/documentation, and I think this shouldn't be changed lightly
- markdown-it offers some good plugins, yet uses token stream while CommonMark uses AST
Will look into it deeper this week.