Opened 4 years ago

Closed 22 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 4 years ago by Melvyn Sopacua

Cc: m.r.sopacua@… added
Needs tests: set
Resolution: needsinfo
Status: newclosed

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

comment:2 Changed 4 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 4 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 4 years ago by Tim Heap

Cc: Tim Heap added
Has patch: set
Needs documentation: set
Resolution: needsinfo
Status: closedreopened

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 4 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 4 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 4 years ago by Russell Keith-Magee

Triage Stage: UnreviewedDesign decision needed

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

comment:8 Changed 4 years ago by anonymous

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

comment:9 Changed 4 years ago by Aymeric Augustin

Status: reopenednew

comment:10 Changed 4 years ago by Aymeric Augustin

Triage Stage: Design decision neededAccepted

comment:11 Changed 3 years ago by jonathanslenders

Owner: changed from nobody to jonathanslenders
Status: newassigned

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

comment:12 Changed 3 years ago by jonathanslenders

Owner: jonathanslenders deleted
Status: assignednew

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 22 months ago by Preston Timmons

Needs documentation: unset
Needs tests: unset

comment:14 Changed 22 months ago by Preston Timmons

I added a pull request here:

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

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

Owner: set to Tim Graham <timograham@…>
Resolution: fixed
Status: newclosed

In cd4282816db9164791cd0ac97a3dc329ad92c522:

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

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