Opened 10 years ago

Closed 2 years ago

#1199 closed New feature (wontfix)

Supporting more than one argument in a custom filter

Reported by: Aggelos Orfanakos Owned by: insin
Component: Template system Version:
Severity: Normal Keywords:
Cc: chrischambers 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 insin 8 years ago.
A first stab at just getting this working at all
ticket-1199.patch (3.9 KB) - added by Jan Rademaker <j.rademaker@…> 7 years ago.
Updated insin's patch to revision 7397 + a bugfix

Download all attachments as: .zip

Change History (22)

comment:1 Changed 9 years ago by SmileyChris

  • Triage Stage changed from Unreviewed to Design 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 9 years ago by anonymous

  • Triage Stage changed from Design decision needed to Accepted

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 9 years ago by mtredinnick

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

comment:4 Changed 8 years ago by lorien

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

comment:5 Changed 8 years ago by lorien

  • Owner lorien deleted
  • Status changed from assigned to new

Changed 8 years ago by insin

A first stab at just getting this working at all

comment:6 Changed 8 years ago by insin

  • Has patch set
  • Needs documentation set
  • Needs tests set
  • Owner set to insin
  • Status changed from new to assigned

comment:7 follow-up: Changed 8 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 8 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 8 years ago by SmileyChris

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 8 years ago by SmileyChris

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 7 years ago by Jan Rademaker <j.rademaker@…>

Updated insin's patch to revision 7397 + a bugfix

comment:11 Changed 7 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 7 years ago by emulbreh

  • milestone set to 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 7 years ago by bbeck

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 7 years ago by emulbreh

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 7 years ago by anonymous

  • milestone post-1.0 deleted

Milestone post-1.0 deleted

comment:16 Changed 5 years ago by chrischambers

  • Cc chrischambers added

comment:17 Changed 4 years ago by lrekucki

  • Severity changed from normal to Normal
  • Type changed from enhancement to New feature

comment:18 Changed 4 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:19 Changed 4 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

comment:20 Changed 2 years ago by aaugustin

  • Resolution set to wontfix
  • Status changed from assigned to closed

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