Add checkbox support to markdown #3459
Conversation
… add-checkbox-support-to-markdown
| # Checks if the following sequence of characters is a checkbox. | ||
| # A valid checkbox is made up of 3 characters and looks like | ||
| # this `[ ]` or this `[x]` with the latter being a checked one. | ||
| def check_checkbox(str, pos, bytesize) |
There was a problem hiding this comment.
Isn't this method a bit too complex? Why not simply
def check_checkbox(str, pos, bytesize)
return false unless pos + 3 < bytesize
return false unless str[pos] == '['.ord
return false unless str[pos + 1] == ' '.ord || str[pos + 1] == 'x'.ord
return false unless str[pos + 2] == ']'.ord
true
endIs it required to be whitespace-insensitive? Is [ ] (that has more than 1 space but markdown renders it as one) really a checkbox?
There was a problem hiding this comment.
Good point. I figured someone with more knowledge of the markdown parser would be able to give me some helpful feedback since before yesterday I didn't know anything about it.
| @@ -383,11 +384,18 @@ class Markdown::Parser | |||
| when '[' | |||
| unless in_link | |||
| link = check_link str, (pos + 1), bytesize | |||
| checkbox = check_checkbox str, (pos + 1), bytesize | |||
| puts link, checkbox | |||
|
Thanks for the PR! I personally think it's a good addition because it's quite useful, a common implementation uses it, and the character sequence is pretty unambiguous. Does this PR limit checkboxes to the beginning of list items though? I'm pretty sure github does that and it might be a good idea. |
|
This was previously closed as not being Markdown, but a GitHub extension. See #2217. |
|
I'm actually in favor of providing a complete markdown implementation in the standard library, together with extensions (that can be disabled). The current markdown parser is a bit ugly/hacky, I don't know if there's a nice/formal algorithm to parse markdown in its entirety. If someone wants to provide a complete implementation, go ahead :-) |
|
I would say that extensions must be disabled by default —they're unexpected. |
|
@asterite I like that idea a lot. Maybe I'll look into this and see if I can come up with something |
|
Here is a WIP implementation of CommonMark spec and markdown-it extensions. It's written on Crystal but it is a carbon-copy of markdown-it JavaScript-flavored algorithms. I don't think the code size and quality is suitable for Crystal standard lib but it's worth mentioning. There are two reference CommonMark implementations but they are not extensible. I'm putting it here mostly for you to realize how big CommonMark spec is. |
|
I think we'll eventually use |
Fixes #2217 and #3458. Figured I'd see if I could do it and it seems to work. Probably still need to add specs.