Opened 16 years ago
Closed 11 years ago
#6271 closed New feature (fixed)
filter arguments with spaces break several template tags
Reported by: | Owned by: | Aymeric Augustin | |
---|---|---|---|
Component: | Template system | Version: | dev |
Severity: | Normal | Keywords: | tplrf-fixed |
Cc: | serg@…, bmispelon@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
After [6956], something like this now breaks:
{% regroup object_list by created|date:"F Y" as grouped %}
I believe it now splits the quoted bits and the arg count is more than 6 in this case, so the templatetag errors out.
Attachments (4)
Change History (19)
Changed 16 years ago by
Attachment: | regroup_regression_test.diff added |
---|
comment:1 Changed 16 years ago by
Description: | modified (diff) |
---|---|
Has patch: | set |
Patch needs improvement: | set |
Summary: | Changeset 6956 broke regroup tag if filter contains a space → filter arguments with spaces break several template tags |
Triage Stage: | Unreviewed → Accepted |
The bigger problem here is that many of the template tags do not accept filtered arguments passed an argument with a space because they are simply doing token.contents.split()
.
Here is a patch that seems to fix things for regroup
, ifchanged
, and with
. firstof
, if
, and ifequal
/ifnotequal
need some more (involved) work. I changed smart_split_re
to allow text before and after the quote characters. Not sure if we want smart_split()
to basically only be for parsing template tags, but it seems to be the only place it's used currently.
Changed 16 years ago by
Changed 16 years ago by
Attachment: | smart_split.diff added |
---|
A rewrite of smart_split generator to properly handle quoted phrases like we need
comment:2 Changed 16 years ago by
I had been thinking about this recently too.
Just allowing text before and after quote characters still isn't good enough. If multiple filters are used which have arguments with spaces then it'll still fall over. Here's my rewrite of the smart_split
method to handle it like we need.
comment:4 Changed 16 years ago by
Owner: | changed from nobody to Gary Wilson |
---|---|
Status: | new → assigned |
comment:5 Changed 16 years ago by
Owner: | Gary Wilson deleted |
---|---|
Status: | assigned → new |
So there seems to be a problem here in that smart_split
is replacing escaped quotes and escaped backslashes with the quote and backslash respectively. The bits from the split are sometimes passed to django.template.compile_filter
, which creates a FilterExpression
object, which again looks for escaped quotes in filter arguments, and causes a parse error.
I'm thinking that we make smart_split
just split and not replace any escaped quotes or backslashes. Then we make sure compile_filter
is being used everywhere it needs to be used. Let compile filter do the processing of escaped quotes, translation strings, multiple filters, arguments with spaces, etc.
Anyway, here is a patch for smart_split
that takes SmileyChris
's regex changes, adds the smart_split
tests from my first patch (plus a few more), and removes the unescaping of quotes and backslashes in smart_split
...
Changed 16 years ago by
Attachment: | smart_split.2.diff added |
---|
comment:6 Changed 16 years ago by
Cc: | serg@… added |
---|
comment:7 Changed 15 years ago by
Keywords: | tplrf-fixed added |
---|
This would be fixed by the refactoring proposed in #7806.
comment:8 Changed 15 years ago by
#8902 has been marked as a duplicate of this ticket. It also contains small patch.
comment:9 Changed 15 years ago by
For this ticket to get to the point of being resolvable, it has to actually fix the template tags that have problems with spaces in their arguments. Just changing smart_split()
is only part of the issue (since it's actually a change that doesn't fix the original problem, as the original problem isn't in the code any longer). We also need to fix the problem identified in #8902: those tags not using smart_split()
, as well as add documentation.
So the currently latest patch is only the first 25.647% (approximately) of the fix. Yes, this may be looked at as an increase in scope of the ticket, but we need one ticket that is the holder for "spaces in arguments" and this one was just nominated to volunteer.
comment:10 Changed 15 years ago by
#10001 has been marked as a duplicate of this ticket. It also contains small patch for smart_split().
comment:11 Changed 13 years ago by
Severity: | → Normal |
---|---|
Type: | → New feature |
Considering how old this "bug" is, fixing it properly today would be equivalent to a new feature, I think.
comment:12 Changed 12 years ago by
Easy pickings: | unset |
---|---|
Owner: | set to Aymeric Augustin |
UI/UX: | unset |
comment:13 Changed 11 years ago by
I "accidentally" fixed this while working on a different bug.
I have a pull request for this and #18260 at https://github.com/django/django/pull/751
comment:14 Changed 11 years ago by
Cc: | bmispelon@… added |
---|
comment:15 Changed 11 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
A test case for regroup with filters containing a space