Opened 8 years ago

Closed 7 years ago

Last modified 5 years ago

#9315 closed (fixed)

Keyword arguments with spaces and the url tag

Reported by: Alexis Bellido Owned by: Natalia Bidart
Component: Template system Version: 1.0
Severity: Keywords: url, tplrf-fixed, pycamp2009
Cc: Robert, matiasb Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Hi, I asked about this on the django-users group and was told it was probably a bug so I'm reporting it:

I was testing named url patterns and I have something like this in my URLConf:

url(r'^search/(?P<words>.*)$', 'books.views.search', name='search_page'),

The view is defined like this:

def search(request, words):

Now I'd like to print a link to the search page with certain words from a template and used the url tag like this:

<p>{% url search_page words="someword" %}</p>

When viewing on the browser I get something like '/search/someword', which is good. My question is how do I pass more than one word in the 'words'
parameter?

If I do this:

<p>{% url search_page words="someword otherword" %}</p>

I get this error:

TemplateSyntaxError at (my page name here)

Could not parse the remainder: '"someword' from '"someword

Can the url tag handle parameters with spaces?

Attachments (2)

patch-9315.diff (4.5 KB) - added by Natalia Bidart 8 years ago.
patch-9315-regex-greedy.diff (4.5 KB) - added by Natalia Bidart 8 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 8 years ago by Johannes Dollinger

Keywords: tplrf-fixed added
Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

This would be fixed by #7806.

comment:2 Changed 8 years ago by Robert

Cc: Robert added

comment:3 Changed 8 years ago by Jacob

milestone: 1.1
Triage Stage: UnreviewedAccepted

comment:4 Changed 8 years ago by Natalia Bidart

Keywords: pycamp2009 added
Owner: changed from nobody to Natalia Bidart
Status: newassigned

Changed 8 years ago by Natalia Bidart

Attachment: patch-9315.diff added

comment:5 Changed 8 years ago by Natalia Bidart

Has patch: set

Added patch to solve this issue. Fixed involved debugging and improvments of the smart_split from [browser:/django/util/text.py], plus a simplification of split_contents at [browser:django/template/__init__.py].

All tests passes except for test_templates (regressiontests.templates.tests.Templates) which was already failing.

Made by nessita (Natalia Bidart) and matiasb (Matías Bordese, not registered yet).
Contributors understanding regexs: Ramiro Morales, Facundo Batista, Pablo Ziliani, John Lenton.

comment:6 Changed 8 years ago by Natalia Bidart

Last night, while being unable to sleep, I figured out that the non-greedy part of the proposed regular expression (in teh pacth attached) can be changed to a greedy expression, improving the performance of it.

So, where it reads:

smart_split_re = re.compile(r"""(\S*?"(?:[^"\\]*(?:\\.[^"\\]*)*)"\S*| # matches '"value with spaces"' and 'keyword="value with spaces"' 
                                 \S*?'(?:[^'\\]*(?:\\.[^'\\]*)*)'\S*| # same as above but with quotes swapped 
                                 \S+)                                 # matches not whitespaces""", 
                            re.VERBOSE) 

the \S+?" can be changed to [^\s"]" (same for the ' character). I'll submit a new patch in a while, prior some checkings.

comment:7 Changed 8 years ago by Natalia Bidart

Cc: matiasb added

comment:8 Changed 8 years ago by Natalia Bidart

Attaching new patch with greedy regular expression (to avoid backtracking).

Changed 8 years ago by Natalia Bidart

comment:9 Changed 7 years ago by Malcolm Tredinnick

I'm not going to apply the changes to the split_contents() portion, since they don't appear to fix an existing problem I'm not convinced they don't introduce a bug. We should fix on problem per ticket.

If somebody wants to fix up that portion (and if it can be reduced to one line, that would be great), the patch should include test of the i18n pieces in that function. It's clearly looking at _("...") strings, but I don't see anything in the replacement code that handles that. It might well be being handled elsewhere automatically (so we're doing it twice now) and that will be demonstrated by the tests in the new ticket and patch.

comment:10 Changed 7 years ago by Malcolm Tredinnick

Resolution: fixed
Status: assignedclosed

(In [10462]) Fixed #9315 -- Handle spaces in URL tag arguments.

Thanks Natalia Bidart and Matías Bordese for most of this patch.

comment:11 Changed 7 years ago by Malcolm Tredinnick

(In [10463]) [1.0.X] Fixed #9315 -- Handle spaces in URL tag arguments.

Thanks Natalia Bidart and Matías Bordese for most of this patch.

Backport of r10462 from trunk.

comment:12 Changed 7 years ago by Natalia Bidart

Malcom,

you're welcome! We had the most fun proposing a solution for this one. We would love to keep contributing to the django project.

comment:13 Changed 5 years ago by Jacob

milestone: 1.1

Milestone 1.1 deleted

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