Skip to content

pull_request(list): prevent extra p tags inside list items #9

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Kristinita
Copy link

@Kristinita Kristinita commented Feb 15, 2025

1. Summary

I’ve fixed issue #8. python-markdown-comments will now remove unwanted <p> tags inside the list.

2. Explanation

I added the deletion of unwanted <p> tags with a regular expression.

Also, I added 3 tests from my issue. They work successfully after running the python -m unittest test/test_comments.py command and don’t break existing tests.

3. Issue #2 possible problem

Difficulties arose with how to leave the test_comments_in_html test asserted. With my pull request, this test remains asserted, but I didn’t really understand everything written in issue #2, and I admit that I might have missed something. If so, I think it would be a nice idea to add other tests for issue #2.

Thanks.

@Kristinita Kristinita changed the title Fix #8 pull_request(list): prevent extra p tags inside list items Feb 15, 2025

# Remove unwanted <p> tags around list items.
# See issue #8 for details.
text = re.sub(r'<li>\s*<p>(.*?)</p>\s*</li>', r'<li>\1</li>', text)
Copy link
Owner

Choose a reason for hiding this comment

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

Per discussion in #8, the thing that makes me uncomfortable with this solution is that I believe this will "correct" undesirable output caused by blank lines and not just by comments. IMO that would not be a great side-effect because it would encourage people to write bad markdown by hiding their mistakes from them.

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

Successfully merging this pull request may close these issues.

2 participants