-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
loose lists #1304
loose lists #1304
Conversation
@@ -365,6 +372,10 @@ Lexer.prototype.token = function(src, top) { | |||
if (!loose) loose = next; | |||
} | |||
|
|||
if (loose) { | |||
listStart.loose = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this if
statement be removed in favor of listStart.loose = loose
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no because if loose
is false and listStart.loose
is true I don't want it to change listStart.loose
to false.
listStart.loose
is true if any of listStart
items are loose
Since I am removing the token |
@@ -1217,28 +1238,20 @@ Parser.prototype.tok = function() { | |||
} | |||
case 'list_item_start': { | |||
body = ''; | |||
var loose = this.token.loose; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason why you create a new var here instead of using this.token.loose
below?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because this.token
is changed when this.next()
is called in the while loop below.
body += !this.token.loose && this.token.type === 'text'
would be checking if the text token is loose not the list start token
@joshbruce @davisjam Can you review this PR? @Martii Can you check to see if this solves your issue? Updated, I confirmed it solves that use case ^ |
Maybe in a little bit I'll have some time... but the Marked demo seems to not be giving me a preview in a clean browser profile. The HTML source that is selectable looks different than I recall. |
And now it appeared for preview in a new tab... love quirks like this. n/m |
@Martii I had a chrome extension that was causing an issue with the demo. Disabling the extension fixed it. That being said, this PR is not released yet and therefore not available in the demo right now. |
I know... that's why I made the suggestion of having a current HEAD option. Was doing a bazillion things yesterday and never got around to testing the current HEAD myself.
...
;) So it was something else... perhaps the CDN wasn't delivering? Anyhow... probably something for a different issue if it continues to crop up. :) Thanks. |
Marked version: 0.4.0
Markdown flavor: CommonMark
Description
Makes "loose" lists conform to the standard, requiring all items in a loose list to be loose.
This conflicts with #1 in which @chjj takes his own approach to loose lists, which I don't think comes from any standard. My proposed solution to the failing non-standard tests would be to just remove the tests.
Contributor
Committer
In most cases, this should be a different person than the contributor.