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

🐛 FIX: Task list item marker can be followed by any GFM whitespace #42

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions mdit_py_plugins/tasklists/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
# ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
# OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.

import re
from typing import List
from uuid import uuid4

Expand Down Expand Up @@ -144,8 +145,4 @@ def is_list_item(token):

def starts_with_todo_markdown(token):
# leading whitespace in a list item is already trimmed off by markdown-it
return (
token.content.startswith("[ ] ")
or token.content.startswith("[x] ")
or token.content.startswith("[X] ")
)
return re.match(r"\[[ xX]][ \t\n\v\f\r]+", token.content)
Copy link
Member

Choose a reason for hiding this comment

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

Why not use \s here, to match whitespace? (see https://docs.python.org/3/library/re.html)

Copy link
Member Author

Choose a reason for hiding this comment

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

We can do that, but then have to set the re.ASCII flag, else we match too much. I listed all the chars so that they are easy check the against the GFM spec which does so too: https://github.github.com/gfm/#whitespace-character

Copy link

Choose a reason for hiding this comment

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

The CommonMark specification (because that is the reference here!) does the same: https://spec.commonmark.org/0.29/#whitespace-character .

Copy link
Member

Choose a reason for hiding this comment

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

😂 can you a link to this white space definition in a comment or the docstring

Copy link
Member Author

Choose a reason for hiding this comment

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

The latest CM spec is 0.30 and that doesn't have the same character set. The latest spec only has "unicode whitespace".

I would recommend only referring to the latest GFM spec here, since task lists have nothing to do with CM.

Copy link
Member Author

Choose a reason for hiding this comment

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

can you a link to this white space definition in a comment or the docstring

Sure. I think I'll make it a private variable or something.

Copy link

@mdeweerd mdeweerd Mar 2, 2022

Choose a reason for hiding this comment

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

Well, the new CM spec is not fully updated then - it still refers to whitespace in the examples without defining it: https://spec.commonmark.org/0.30/#example-350 .

I was actually thinking why this should not defined as a constant because it has just proven to change over time... . GFM_WHITESPACE, CM_WHITESPACE.

Edit: which you did 😃

22 changes: 22 additions & 0 deletions tests/fixtures/tasklists.md
Original file line number Diff line number Diff line change
Expand Up @@ -107,3 +107,25 @@ oedered.md:
</ol>

.

Tab after task list item marker
.
+ [x] item 1
+ [ ] item 2
.
<ul class="contains-task-list">
<li class="task-list-item"> item 1</li>
<li class="task-list-item"> item 2</li>
</ul>
.

Form feed after task list item marker
.
+ [x] item 1
+ [ ] item 2
.
<ul class="contains-task-list">
<li class="task-list-item"> item 1</li>
<li class="task-list-item"> item 2</li>
</ul>
.