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

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

Closed
laukstein opened this issue Apr 29, 2019 · 11 comments
Closed

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

laukstein opened this issue Apr 29, 2019 · 11 comments

Comments

@laukstein
Copy link

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
Copy link
Collaborator

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
Copy link
Author

laukstein commented Apr 29, 2019

@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
Copy link
Collaborator

@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
Copy link
Author

laukstein commented Apr 29, 2019

@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
Copy link
Collaborator

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
Copy link
Author

@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
Copy link
Author

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

@aidantwoods
Copy link
Collaborator

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

@laukstein
Copy link
Author

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
Copy link
Author

And thanks @aidantwoods for so quick responses!

@aidantwoods
Copy link
Collaborator

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: #601 (comment)). 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 #708, but still a couple bits and pieces that need ironing out before that'll be ready (also needs documenting :) )

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

2 participants