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

html.unescape in tokenizer #135

Merged
merged 6 commits into from
Feb 14, 2022

Conversation

not-my-profile
Copy link
Contributor

This PR fixes that HTML entities end up in the LaTeX output by the LaTeXRenderer. For details see the individual commits.

If you like these changes, please merge them without squashing.

README.md Outdated Show resolved Hide resolved
Python 3.6 has already reached end-of-life,
so supporting 3.3 doesn't make any sense.
mistletoe/span_token.py Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@pbodnar pbodnar left a comment

Choose a reason for hiding this comment

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

Looks great, just some smaller details to tune.

Previously & ended up in the LaTeX when using
the LaTeXRenderer, which is obviously wrong.
Previously the HTML unescaping took place in the HTMLRenderer.
The LaTeXRenderer didn't do it so e.g. HTML entities in links
ended up in the LaTeX output. The correct way is to resolve
the HTML entities during the tokenization.
It does not make sense to put test-specific code in the code that runs
in production.  Escaping ' as ' is perfectly fine and CommonMark
compliant.  (The official CommonMark test suite actually performs HTML
normalization by default).
Now that the method just calls html.escape,
using it doesn't make sense anymore.
@pbodnar pbodnar merged commit ce8ac0a into miyuchina:master Feb 14, 2022
@pbodnar
Copy link
Collaborator

pbodnar commented Feb 14, 2022

Thank you. :)

BTW You may have noticed that I'm a fan of conventional commits, if you would like to give it a try. But I won't force it on you. ;)

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