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

GFM parser: list compatibility #336

Closed
tsl0922 opened this issue May 14, 2016 · 19 comments
Closed

GFM parser: list compatibility #336

tsl0922 opened this issue May 14, 2016 · 19 comments

Comments

@tsl0922
Copy link
Contributor

tsl0922 commented May 14, 2016

markdown source

* level 1
  some text

  begin level 2
  * level 2
  * level 2

level 2 can be rendered by github:

  • level 1
    some text

    begin level 2

    • level 2
    • level 2

but not kramdown:

image

if a new line is inserted after begin level 2, kramdown can render it correctly

@gettalong gettalong self-assigned this May 14, 2016
@gettalong
Copy link
Owner

As you wrote at the end: You need to insert a newline before starting a list. This is how kramdown defines its list syntax - see http://kramdown.gettalong.org/syntax.html#lists.

@tsl0922
Copy link
Contributor Author

tsl0922 commented May 14, 2016

@gettalong I mean: can we add support for this? or for the gfm parser? since github support this.

@gettalong
Copy link
Owner

This is probably not going to happen, at least not for kramdown itself. Because it would mean changing the way block paragraphs and lists are parsed.

@tsl0922
Copy link
Contributor Author

tsl0922 commented May 14, 2016

ok, thanks!

@lrytz
Copy link

lrytz commented Jul 6, 2016

It's quite unfortunate to have differences between GitHub's parser and kramdown's GFM.

A
  - b

renders as a bullet on GitHub, but not in kramdown + GFM. This keeps tripping us up, we often write articles / release notes on GitHub and then need to fix them when publishing them on the website.

@gettalong gettalong changed the title nested list compatibility GFM parser: nested list compatibility Jul 6, 2016
@gettalong gettalong changed the title GFM parser: nested list compatibility GFM parser: list compatibility Jul 6, 2016
@gettalong
Copy link
Owner

@lrytz Pull requests to implement this are welcome.

@gettalong gettalong reopened this Jul 6, 2016
@tsl0922
Copy link
Contributor Author

tsl0922 commented Jul 6, 2016

Hi, @gettalong @lrytz

Since this issue is reopened, I'd like to share the other incompatible syntax I found for the GFM parser, for example:

# h1
## h2
### h3

renders with kramdown like this:

<h1 id="h1">h1</h1>
<p>## h2
### h3</p>

but github renders it correctly:

h1

h2

h3


Another issue, it seems that kramdown does not check html tag name character strictly:

<中文>

renders with kramdown like this:

<中文>
</中文>

this is conflict with the html syntax: https://www.w3.org/TR/html-markup/syntax.html#tag-name

tag names are used within element start tags and end tags to give the element’s name. HTML elements all have names that only use characters in the range 0–9, a–z, and A–Z.

but github handles this correctly too:

<中文>

And the auto-closing tag feature causes trouble for something like this:

<中文>

# h1
## h2
### h3

kramdown renders it as:

<中文>

# h1
## h2
### h3

</中文>

@gettalong
Copy link
Owner

The kramdown syntax require blank lines between block level elements and since the GFM parser is just a specialized kramdown parser, it follows the same rules except where changed. Therefore headers also need blank lines preceding them.

As for the tag parsing: kramdown parses XML tags and handles those that resemble HTML tags like HTML. As any letters are allowed in XML, not just A-Za-z, tags that use them are parsed correctly.

And the auto-closing feature is there to make sure that the output is a correct XHTML document.

@tsl0922
Copy link
Contributor Author

tsl0922 commented Jul 7, 2016

Since it is a GFM parser, I think it should be compatible with GFM as much as possible. This makes sense to me, and the github pages users, too. I'm trying to migrate the markdown parser on our site from redcarpet to kramdown but failed because of the issues above. We can't change our user's data,
so it would be better to make changes to the markdown parser.

You said it would mean changing the way block paragraphs and lists are parsed, so can you provide something like an option to disable this behavior which can be used in the GFM parser?

@gettalong
Copy link
Owner

I agree that it would be beneficial if kramdown's GFM parser would behave the same as "the" Github GFM parser. However, there is no spec, so this probably won't happen.

Since kramdown itself works as intended, only the GFM parser needs to be changed and therefore there is no need for an option. And I can't just disable it, the code would need to be rewritten to conform more to the GFM way.

@Tuckie
Copy link

Tuckie commented Jul 7, 2016

@gettalong I just wanted to take a moment thank you for your work on this project. If you're not on Github's payroll, you should be. (Or at least I hope they talked with you before making the switch.)

My only request when evolving GFM would be to allow for some of the GFM features (quirks?) to be optionally disabled in the GFM parser in addition to optionally enabling them in the other parsers too.

@gettalong
Copy link
Owner

@Tuckie Thanks, this is very appreciated!

As for your request: I'm open for options regarding the (de)activation of certain features when using the GFM parser (as done with the paragraph hard wrap option).

As for other parsers: There is really just only one other lightweight markup parser, the main kramdown parser. The GFM parser as well as the "plain" Markdown parser are based on this kramdown parser.

My intent with the kramdown parser was and still is that any kramdown document can be correctly parsed with it without knowing which optional parsing features need to be activated or deactivated. Therefore there are only a handful of options regarding the kramdown parser and if I could I even would remove them as I think it was a mistake adding them in the first place.

Long story short: The kramdown parser won't be customizable in the future; however, the HTML output or any other output may receive further options to customize them.

@gettalong
Copy link
Owner

This issue is fixed by the pull request #358.

@gettalong
Copy link
Owner

@Tuckie I have added an option 'gfm_quirks' for enabling/disabling certain GFM parsing quirks - please have a look at 5dd782f if this is what you wanted.

@Tuckie
Copy link

Tuckie commented Aug 9, 2016

@gettalong That's definitely headed down the right track, but truthfully, I was more interested in the other half: utilizing some of the GFM quirks in other parsers.

For me an ideal parser setup would have a shared set of transforms/methods/settings that can be en(dis)abled (with appropriate defaults) for any parser, which in turn would create the different markdown flavors (kramdown/markdown/gfm/etc) or something new. This could potentially provide a better injection point for custom markdown functionality as well.

@gettalong
Copy link
Owner

@Tuckie I understand what you mean but this is completely the opposite of what I have in mind for the kramdown parser: One parser that reads one format so that anyone having written a document for kramdown can rest assured that it will be parsed correctly by the kramdown parser.

If there were options to enable or disable certain parsing functionalities, this might not be the case.

However, if you look at how the GFM or Markdown parsers of the kramdown library work, you will find that what you want is already possible. For example, have a look at https://github.com/gettalong/kramdown/blob/master/lib/kramdown/parser/markdown.rb#L34 where you will find that the Markdown parser is created by subclassing the kramdown parser and removing functionality.

@Tuckie
Copy link

Tuckie commented Aug 12, 2016

@gettalong Thank you for the time to discuss. While I fully appreciate the desire to work towards a standard for markdown, I believe that there will always be instances where domain-specific behavior is needed in order to increase ease of use for a particular application.

@gettalong
Copy link
Owner

@Tuckie For what it's worth, I have planned for the 2.x release to be better in this regard (e.g. by not using constants etc.)

@Tuckie
Copy link

Tuckie commented Aug 15, 2016

Sounds great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants