Opened 3 years ago

Closed 6 months ago

#18651 closed New feature (fixed)

Assignment tags should allow optional assignments

Reported by: mitar Owned by: Tim Graham <timograham@…>
Component: Template system Version: 1.4
Severity: Normal Keywords:
Cc: mmitar@…, m.r.sopacua@…, tim_heap Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Assignment tags should allow optional assignments, so that if as part is missing, they output the result. Otherwise store it into variable.

Change History (15)

comment:1 Changed 3 years ago by msopacua

  • Cc m.r.sopacua@… added
  • Needs documentation unset
  • Needs tests set
  • Patch needs improvement unset
  • Resolution set to needsinfo
  • Status changed from new to closed

Why? What is your use case? What is the problem you cannot solve?

comment:2 Changed 3 years ago by mitar

To have a simple tag made with a decorator which could serve both as a simple tag or as a assignment tag. Currently you can have only simple tag OR assignment tag, not both.

comment:3 Changed 3 years ago by tim_heap

I can think of two main use-cases for this, and two problems it solves. One of these use-cases has already been encountered in the built in tags, as documented below, so the problem it solves is real.

Firstly, when a sensible default name exists for a tag, but you may want to change it. Consider a tag {% get_comments_for object %}, which find all comments against the object passed in, and assigns the result to the comments variable in the template. If you have two objects on the page, and want to get the comments from both, the names would collide. In this case, you could use {% get_comments_for foo as foo_comments %} and {% get_comments_for bar as bar_comments %}.

The second use case is assigning or outputting the result. This is already used by the built in {% url %} tag. You can use {% url foo %} to print the URL directly, or {% url foo as foo_url %} to assign it to the foo variable in the template.

This could be implemented as either two new decorators:

@register.assignment_tag_with_default(default='comments')
def get_comments_for(object):
    pass

@register.optional_assignment_tag()
def url(name, *args, **kwargs):
    pass

or as options on the existing decorator

@register.assignment_tag(default_name='comments')
def get_comments_for(object):
    pass

@register.assignment_tag(optional_assignment=True)
def url(name, *args, **kwargs):
    pass

In the second case of extending the current decorator, default_name and optional_assignment would have to be mutually exclusive.

comment:4 Changed 3 years ago by tim_heap

  • Cc tim_heap added
  • Has patch set
  • Needs documentation set
  • Resolution needsinfo deleted
  • Status changed from closed to reopened

I've implemented this and published it on github. I went with the second option of extending the existing assignment_tag decorator. Tests have been written to make sure it works. You can find my branch here: https://github.com/maelstrom/django/commits/ticket-18651

I have not yet written documentation for the changes.

comment:5 Changed 3 years ago by tim_heap

After discussing with people in #django-dev, the following was proposed:

  • Merge these changes in to simple_tag.
  • Start assignment_tag on the deprecation path.

For example:

@register.simple_tag()
def plain_output():
    """
    Use as `{% plain_output %}`. Prints 'bar'
    """
    return 'bar'


@register.simple_tag(assignable=True)
def optional_assignment():
    """
    Use as either:

    * `{% optional_assignment %}` - prints 'bar'
    * `{% optional_assignment as foo %}` - assigns 'bar' to 'foo'
    """
    return 'bar'


@register.simple_tag(assignable=True, default_name='foo')
def default_assignment():
    """
    Use as either:

    * `{% default_assignment %}` - assigns 'bar' to 'foo'
    * `{% default_assignment as foo %}` - assigns 'bar' to 'foo'
    """
    return 'bar'

This has the down side of being unable to force assignment, as you currently can with assignment_tag. I do not see this being a problem though, as I can not think of a reasonable example where outputting a variable is so harmful that the possibility of it happening should be guarded against explicitly in the tag. If you really need this case, write a completely custom tag.

comment:6 Changed 3 years ago by tim_heap

I have made the modifications to simple_tag now, so that it supports can_assign and default_name. The code is on github: https://github.com/maelstrom/django/compare/django:master...ticket-18651-v2

comment:7 Changed 3 years ago by russellm

  • Triage Stage changed from Unreviewed to Design decision needed

I'll accept the broad idea, but the specifics of the API design still need to be clarified.

comment:8 Changed 3 years ago by anonymous

A write up of the proposals can be found here: https://gist.github.com/4534963

comment:9 Changed 2 years ago by aaugustin

  • Status changed from reopened to new

comment:10 Changed 2 years ago by aaugustin

  • Triage Stage changed from Design decision needed to Accepted

comment:11 Changed 2 years ago by jonathanslenders

  • Owner changed from nobody to jonathanslenders
  • Status changed from new to assigned

I think I can do this. It fits nicely in my work on ticket #20434 which is a huge refactoring.

comment:12 Changed 2 years ago by jonathanslenders

  • Owner jonathanslenders deleted
  • Status changed from assigned to new

Deassigned, because I'm lacking some time right now. So, anyone feel free to work on this ticket. Otherwise, I'll take it again in a few weeks.

comment:13 Changed 6 months ago by prestontimmons

  • Needs documentation unset
  • Needs tests unset

comment:14 Changed 6 months ago by prestontimmons

I added a pull request here:

https://github.com/django/django/pull/4034

comment:15 Changed 6 months ago by Tim Graham <timograham@…>

  • Owner set to Tim Graham <timograham@…>
  • Resolution set to fixed
  • Status changed from new to closed

In cd4282816db9164791cd0ac97a3dc329ad92c522:

Fixed #18651 -- Enabled optional assignments for simple_tag().

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