Opened 6 years ago

Last modified 4 years ago

#10107 new New feature

mark_safe and mark_for_escaping as decorators

Reported by: matehat Owned by: matehat
Component: Template system Version: 1.0
Severity: Normal Keywords:
Cc: mathieu.damours@…, mmitar@… Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I think the two safety markers located in django.utils.safestring could be improved a bit to help DRYing up a redundant situation. Say some function of a model or whatever class instance is often used in templates and instead of applying the safe filter to it everywhere it appears in the template code, the returned value is marked as safe. Now the following pattern can be DRYed up:

def some_method(self, arg):
  if arg == 1:
    return mark_safe(some_tags)
  elif arg == 2:
    return mark_safe(some_other_tags)
  elif arg == 3:
    return mark_safe(some_third_tags)

Sure there are better ways to organize this type of pattern but it shows my point. Instead of appying the marker everywhere data is returned, it could be used as a decorator on the whole method:

@mark_safe
def some_method(self, arg):
  if arg == 1:
    return some_tags
  elif arg == 2:
    return some_other_tags
  elif arg == 3:
    return some_third_tags

Anyway, I made the necessary patch, so take a look at it! The same is applied to the mark_for_escaping method for the same reasons.

Attachments (3)

safestring.diff (2.0 KB) - added by matehat 6 years ago.
patch.2.diff (5.0 KB) - added by matehat 6 years ago.
patch.diff (5.5 KB) - added by matehat 6 years ago.
New patch for regressiontests, documentation and django.utils.safestring

Download all attachments as: .zip

Change History (13)

Changed 6 years ago by matehat

comment:1 Changed 6 years ago by matehat

  • Needs documentation unset
  • Needs tests unset
  • Owner changed from nobody to matehat
  • Patch needs improvement unset

comment:2 Changed 6 years ago by mtredinnick

  • Triage Stage changed from Unreviewed to Accepted

This isn't a bad idea. There are some improvements that jump out at me from reading the patch, however:

  1. Code must be Python 2.3 compatible, which means you can't use a decorator in any of this code (you're using one for one of the nested functions).
  2. Don't worry about allowing the case of @mark_safe(). That's just poor style, since the decorator/function never takes any arguments. People can get by with only writing @mark_safe. That removes the need for s=None, too, which avoids accidental errors.
  3. Patch needs to include documentation changes to describe the new usage.
  4. Patch needs to include test changes, so that we can verify it continues to keep on working in the future.

Changed 6 years ago by matehat

comment:3 Changed 6 years ago by matehat

The new patch includes fixes for python 2.3 compatibility and a test case for testing escaping behavior (though for a unknown reason, the file's content doesn't show up in trac ... I can only see its content when downloading it ...) I'm still unsure where I should put the documentation for this: in 'Custom template tags' (It's not quite designed for this) or in '(Template) Syntax overview' (though turning autoescaping off is only mentioned there for the template side of coding, i.e. for template designers).

comment:4 Changed 6 years ago by matehat

  • Needs documentation set

comment:5 Changed 6 years ago by dc

Maybe offtopic, but in your case better use function with one exit point:

def some_method(self, arg):
  if arg == 1:
    tags = some_tags
  elif arg == 2:
    tags = some_other_tags
  elif arg == 3:
    tags = some_third_tags
  return mark_safe(tags)

comment:6 Changed 6 years ago by matehat

Yes, of course :P, I made up a simplistic situation that is rather a 'reduced' version of more complex cases, where a very complicated processing sometimes happen. Of course it can always be rearranged the way you suggested, but that can also make things less straightfoward.

comment:7 Changed 6 years ago by matehat

Sorry about the long time in adding patch to documentation for using mark_safe and mark_for_escaping as decorators. The new patch includes tests and documentation.

Changed 6 years ago by matehat

New patch for regressiontests, documentation and django.utils.safestring

comment:8 Changed 4 years ago by SmileyChris

  • Severity set to Normal
  • Type set to New feature

comment:9 Changed 4 years ago by mitar

  • Cc mmitar@… added
  • Easy pickings unset

comment:10 Changed 3 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

Note: See TracTickets for help on using tickets.
Back to Top