Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GitHub markdown #107

Closed
HarpyWar opened this issue Jul 1, 2013 · 7 comments
Closed

GitHub markdown #107

HarpyWar opened this issue Jul 1, 2013 · 7 comments

Comments

@HarpyWar
Copy link

HarpyWar commented Jul 1, 2013

Hello, thanks for great implementation.

Is it possible to add full support of github markdown?
For example, tag ``` is ignored.

@jjok
Copy link

jjok commented Jul 1, 2013

You should extend the Markdown class with a GithubMarkdown class, with the additional functionality, if you can.

@HarpyWar
Copy link
Author

HarpyWar commented Jul 1, 2013

Could you please show me a sample code how to add a new tag, so I could start.

@michelf
Copy link
Owner

michelf commented Jul 1, 2013

Code blocks are notoriously difficult to add because the HTML block parser needs to know how to avoid them. If you want to have an idea you can check how fenced code blocks (delimited by ~~~) are implemented in PHP Markdown Extra.

I don't intend to add Github-flavored fenced code blocks to PHP Markdown Extra, but having a third parser class for Github-flavored markdown could work.

@bmcminn
Copy link

bmcminn commented Aug 7, 2013

I'm in favor of the GFM class extension, however I also find it's trivial to do this as-is with a simple regex modification.

On line #2770

<?php
  // change this
  ~{3,} # Marker: three tilde or more.

  // to this
  [~`]{3,} # Marker: three (tilde|acute|etc) or more.

It's overly simplistic, but I tested this yesterday and it works pretty well. The one caveat being if you use any combination of these characters of length 3+ (eg: ~``~~) it triggers, but I personally don't think that's a deal breaker.

My one suggestion may be to provide a code fence string config option for the MarkdownExtended object so that you can define what characters delineate a code fence.

@michelf
Copy link
Owner

michelf commented Aug 7, 2013

@bmcminn It appears to work pretty well, but it'll break if you put HTML code inside. The HTML block parser needs to be aware of the syntax for it to work correctly, and because that's a somewhat perilous task (because it parses things very differently) it'd need a battery of tests. And the general way block-level elements are parsed needs to be revised rethought to avoid a couple of existing loopholes in that department.

Also, your proposed change would allow ridiculous things like ~~` as a marker.

@bmcminn
Copy link

bmcminn commented Aug 13, 2013

I know it's not very structured in this approach, however I don't believe it's all that big an issue since I don't foresee anyone using ` as a their typical code fence.

Also, my use of this modification hasn't broken in regards to parsing HTML code inside a code fenced block. That was the first thing I tested and it rendered just fine. Although my markup snippet was fairly simple. So far since changing this one line I haven't noticed any breaking changes, however I do understand there is need for a more robust solution.

I still think allowing a user defined Class/Parser level config for the code fence string or even a REGEXP to be checked would be a considerable approach.

@michelf
Copy link
Owner

michelf commented Nov 28, 2013

Merged pull #132. The feature is in HEAD now.

@michelf michelf closed this as completed Nov 28, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants