Skip to content

Support any markdown parser via option#167

Merged
acrobat merged 1 commit intorefactory-id:masterfrom
mhuggins:support-all-markdown-libraries
Apr 6, 2015
Merged

Support any markdown parser via option#167
acrobat merged 1 commit intorefactory-id:masterfrom
mhuggins:support-all-markdown-libraries

Conversation

@mhuggins
Copy link
Copy Markdown
Contributor

@mhuggins mhuggins commented Apr 4, 2015

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
Copy link
Copy Markdown
Contributor

acrobat commented Apr 4, 2015

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

@mhuggins
Copy link
Copy Markdown
Contributor Author

mhuggins commented Apr 4, 2015

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
Copy link
Copy Markdown
Contributor

acrobat commented Apr 4, 2015

Parser would indeed be a better option I think!

@mhuggins
Copy link
Copy Markdown
Contributor Author

mhuggins commented Apr 5, 2015

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 mhuggins force-pushed the support-all-markdown-libraries branch from c43e9a1 to fced7dc Compare April 5, 2015 14:37
@mhuggins
Copy link
Copy Markdown
Contributor Author

mhuggins commented Apr 5, 2015

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
Copy link
Copy Markdown
Contributor

acrobat commented Apr 5, 2015

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

@mhuggins
Copy link
Copy Markdown
Contributor Author

mhuggins commented Apr 5, 2015

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
Copy link
Copy Markdown
Contributor

acrobat commented Apr 5, 2015

@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
Copy link
Copy Markdown
Member

toopay commented Apr 6, 2015

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

@acrobat
Copy link
Copy Markdown
Contributor

acrobat commented Apr 6, 2015

As far as I can see BC breaks are avoided!

@toopay
Copy link
Copy Markdown
Member

toopay commented Apr 6, 2015

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

@acrobat
Copy link
Copy Markdown
Contributor

acrobat commented Apr 6, 2015

Thanks @mhuggins!

acrobat added a commit that referenced this pull request Apr 6, 2015
@acrobat
Merge pull request #167 from mhuggins/support-all-markdown-libraries
c2ab87f 
Support any markdown parser via option
@acrobat acrobat merged commit c2ab87f into refactory-id:master Apr 6, 2015
@acrobat
Copy link
Copy Markdown
Contributor

acrobat commented Apr 6, 2015

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

@mhuggins
Copy link
Copy Markdown
Contributor Author

mhuggins commented Apr 7, 2015

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

@ChrisGPen
Copy link
Copy Markdown

ChrisGPen commented Jul 30, 2016

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.

@ChrisGPen ChrisGPen mentioned this pull request Aug 6, 2016
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mhuggins @acrobat @toopay @ChrisGPen