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 hard line breaks don't follow spec #1380

Closed
superhawk610 opened this issue Nov 27, 2018 · 9 comments
Closed

GFM hard line breaks don't follow spec #1380

superhawk610 opened this issue Nov 27, 2018 · 9 comments

Comments

@superhawk610
Copy link

Describe the bug
GFM spec describes the following as a hard line break:

foo··
bar

and should translate to the following HTML

<p>foo<br />bar</p>

Similarly, the following is a soft line break (note the lack of 2 trailing spaces):

foo
bar

and should translate to the following HTML

<p>foo\nbar</p>

This spec should be followed when gfm and breaks are both enabled, however enabling breaks will always output the first style (hard line breaks), even when the trailing spaces are omitted.

To Reproduce
RunKit example

Expected behavior
When gfm and breaks are enabled, the second example (soft line break) should not output a <br /> tag.

foo
bar

should output

<p>foo\nbar</p>
@superhawk610
Copy link
Author

Relevant GFM spec:
Hard line breaks
Soft line breaks

@styfle styfle added L1 - broken Valid usage causes incorrect output OR a crash AND there is no known workaround for the issue parser: GFM labels Nov 27, 2018
@styfle
Copy link
Member

styfle commented Nov 27, 2018

Yes this appears to be a bug.
I'm surprised these are not in our tests.

function testOptions(opts) {
  var md632 = 'foo  \nbaz';
  var md633 = 'foo\\\nbaz';
  var md647 = 'foo\nbaz';
  var md648 = 'foo \n baz';
  console.log(opts);
  console.log(marked(md632, opts));
  console.log(marked(md633, opts));
  console.log(marked(md647, opts));
  console.log(marked(md648, opts));
  console.log('\n');
}

testOptions({gfm: true, breaks: true});
testOptions({gfm: true, breaks: false});

@joshbruce Any idea why gfm.0.2.8.json is missing these tests? How did you acquire this file?

@UziTech
Copy link
Member

UziTech commented Nov 27, 2018

testing
this

@UziTech UziTech closed this as completed Nov 27, 2018
@UziTech UziTech reopened this Nov 27, 2018
@UziTech
Copy link
Member

UziTech commented Nov 27, 2018

Sorry clicked the wrong button...

Apparently GitHub doesn't follow the GFM spec.

If you look at the source of the comment above github does add a <br/> but there were no spaces after "testing"

image

@UziTech
Copy link
Member

UziTech commented Nov 27, 2018

The "breaks" option was added to make the "gfm" option function like github (i.e. add <br> on single \n) #51

With the "breaks" option off it will follow the spec.

@UziTech UziTech removed the L1 - broken Valid usage causes incorrect output OR a crash AND there is no known workaround for the issue label Nov 27, 2018
@UziTech
Copy link
Member

UziTech commented Nov 27, 2018

@styfle gfm.0.28.json only has tests for things that are different between GFM 0.28 and CommonMark 0.28

The hard and soft line breaks are the same in both specs (even though github doesn't follow them)

@UziTech
Copy link
Member

UziTech commented Nov 27, 2018

Looks like the real problem is that our documentation for the "breaks" option is wrong.

@Feder1co5oave
Copy link
Contributor

Agree with @UziTech.
How many of those hard line-breaks test cases do pass?

@UziTech
Copy link
Member

UziTech commented Nov 28, 2018

according to the Common Mark spec tests 100% pass. And just testing manually 100% pass with gfm: true, breaks: false

|Hard line breaks | 15 of 15 | 100%|
|Soft line breaks | 2 of 2 | 100%|

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