erusev / parsedown

Better Markdown Parser in PHP
https://parsedown.org
MIT License
14.74k stars 1.12k forks source link

[Deprecation][v1.8.0-beta-7] List broken when used new line #714

Closed laukstein closed 5 years ago

laukstein commented 5 years ago

Latest Parsedown beta version 1.8.0-beta-7, breaks list markdown containing new line like

1. a

  b

turning to

<ol>
<li>
<p>a</p>
</li>
</ol>
<p>b</p>

While it works fine in latest stable v1.7.3, returning

<ol>
<li>
<p>a</p>
<p>b</p>
</li>
</ol>
aidantwoods commented 5 years ago

The result given by 1.7.3 isn't correct—the line with "b" needs to be indented by one more space so that it matches up with the original list content's indentation.

laukstein commented 5 years ago

@aidantwoods, you are wrong. I just rechecked it in https://parsedown.org/demo and I switched to Parsedown v1.7.3, and it works as I mentioned above.

And Markdown PHP 1.3 results

<ol>
<li><p>a</p>

<p>b</p></li>
</ol>
aidantwoods commented 5 years ago

@aidantwoods, you are wrong. I just rechecked it in https://parsedown.org/demo and I switched to Parsedown v1.7.3, and it works as I mentioned above.

The result given by Parsedown 1.7.3 isn't correct (not for). I agree that Parsedown gives the result you mentioned, however it is this result which is incorrect.

The CommonMark reference parser indeed agrees with the new (1.8.x-beta) result, giving:

<ol>
<li>a</li>
</ol>
<p>b</p>

To see why this is the case, see the spec description under example 224, which says:

The most important thing to notice is that the position of the text after the list marker determines how much indentation is needed in subsequent blocks in the list item. If the list marker takes up two spaces, and there are three spaces between the list marker and the next non-whitespace character, then blocks must be indented five spaces in order to fall under the list item.

laukstein commented 5 years ago

@aidantwoods, having space/s or not, 1.8.0-beta-7 currently fails on it and doesn't wrap inside <li>.

Isn't there a bug in commonmark.js, it doesn't support multiple paragraphs inside lists, it doesn't depend how many spaces I add?

aidantwoods commented 5 years ago

As mentioned, it needs to be indented to match the indentation of the list content where the marker begins, like this:

Screenshot 2019-04-29 at 20 37 56
laukstein commented 5 years ago

@aidantwoods, sorry, You are right! With right spacing, spec wraps correctly

https://spec.commonmark.org/dingus/?text=1.%20a%0A%20%20%20%0A%20%20%20b

1. a

   b

results

<ol>
<li>
<p>a</p>
<p>b</p>
</li>
</ol>

Meaning Parsedown and Markdown PHP doesn't yet fallow the latest spec.

laukstein commented 5 years ago

Or are you saying that 1.8.x-beta fallows the latest spec?

aidantwoods commented 5 years ago

1.8.x-beta follows spec here as far as I can tell, did you have an example where it differs from CommonMark?

laukstein commented 5 years ago

Sorry @aidantwoods, it was my mistake. I got confused by latest stable vs beta resulting different results. 1.8.x is resulting fine based on latest spec.

Any ETA for 1.8.x release as stable?

laukstein commented 5 years ago

And thanks @aidantwoods for so quick responses!

aidantwoods commented 5 years ago

Sorry @aidantwoods, it was my mistake. I got confused by latest stable vs beta resulting different results. 1.8.x is resulting fine based on latest spec.

No worries :)

Any ETA for 1.8.x release as stable?

I ended up deciding not to release 1.8.x as a minor upgrade due to quite a few internal changes that might break extensions (context: https://github.com/erusev/parsedown/issues/601#issuecomment-396345464). If you're not using any extensions then I'd just go ahead and use 1.8.x for now (it contains quite a few bug fixes over 1.7.x).

I'm also holding off releasing it as a major upgrade since there are a couple things in the public API that, given the opportunity to change, will make things a lot better. There's quite a bit of progress towards 2.0.x over in https://github.com/erusev/parsedown/pull/708, but still a couple bits and pieces that need ironing out before that'll be ready (also needs documenting :) )