Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

added tasklist extra and unit test #218

Merged
merged 2 commits into from
Jun 24, 2016
Merged

added tasklist extra and unit test #218

merged 2 commits into from
Jun 24, 2016

Conversation

liguangsheng
Copy link
Contributor

added tasklist extra and unit test.

See Issue #216

@nicholasserra
Copy link
Collaborator

Looks good, thank you :D

@nicholasserra nicholasserra merged commit 70a28df into trentm:master Jun 24, 2016
This was referenced Jun 24, 2016
(.*) # list item text = \2
''', re.M | re.X | re.S)

_task_list_warpper_str = r'<p><input type="checkbox" class="task-list-item-checkbox" %sdisabled>%s</p>'
Copy link
Contributor

Choose a reason for hiding this comment

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

In my opinion, the <p> tag is unnecessary.
We can also have a look at github flavour markdown parser result

<li class="task-list-item"><input type="checkbox" class="task-list-item-checkbox" checked="" disabled=""> text body </li>

Copy link
Contributor

@thealphadollar thealphadollar left a comment

Choose a reason for hiding this comment

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

Please look into this.

def _task_list_item_sub(self, match):
marker = match.group(1)
item_text = match.group(2)
if marker == '[x]':
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe we should also take into account the situation where an uppercase X is used since Github markdown also accepts that.

For e.g.

  • this is marked
  • this is also marked

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants