Opened 13 years ago

Last modified 5 months ago

#6135 assigned New feature

Introduce a short-cut for template filters that has needs_autoescape = True

Reported by: anonymous Owned by: Seth Thoburn
Component: Template system Version: master
Severity: Normal Keywords: autoescape
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


After the autoescaping was introduced I had to rewrite all filters to comform new requirements: put is_safe=True and needs_autoescape=True where needed.

I have written about 25 filters that have needs_autoescape=True and absolutely of them contained contained the same strings

def filter(text, autoescape=None): 
    if autoescape: 
        esc = conditional_escape 
        esc = lambda x: x 
    text = esc(text)

   # Then goes the part which is different for all filters

    return mark_safe(result)

I have checked the filters in django. All the same.
I think it is way to much of writting.

I propose to split property needs_autoescape = True on two properties:

needs_manual_autoescape = True, then Do all the stuff above, manually.

needs_autoescape = True, then all above steps will do template system for user. All this thinks can do a simple decorator as well.

Change History (10)

comment:1 Changed 13 years ago by anonymous

Forgot to mention that above code, which very common and annoying to write, can be transformed to

def filter(text, autoescape=None): 
filter.needs_autoescape = True

Or even simplier version

def filter(text): 
   #Actually the jod done
filter.needs_automatic_autoescape = True

comment:2 Changed 13 years ago by Jacob

I think the decorator approach is the best way. I'd expect it to work like this:

def filter(text):

comment:3 Changed 13 years ago by Eratothene

In case if

def filter(text):

It will be inconsistent with filter.is_safe = True
In one case we have decorator (autoescaped), in other a property (filter.is_safe).
I think we should stick to one syntax, either decorator or propertry.

One way (Backward incompatible):

def filter(text):

def filter(text):

def filter(text, autoescape=None):

Another way (Backward Compatible):

def filter(text):
filter.needs_autoescape = True

def filter(text):
filter.is_safe = True

def filter(text, autoescape=None):
filter.needs_manual_autoescape = True

comment:4 Changed 13 years ago by Eratosfen

I prefer the decorator syntax more than existing one

comment:5 Changed 13 years ago by Simon G <dev@…>

Triage Stage: UnreviewedDesign decision needed

comment:6 Changed 13 years ago by Malcolm Tredinnick

Triage Stage: Design decision neededAccepted

Comment 3 is mistaken. The is_safe property performs quite a different function to the needs_autoescape attribute (which is really just a flag). There's no inconsistency imposed by changing needs_autoescape functionality into a decorator. Also, the decorator is probably fully backwards compatible, since all it will do is add the needs_autoescape attribute to the resulting function and accept the extra argument (plus doing the initial processing). It's an addition to needs_autoescape (turning that into an implementation detail, for the most part), not a replacement. Filters are possible which don't subscribe to the pattern mentioned, so the decorator approach shouldn't be the only way to do this -- don't remove needs_autoescape, just implement a helper, in other words.

Jacob's decorator name doesn't feel right, though. Perhaps autoescape_aware is a better name. The function itself isn't a past-tense of anything.

comment:7 Changed 9 years ago by Gabriel Hurley

Severity: Normal
Type: New feature

comment:8 Changed 9 years ago by Jannis Leidel

Easy pickings: unset
UI/UX: unset

#16726 was a dupe.

comment:9 Changed 9 months ago by Johannes Hoppe

Needs tests: set
Patch needs improvement: set

comment:10 Changed 5 months ago by Seth Thoburn

Needs tests: unset
Owner: changed from nobody to Seth Thoburn
Patch needs improvement: unset
Status: newassigned
Note: See TracTickets for help on using tickets.
Back to Top