Comment 9 for bug 2024325

Revision history for this message
Sudip Mukherjee (sudipmuk) wrote :

> a) In presentation.py, you are leaving the "from jinja2 import Markup" bit, and again, later down, importing it again via "from .html import Markup", which is a change you added.
>
> That comes from https://trac.edgewall.org/changeset/17543, but note that that diff also *removed* the Markup import from jinja2:
> -from jinja2 import Markup, Undefined, contextfilter, evalcontextfilter
> +from jinja2 import Undefined, contextfilter, evalcontextfilter
>
>
> Could you please review these changes, and the import changes overall in the patch?

Looks like I messed up here. I added the old debdiff and forgot to add the final debdiff which had this change. Adding the debdiff here for your reference, and will be great if you can reject the upload please.

>
> b) If I understood this correctly, this is the big import change:
> soft_unicode is no longer imported from jinja2.utils, but now comes from
> markupsafe *either* as soft_unicode, or renamed from soft_str. What's
> the story here? What guarantees do we have that these modules are
> accurate replacements? Did jinja2.utils remove soft_unicode in some
> version, and if yes, in which?

Yes, jinja2.utils removed soft_unicode in v3.0.0 and you can see the change at:
https://github.com/pallets/jinja/commit/b0015c72d5acbf93b9d99a1ce6167889338db85b#diff-99868302c0e75a5c181912aab0e396266e2ce25fd7b0b8f29684df74eef49984L718

The original function in jinja2.utils was just doing:
def soft_unicode(s):
    from markupsafe import soft_unicode
    return soft_unicode(s)

So, it was basically just returning soft_unicode from markupsafe.

From the pov of markupsafe, soft_unicode has been renamed as soft_str and you can see the change at:
https://github.com/pallets/markupsafe/blob/2.0.1/src/markupsafe/_native.py#L70

So, these should be accurate replacements.