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

Refs #27753 -- Deprecated django.utils.text.unescape_entities(). #11277

Merged
merged 1 commit into from May 8, 2019
Merged

Refs #27753 -- Deprecated django.utils.text.unescape_entities(). #11277

merged 1 commit into from May 8, 2019

Conversation

jdufresne
Copy link
Member

The function was undocumented and only required for compatibility with Python 2.

Use Python's html.unescape() instead -- available since Python 3.4.

https://docs.python.org/3/library/html.html#html.unescape

https://code.djangoproject.com/ticket/27753

Copy link
Contributor

@bluetech bluetech left a comment

Choose a reason for hiding this comment

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

The code changes look good to me FWIW.

A GitHub code search shows that the function is inevitably used by some projects: https://github.com/search?q=django+unescape_entities&type=Code. So maybe it'd be nice to keep unescape_entities around for one deprecation cycle (just calling html.unescape)?

docs/releases/3.0.txt Outdated Show resolved Hide resolved
@jdufresne jdufresne changed the title Refs #27753 -- Removed django.utils.text.unescape_entities(). Refs #27753 -- Deprecated django.utils.text.unescape_entities(). May 4, 2019
@jdufresne
Copy link
Member Author

Thanks for the review. As suggested, I've changed the PR to simply deprecate the function rather than removing it. I also added a note about lazy strings.

@felixxm
Copy link
Member

felixxm commented May 7, 2019

@jdufresne Thanks for this PR 👍 Please add also a note to the docs/internals/deprecation.txt.

@felixxm felixxm self-assigned this May 7, 2019
@jdufresne
Copy link
Member Author

Done. 🙂

@@ -411,6 +411,9 @@ Miscellaneous
* ``alias=None`` is added to the signature of
:meth:`.Expression.get_group_by_cols`.

* ``django.utils.text.unescape_entities()`` is deprecated in favor of
:func:`html.unescape`. For lazy strings, first coerce using ``str()``.
Copy link
Contributor

Choose a reason for hiding this comment

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

For lazy strings, first coerce using str()

That's not exactly right. With unescape_entities, given a lazy string, it is not evaluated immediately, but returns a lazy string which will unescape when it is evaluated. With html.unescape, the argument is evaluated immediately and a string is returned.

If you want to keep a note, I'd say "Note that unlike django.utils.text.unescape_entities(), html.unescape will evaluate lazy strings immediately".

Copy link
Member Author

Choose a reason for hiding this comment

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

Excellent point. I've used your wording in the latest revision.

The function was undocumented and only required for compatibility with
Python 2.

Code should use Python's html.unescape() that was added in Python 3.4.
@felixxm felixxm merged commit b915b9f into django:master May 8, 2019
@jdufresne jdufresne deleted the unescape-entities branch May 9, 2019 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants