Opened 11 years ago

Closed 4 years ago

#1199 closed New feature (wontfix)

Supporting more than one argument in a custom filter

Reported by: Aggelos Orfanakos Owned by: Jonathan Buchanan
Component: Template system Version:
Severity: Normal Keywords:
Cc: Chris Chambers Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

It would be nice to be able to pass more than one argument to a (custom) filter. E.g.:

{{ value|custom_filter:arg1,arg2,arg3 }}

The only current workaround for this, that I have found, is passing a sequence (e.g. tuple, list) containing many arguments and then, inside the custom filter, do something like:

def custom_filter(value, arg):
    arg1 = arg[0]
    arg2 = arg[1]

    # ...

Attachments (2)

sprint-patch1.diff (3.9 KB) - added by Jonathan Buchanan 9 years ago.
A first stab at just getting this working at all
ticket-1199.patch (3.9 KB) - added by Jan Rademaker <j.rademaker@…> 9 years ago.
Updated insin's patch to revision 7397 + a bugfix

Download all attachments as: .zip

Change History (22)

comment:1 Changed 10 years ago by Chris Beaven

Triage Stage: UnreviewedDesign decision needed

I don't know why this hasn't been already implemented. Perhaps if someone wrote a patch whether it'd speed things along. ;)

comment:2 Changed 10 years ago by anonymous

Triage Stage: Design decision neededAccepted

Currently, everywhere we need multiple arguments, we pass them in as comma-separated strings. However, this isn't perfect if you want a comma in your argument list.

So the idea seems reasonable. If somebody writes a patch (with tests), we'll look over it.

comment:3 Changed 10 years ago by Malcolm Tredinnick

Last comment was from me. Sorry, accidently not logged in.

comment:4 Changed 9 years ago by Grigoriy Petukhov

Owner: changed from nobody to Grigoriy Petukhov
Status: newassigned

comment:5 Changed 9 years ago by Grigoriy Petukhov

Owner: Grigoriy Petukhov deleted
Status: assignednew

Changed 9 years ago by Jonathan Buchanan

Attachment: sprint-patch1.diff added

A first stab at just getting this working at all

comment:6 Changed 9 years ago by Jonathan Buchanan

Has patch: set
Needs documentation: set
Needs tests: set
Owner: set to Jonathan Buchanan
Status: newassigned

comment:7 Changed 9 years ago by FogleBird

Currently, everywhere we need multiple arguments, we pass them in as comma-separated strings. However, this isn't perfect if you want a comma in your argument list. 

Also not perfect if your arguments aren't hard-coded, constant strings.

I ran into this limitation today and found this ticket. Are tests and documentation all that are holding this back? If so, perhaps now is a good time for me to make a contribution to Django. IMO, it's little things like these that make Django a pain sometimes (as much as I love it), so I'd like to help remove this limitation if possible.

comment:8 Changed 9 years ago by limeyd

I was wondering rather than a argument list wouldn't an optional dictionary then you'd have the best of both worlds?

comment:9 in reply to:  7 Changed 9 years ago by Chris Beaven

Replying to FogleBird:

Currently, everywhere we need multiple arguments, we pass them in as comma-separated strings. However, this isn't perfect if you want a comma in your argument list. 

Also not perfect if your arguments aren't hard-coded, constant strings.

I ran into this limitation today and found this ticket. Are tests and documentation all that are holding this back? If so, perhaps now is a good time for me to make a contribution to Django. IMO, it's little things like these that make Django a pain sometimes (as much as I love it), so I'd like to help remove this limitation if possible.

comment:10 Changed 9 years ago by Chris Beaven

Oops, lets try that again:

Replying to FogleBird:

If so, perhaps now is a good time for me to make a contribution to Django.

Foglebird - it's been accepted, so if you can help get the ticket closer to ready-for-checkin then start coding! Since the patch was described as "a first stab", I'd imagine that when you start the tests you may find some areas which need improvement on the patch.
Drop into IRC and have a chat if you need any pointers for writing the tests / docs.

Changed 9 years ago by Jan Rademaker <j.rademaker@…>

Attachment: ticket-1199.patch added

Updated insin's patch to revision 7397 + a bugfix

comment:11 Changed 9 years ago by Jan Rademaker <j.rademaker@…>

The previous patch only worked for string literals, ticket-1199.patch should fix that (eg Variable(var_arg)).

comment:12 Changed 8 years ago by Johannes Dollinger

milestone: post-1.0

If whitespace handling will ever be relaxed (as in #7806), the proposed syntax will not work for {% url %} and similar tags that use ",".
{% url view x|filter:a,b %} could mean (x|filter:(a,b)) or ((x|filter:a), b).

Syntax proposals:

  1. New syntax: foo|bar(x,y,z), keeping foo|bar:x for bc.
  2. Tupel primitives: foo|bar:(x,y,z), would allow {% for x in (a,b,c) %}
  3. Lisp style: foo|bar:(x|cons:y), this would basically allow bracketed expressions as arguments to filters. Arguably the ugliest.

The second and third option could either pass the tuple to the filter function, which would mean render-time argc checks, or pass *tuple as proposed in the current patch.

I favour the first option although tuple primitives seem useful. But that could easily be separate ticket.

comment:13 Changed 8 years ago by Brian Beck

I don't think the {% url %} issue above warrants avoiding a,b,c syntax, but I always imagined it might look like this:

    foo|bar:x:y:z

comment:14 Changed 8 years ago by Johannes Dollinger

I would favour a syntax that allows filter expressions as arguments to filters.
foo|bar:a:b|baz:c:d and foo|bar:a,b|baz:c,d cannot distinguish foo|bar(a, b|baz:c, d) and foo|bar(a, b|baz(c, d)).

comment:15 Changed 8 years ago by (none)

milestone: post-1.0

Milestone post-1.0 deleted

comment:16 Changed 6 years ago by Chris Chambers

Cc: Chris Chambers added

comment:17 Changed 6 years ago by Łukasz Rekucki

Severity: normalNormal
Type: enhancementNew feature

comment:18 Changed 5 years ago by Aymeric Augustin

UI/UX: unset

Change UI/UX from NULL to False.

comment:19 Changed 5 years ago by Aymeric Augustin

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:20 Changed 4 years ago by Aymeric Augustin

Resolution: wontfix
Status: assignedclosed

Every instance of "comma separated things" proved to be a problem and was removed, and arguments are now allowed to contain any character.

If we update the proposals from comment 12 to take this into account, we end up with this syntax: myobj|mymethod(variable1, "literal2") which boils down to an instance method call! This doesn't fit in the scope of the Django Template Language.

This proposal no longer fits the modern Django Template Language and I'm going to close the ticket.

If you need multiple arguments, use a custom tag (assignement tags are easy) or preprocess data in the view function.

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