Skip to content

Add special case for closing nested quotes; fixes #1514 #1515

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

Merged
merged 3 commits into from
Mar 20, 2025

Conversation

ChrisMayfield
Copy link
Contributor

This PR adds two more regular expressions to the smarty extension to handle nested quote marks at the end of a sentence, such as "She said 'Hello.'" Additional unit tests are included.

@mitya57
Copy link
Collaborator

mitya57 commented Mar 19, 2025

Looks good, thank you!
Can you please also add a test for #1514 (comment)? It seems to pass with your change:

        self.assertMarkdownRenders(
            '<span>He replied, "She said \'Hello.\'"</span>',
            '<p><span>He replied, &ldquo;She said &lsquo;Hello.&rsquo;&rdquo;</span></p>'
        )

And please add a changelog entry to docs/changelog.md.

@ChrisMayfield
Copy link
Contributor Author

Yes, I thought about adding that test with <span> tags, but none of the existing tests have HTML in the input text, only Markdown. So I instead added a test with parentheses based on facelessuser's comment, which tests a similar situation (closing quotes before non-word character). But I would be happy to add the <span> test as well, if you still prefer. Just let me know.

@facelessuser
Copy link
Collaborator

I think adding the span case is good as well. As noted before, it triggered different logic, and your fix applies to all the broken cases.

@waylan waylan merged commit 7aae61b into Python-Markdown:master Mar 20, 2025
17 checks passed
@waylan waylan added the approved The pull request is ready to be merged. label Mar 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved The pull request is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants