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 line breaks #51

Closed
PlanetIrata opened this issue May 11, 2012 · 19 comments
Closed

GFM line breaks #51

PlanetIrata opened this issue May 11, 2012 · 19 comments

Comments

@PlanetIrata
Copy link

GFM line breaks are not supported :

Line 1
Line 2

Sould be rendered as

Line 1<br/>Line 2

When the gfm flag is set to true.

@tiye
Copy link

tiye commented Oct 6, 2012

I do hope there's an option given about using GFM-style line breaks. That is always better to be compatible to someone else. And usually people hit Enter for one time to make line break, isn't it?
I like to use markdown to write notes about programming in Chinese. And I really appreciate to use Markdown to markup my notes. But each Markdown implementation just require two s to make line breaks. That's really inconvenient for me. Please help me.

@firien
Copy link

firien commented Oct 19, 2012

you can force a newline by converting each "\n" => " \n". but this quick and dirty method overrides the gfm option

--- untitled
+++ (clipboard)
@@ -77,6 +77,7 @@

   src = src
     .replace(/\r\n|\r/g, '\n')
+    .replace(/\n/g, '  \n')
     .replace(/\t/g, '    ');

   return block.token(src, tokens, true);

@midnightmonster
Copy link

That's a really, really dirty option that will break all your paragraphs and lists and most everything else.

@tiye
Copy link

tiye commented Oct 19, 2012

I think markdown should use a \ and line end to hint the following line break is converted to a space rather than use space space to make the line break.

@leeoniya
Copy link

the double-space line breaks also bite when your editor is set to auto-trim-trailing during save. <br> is a simple enough solution to this though, with the added bonus of also working within a single h1-6 header.

i agree that a single \ or \n at EOL would have been a much better solution for the original spec.

@firien
Copy link

firien commented Oct 19, 2012

i agree it's very dirty. I wanted to run the tests quick but node test (on a clean master clone) threw an error immediately. It seemed to do the job in some quick browser tests so i figured i'd throw it up here.

GitHub's GFM does something similar, though they seem to be checking for 2 \n

  # in very clear cases, let newlines become <br /> tags
  text.gsub!(/^[\w\<][^\n]*\n+/) do |x|
    x =~ /\n{2}/ ? x : (x.strip!; x << "  \n")
  end

@midnightmonster
Copy link

Thanks--I didn't know where to find GFM's code. That can be directly translated into JS and will produce the same behavior. Looks something like:

src = src
  .replace(/\r\n|\r/g, '\n')
  .replace(/\t/g, '    ')
  .replace(/^[\w\<][^\n]*\n+/mg,function(m){
    return /\n{2}/.test(m) ? m : m.replace(/\s+$/,"")+"  \n";
  });

That's not integrating with the gfm setting at all, but it will duplicate the behavior of the original GFM.

(Edited to add global flag on big regexp; edited to add multi-line flag. This code actually works now: the issue @stof mentioned is fixed my the multiline flag.)

@stof
Copy link

stof commented Nov 27, 2012

@chjj is there any plan to fix this issue ? It is rather annoying for me as I'm using marked in an editor preview whereas the Markedown will later be rendered server-side by Sundown (which handles the GFM line-breaks)

@stof
Copy link

stof commented Nov 27, 2012

@midnightmonster your big regex should not be anchored at the beginning of the string, otherwise it will only handle the first newline of the string instead of handling all of them.

The working solution is:

src = src
  .replace(/\r\n|\r/g, '\n')
  .replace(/\t/g, '    ')
  .replace(/[\w\<][^\n]*\n+/g,function(m){
    return /\n{2}/.test(m) ? m : m.replace(/\s+$/,"")+"  \n";
  });

@buttjer
Copy link

buttjer commented Dec 9, 2012

github-flavored-markdown implemented it with

replace(/  +\n/g," <br />\n")

@midnightmonster
Copy link

@mbuttjer that code just implements the original markdown line breaks.

@vickychijwani
Copy link

I think it is slightly misleading to state that "marked also implements GFM features" in the README. I found - after integrating marked into my application - that all GFM features were supported except the most prominent one (also the one I had come looking for): GFM line-breaks.

Since line break handling is the biggest difference between GFM and strict Markdown (as stated on the GFM page itself), I think marked should either implement it, or else at least mention the lack of support for this feature (and perhaps link to this issue) in the README.

@midnightmonster thanks for that quick hack.

@chjj
Copy link
Member

chjj commented Dec 24, 2012

This feature was left out because markdown already has a brilliant way of dealing with hard line breaks. I feel like I've mentioned this before. Of course it would be easy to implement, but I would want it disabled by default. We'd have to change the options around, because I really don't want a mess of options like: { gfm: true, gfmBreaks: true }.

@vickychijwani
Copy link

@midnightmonster there seems to be some slight error with your hack. Take for example this input:

**foo**
bar
baz

On GitHub, this is rendered as expected:
foo
bar
baz

But using marked and your hack, it renders as:
foo bar
baz

I think it might be because the code posted by @firien is from an old version of GFM. The latest version of the code (taken from here) is:

  # in very clear cases, let newlines become <br /> tags
  text.gsub!(/(\A|^$\n)(^\w[^\n]*\n)(^\w[^\n]*$)+/m) do |x|
    x.gsub(/^(.+)$/, "\\1  ")
  end

@vickychijwani
Copy link

Oops, sorry. Forgot to compare the last commit date on that repository with the gist @firien posted from. So if that gist is indeed the latest version, there must be some other error.

@buttjer
Copy link

buttjer commented Dec 25, 2012

I agree with vickychijwani

The biggest difference that GFM introduces is in the handling of linebreaks. With SM you can hard wrap paragraphs of text and they will be combined into a single paragraph. We find this to be the cause of a huge number of unintentional formatting errors. GFM treats newlines in paragraph-like content as real line breaks, which is probably what you intended.
http://github.github.com/github-flavored-markdown/

I would not call it GFM. Call it chjj markdown ;-)

@vickychijwani
Copy link

Ok, so I've modified @midnightmonster's workaround to resolve the error mentioned in my comment earlier. Basically I've added \>\* to the regex to fix GFM line breaks when the line starts with a > (blockquote) or * (emphasized / strong text).

--- marked.js   2012-12-25 19:03:55.262335239 +0530
+++ marked.2.js 2012-12-25 19:04:17.025666623 +0530
@@ -1,6 +1,6 @@
 src = src
   .replace(/\r\n|\r/g, '\n')
   .replace(/\t/g, '    ')
-  .replace(/^[\w\<][^\n]*\n+/mg,function(m) {
+  .replace(/^[\w\<\>\*][^\n]*\n+/mg,function(m) {
     return /\n{2}/.test(m) ? m : m.replace(/\s+$/,"")+"  \n";
   });

The gist posted on the GFM page (gfm.rb) doesn't seem to be canonical GFM anyway, since GitHub uses redcarpet for actually rendering GFM on the site, and redcarpet is written in C (for instance, the C code for GFM line breaks is here).

@chjj
Copy link
Member

chjj commented Jan 3, 2013

Added in 7aa7934

@chjj chjj closed this as completed Jan 3, 2013
@tiye
Copy link

tiye commented Jan 13, 2013

A bit strange here, but isn't gfm: true was enabled by default?
https://github.com/chjj/marked#usage

➤➤ cat t.md
look at the
pretty line
breaks
➤➤ marked t.md --breaks
<p>look at the
pretty line
breaks
</p>

➤➤ marked t.md --breaks --gfm
<p>look at the<br>pretty line<br>breaks
</p>

➤➤ 

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

9 participants