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

Inconsistent hard line break behavior in GFM (GitHub Flavored Markdown) parser #82

Closed
brandur opened this issue Sep 29, 2013 · 7 comments
Assignees

Comments

@brandur
Copy link
Contributor

brandur commented Sep 29, 2013

I was perusing Kramdown's GFM (GitHub Flavored Markdown) parser, and came across one area that was slightly surprising. It seems that it automatically adds hard line breaks to Markdown it parses (see https://github.com/gettalong/kramdown/blob/master/lib/kramdown/parser/gfm.rb#L17-L20):

      def parse
        super
        add_hard_line_breaks(@root)
      end

This is conspicuously different from Redcarpet's default behavior in this area (which I believe is the closest thing to a GFM reference parser as we have), from Kramdown's documented behavior there too (which is only to insert a hard break only on two spaces or two backslashes at the end of a line), and even from GitHub's behavior while editing Markdown documents in-browser (see this Gist for reference):

irb(main):001:0> require "redcarpet"
=> true
irb(main):002:0> str = <<-eos
irb(main):003:0" The result of this piece of Markdown should
irb(main):004:0" be two paragraphs with this being the first
irb(main):005:0" one.
irb(main):006:0"
irb(main):007:0" This is the second.
irb(main):008:0" eos
=> "The result of this piece of Markdown should\nbe two paragraphs with this being the first\none.\n\nThis is the second.\n"
irb(main):009:0> renderer = Redcarpet::Markdown.new(Redcarpet::Render::HTML.new)
=> #<Redcarpet::Markdown:0x007fbaf2be95d8>
irb(main):010:0> renderer.render(str)
=> "<p>The result of this piece of Markdown should\nbe two paragraphs with this being the first\none.</p>\n\n<p>This is the second.</p>\n"
irb(main):012:0> require "kramdown"
=> true
irb(main):013:0> Kramdown::Document.new(str).to_html
=> "<p>The result of this piece of Markdown should\nbe two paragraphs with this being the first\none.</p>\n\n<p>This is the second.</p>\n"
irb(main):014:0> Kramdown::Document.new(str, input: "GFM").to_html
=> "<p>The result of this piece of Markdown should<br />\nbe two paragraphs with this being the first<br />\none.</p>\n\n<p>This is the second.</p>\n"

Is there a reason that it behaves this way? Can I send a pull to change it or put it in an option?

/cc @plexus

@ghost ghost assigned gettalong Sep 30, 2013
@gettalong
Copy link
Owner

Actually, Github uses both, hard line breaks and the normal Markdown behaviour.
For example, in this comment line breaks are automatically converted to hard
line
breaks.

Whereas in other parts (e.g. conversion of README files) hard line breaks are not used.

Maybe an option to turn off hard line breaks in GFM would be useful?

@plexus
Copy link
Contributor

plexus commented Sep 30, 2013

I based myself on the documentation at https://help.github.com/articles/github-flavored-markdown

But since it's apparently not consitently used everywhere I agree a switch would be useful.

@brandur
Copy link
Contributor Author

brandur commented Sep 30, 2013

Good call guys. I've opened a basic pull materializing this idea at #83. Thanks.

@gettalong
Copy link
Owner

@brandur Thanks for the pull request. Now, what should the default value for this be? Hard line breaks as in the documentation link provided by @plexus or default Markdown behaviour?

@plexus
Copy link
Contributor

plexus commented Oct 26, 2013

I would say take the documented behavior as default. That way if people are
already using GFM it doesn't change how their applications behave.
On 26 Oct 2013 09:08, "Thomas Leitner" notifications@github.com wrote:

@brandur https://github.com/brandur Thanks for the pull request. Now,
what should the default value for this be? Hard line breaks as in the
documentation link provided by @plexus https://github.com/plexus or
default Markdown behaviour?


Reply to this email directly or view it on GitHubhttps://github.com//issues/82#issuecomment-27141009
.

@brandur
Copy link
Contributor Author

brandur commented Oct 27, 2013

@gettalong @plexus Thanks for taking a look guys. I was originally trying to make the interface look like Redcarpet's, where a flag has to be set for hard wrapping to come into effect. +1 on making it the documented default though, if only so that the behavior for existing users doesn't change under them. I've updated my pull to toggle the default.

Slightly unrelated, I added another set of tests, but was slightly surprised when they didn't fail because nothing written in Ruby ever works on the first try. Am I doing it right?

@gettalong
Copy link
Owner

Thanks @brandur for the pull request. I have merged your code now.

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

Successfully merging a pull request may close this issue.

3 participants