renderer feature for customising output - #129

There are 4 failed test cases, and the master branch failed too.

some references of sundown bindings:

• http://misaka.61924.nl/manual/#toc_14
• https://github.com/vmg/redcarpet#and-you-can-even-cook-your-own
• https://github.com/benmills/robotskirt#custom-renderers

ping ...

@lepture interesting your work :-)
I did testing based on your code. But it's not working

on node v0.8.16 & browser too

var marked = require('./index');
var renderer = new marked.Renderer();

renderer.header = function(text, level) {
   return '<div class="h-' + level + '">' + text + '</div>';
};

var parser = new marked.Parser(marked.defaults, renderer);
console.log(parser.parse('# h1'));

Error: Tokens array requires a `links` property.
    at new InlineLexer (~/Works/test/marked/lib/marked.js:517:11)
    at Parser.parse (~/Works/test/marked/lib/marked.js:833:17)
    at repl:1:20
    at REPLServer.self.eval (repl.js:109:21)
    at rli.on.self.bufferedCmd (repl.js:258:20)
    at REPLServer.self.eval (repl.js:116:5)
    at Interface.<anonymous> (repl.js:248:12)
    at Interface.EventEmitter.emit (events.js:96:17)
    at Interface._onLine (readline.js:200:10)
    at Interface._line (readline.js:518:8)

And README.md example on renderer branch in your github repo too

var renderer = new marked.Renderer();
renderer.blockcode = function(code, lang) {
  if (!lang) {
    return '<pre><code>' + code + '</code></pre>';
  }
  return highlight(code, lang);
};
renderer.header = function(text, level) {
  return '<div class="h-' + level + '">' + text + '</div>';
};

var parse = function(src, options) {
  marked.parser(marked.lexer(src, options), options, renderer);
}
console.log(parse('# h1'));

@rhiokim the example is not clearly. I've updated readme and this issue. try this:

var marked = require('./index')

var renderer = new marked.Renderer()

renderer.header = function(text, level) {
  return '<div class="h-' + level + '">' + text + '</div>'
}


var parse = function(src, options) {
  options = options || {};
  return marked.parser(marked.lexer(src, options), options, renderer);
}

console.log(parse('#h1'))

@lepture thanks :)

ping ....

do want.

@chjj Has any idea?

@lepture, have you benchmarked this? I've considered this before, but the overhead of so many function calls scares me. It doesn't seem like it would be a lot, but when you're calling those functions several thousands of times, it can add a lot of overhead. It all depends on whether v8 does some kind of function inlining optimization there. If the benchmarks check out, I'll merge it.

@chjj Yes, I did the benchmark, it is slower, but not much. The convenience worth it.

Before this pull request:

# test 1
marked completed in 3833ms.
marked (gfm) completed in 3971ms.
marked (pedantic) completed in 3384ms.
# test 2
marked completed in 3726ms.
marked (gfm) completed in 3942ms.
marked (pedantic) completed in 3364ms.
# test 3
marked completed in 3733ms.
marked (gfm) completed in 3922ms.
marked (pedantic) completed in 3379ms.

With this pull request:

# test 1
marked completed in 3727ms.
marked (gfm) completed in 3952ms.
marked (pedantic) completed in 3407ms.
# test 2
marked completed in 3830ms.
marked (gfm) completed in 4052ms.
marked (pedantic) completed in 3480ms.
# test 3
marked completed in 3776ms.
marked (gfm) completed in 3934ms.
marked (pedantic) completed in 3422ms.

On my Macbook Pro. Node v0.10.0

👍 +1 here, would be great for marked to have plugins, because this looks like plugging into marked

@chjj ping ...

+1 (github-like anchors being motivation)

I would really like to see this being merged. If it won't be included, it would be great to have an actively updated fork which would be more feature rich - I see more use for feature rich than super fast Markdown converter.

I see more use for feature rich than super fast Markdown converter.

Completely agree. I think the speed objective is outdated considering that 95% of users are only building a couple of pages, and the people who are converting more than a few pages need more features, 2) marked is usually only a small piece of a larger picture, where features are more important, 3) I've never seen a request to make marked faster (I'm not saying there hasn't been), 4) users keep saying they want these features.

@lepture think you could implement the span-level features to make this feature-complete?

I sent this patch months ago. At that time, this.options.headerPrefix was not added. The renderer feature can do the same thing.

@aleemb I need to make sure that this renderer feature is accepted before adding more features.

I've created a forked project: https://github.com/lepture/markit

Close now.

@chjj when you are ready to accept this feature, please tell me. I would send another pull request.

For now. I am going to maintain https://github.com/lepture/markit

It's not that slow. Here is the benchmark: https://github.com/lepture/markit#benchmark

Here are the results:

markit completed in 3330ms.
markit (gfm) completed in 3356ms.
markit (pedantic) completed in 2897ms.
marked completed in 3181ms.
marked (gfm) completed in 3327ms.
marked (pedantic) completed in 2908ms.
robotskirt completed in 722ms.
showdown (reuse converter) completed in 9926ms.
showdown (new converter) completed in 17152ms.
markdown.js completed in 13733ms.

markit completed in 3290ms.
markit (gfm) completed in 3356ms.
markit (pedantic) completed in 2874ms.
marked completed in 3097ms.
marked (gfm) completed in 3268ms.
marked (pedantic) completed in 2879ms.
robotskirt completed in 721ms.
showdown (reuse converter) completed in 10064ms.
showdown (new converter) completed in 14582ms.
markdown.js completed in 12390ms.

markit completed in 3263ms.
markit (gfm) completed in 3377ms.
markit (pedantic) completed in 2835ms.
marked completed in 3135ms.
marked (gfm) completed in 3279ms.
marked (pedantic) completed in 2842ms.
robotskirt completed in 710ms.
showdown (reuse converter) completed in 11228ms.
showdown (new converter) completed in 17194ms.
markdown.js completed in 13445ms.

I'm very sorry. Will accept. I've been absorbed in other things lately. Merging and refactoring now.

Merged with 7eed150

All tests passing.

I suppose there was an overwhelming demand for this. 59 stars on lepture/markit in one day. Glad it's sorted.

Yeah, well. This is me breaking out of my shell. I believe the thing that made marked successful in the first place was the attention put on speed. marked does so many weird optimizations that I'm worried only I understand (not because I'm smart, but because I wrote them and I do stupidly-designed optimizations that no sane programmer would do).

This is the largest pull request I've ever accepted for marked. So while it's partially true that I've been busy with work and couldn't accept this PR, it's also partially true that I've trained myself to be hesitant about large PRs: I'm worried people may make changes without realizing they're overlooking some esoteric optimization. I wrote marked while running benchmarks after every single change to a line. If I lose a few milliseconds, it's a tragedy to me. I've come to terms with the fact that a renderer is a good idea (much thanks to @lepture for his contribution). Hopefully we can do the same thing with the lexer(s) without too much overhead.

I've also just added @ChrisWren as a core committer for documentation. It's been an eventful night for marked.

Actually, I don't think headerPrefix is a good idea.

@chjj I've already added renderer in InlineLexer. https://github.com/lepture/markit

I think you shouldn't merge this patch. Maybe I can send you another clean one.

I don't think headerPrefix is a good idea.

Agreed. It's not necessary with this

@lepture, I imagine you would prefer that functionality to be handled by the renderer? We can remove it. Being that it was never officially released in an npm verson, we arguably don't have to maintain backward compat. Although, I do worry about a lot of people who simply grab the latest HEAD for their clientside app.

Anyway, maybe we could open a new issue for this.

Also, if nothing else, we should definitely pass the raw text to the renderer function, since the text handled by the inline lexer is a bit harder to sanitize.

@lepture, open another pull request for this and the inline renderer. I'll take a look.

@chjj I will start it at 77352ec

@chjj could you please create a new branch at 77352ec

I will send it to this branch. The master branch is a little mess.

The compare diff: https://github.com/lepture/marked/compare/chjj:77352ec9fc65ae0250c8eb52eaca3236e55f7d3b...feature-renderer?expand=1

marked does so many weird optimizations that I'm worried only I understand

I think you did it very well. The code is very pretty, and easy to understand. I have looked into markdown.js, which is a disaster, I can't figure out where to begin.

I think you did it very well. The code is very pretty, and easy to understand. I have looked into markdown.js, which is a disaster, I can't figure out where to begin.

Thanks. That's good to hear.

I have a branch up at for_lepture if that helps.