refactory-id / bootstrap-markdown

Bootstrap plugin for markdown editing
Apache License 2.0
1.99k stars 371 forks source link

Support any markdown parser via option #167

Closed mhuggins closed 9 years ago

mhuggins commented 9 years ago

This permits any parser to be used regardless of its interface. My reason for wanting this is that I'd like to use markdown-it since it is stricter, as both markdown.js and marked share the same XSS vulnerability. The problem is that markdownit is not referred to as markdown or marked in the window namespace, and its function for converting markdown to HTML is named render rather than toHTML.

This change will allow a new parse option to be supplied when calling bootstrap-markdown's $.fn.markdown method, which is a function definition that is called in place of the existing options. (I maintained the existing options for backward compatibility.) For example:

var md = markdownit();

$('[data-provide="markdown-editor"][data-preview]').markdown({
  parser: md.render.bind(md),
  onChange: function (e) {
    var $preview = $(e.$textarea.data('preview'));
    $preview.html(e.parseContent());
  }
});
acrobat commented 9 years ago

Looks like a better way of parsing the content! Without the restriction of specific libs

mhuggins commented 9 years ago

Yeah, I don't know if I like the option name of parse, maybe parser or something would be better. Definitely open to revise this, but I'd like to have the option of using a third party regardless, primarily with my XSS concern. :)

acrobat commented 9 years ago

Parser would indeed be a better option I think!

mhuggins commented 9 years ago

Branch updated! Not sure if I have to recreate the pull request? I thought committing to the same branch would update the PR automatically (or at least used to), but apparently not anymore.

mhuggins commented 9 years ago

I rebased the branch, and that seems to have made the pull request update. Not sure if GH changed their approach or something since the PR didn't update with the additional commit before squashing. Weird!

acrobat commented 9 years ago

Normally the PR should update, did you push the changes? But a rebase/squash and force push does the job too!

mhuggins commented 9 years ago

Yeah, I definitely did! I was surprised it wasn't updating just as well. I verified the changes were showing in the commit history on the GH branch. Wondering if there was a callback issue on GH's servers or something.

acrobat commented 9 years ago

@toopay, @lodev09 you guys are ok with the name of the option and the possibility to defined your custom parser instead of the build-in parsers?

toopay commented 9 years ago

@acrobat As long as it doesn't break compatibility, i'm happy.

acrobat commented 9 years ago

As far as I can see BC breaks are avoided!

toopay commented 9 years ago

@acrobat If you doesn't have any further suggestion to @mhuggins than your last review, i'll let you push the green button.

acrobat commented 9 years ago

Thanks @mhuggins!

acrobat commented 9 years ago

@mhuggins can you just send a PR to the gh-pages branch to add some docs about this feature?

mhuggins commented 9 years ago

Happy to do this! I'll try to do it as soon as I get home from work tomorrow. :) Thanks for merging!

ChrisGPen commented 8 years ago

I would like to use this feature Support any markdown parser via option #167, but can not find the documentation for it. Can you tell me where I can find it? Thanks.