Skip to content
This repository has been archived by the owner on Apr 3, 2024. It is now read-only.

GitHub list parsing #281

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

GitHub list parsing #281

wants to merge 12 commits into from

Conversation

jamestautges
Copy link
Contributor

@jamestautges jamestautges commented Oct 27, 2016

This isn't complete by any means, but it does implement the basics of turning indentation into list levels, which is half of #165. I copied the implementation github uses as much as I could. There are certain things that markdown-it doesn't like you doing--such as calling the inline parser from a block rule--that are essential to this working properly. It's definitely possible, but it'll take another day or two of jiggery-pokery ✨ 🎩 🐰 ✨.

There are going to be tons of edge cases, so I'd like some help trying to identify them for unit tests.

@revin
Copy link
Collaborator

revin commented Oct 27, 2016

Oh this is awesome! Thank you!

Yes, with regard to edge cases, I think we're going to need to create some kind of list parsing torture test in test/fixtures/, and then fill it up with all kinds of malformed markdown lists 😬

It'll be amazing 😛

@revin revin added the wip label Oct 28, 2016
@ashleygwilliams
Copy link
Contributor

hey @jamestautges ! checking back in - do you need any help working on this? we'd love to see it complete and are happy to help! let us know :)

@ashleygwilliams ashleygwilliams added this to the 10.0.0 milestone Nov 3, 2016
@jamestautges
Copy link
Contributor Author

Sorry everyone. I hit an unexpectedly busy couple of days, but I should have more time this weekend. I'd like to get both of these PRs done before Monday.

@ashleygwilliams
Copy link
Contributor

no worries! was just making sure that you had what you needed, no rush :)

@jamestautges
Copy link
Contributor Author

Alright, that should do it. It ended up being much easier than I thought once I dug into markdown-it's API. This commit includes changes to how code fences are handled in lists, so I think that might address the original issue, but I didn't test it so I'm not confident. Unit test time I guess.

@jamestautges
Copy link
Contributor Author

Oops, broke task lists 😬 . I'll fix it tomorrow.

var prefixCodefence = function (line) {
if (line.length < 3) return 0

// This is how it looks in redcarpet and I didn't feel like making it better
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This cracked me up 😆

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The weirdest part is that they use while (i < 4 && line[i] === ' ') i++ just 40 lines later. But hey, it beats the bit-flags they pass through every parsing function.

@revin
Copy link
Collaborator

revin commented Jan 4, 2017

Hi @jamestautges just checking in now that the holiday season is mostly over. No pressure at all or anything, but I was wondering if you were still planning on trying to get the tests figured out? If you've lost interest and/or moved on that's totally cool; I'm excited about this change though 🎉🎉🎉 so I'm happy to take it over if you like 😄

@revin revin removed this from the 10.0.0 milestone Jun 2, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants