Code

Opened 8 years ago

Closed 6 years ago

#1522 closed defect (fixed)

[patch] Allow template tags to accept hardcoded strings with spaces

Reported by: SmileyChris Owned by: nobody
Component: Template system Version:
Severity: normal Keywords:
Cc: ferringb@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

As requested in #1127 and #208, here is a patch that allows templates to accept hardcoded strings with spaces.

Added a test for this too.

Attachments (1)

smart_token_splitting.diff (12.1 KB) - added by SmileyChris 8 years ago.

Download all attachments as: .zip

Change History (14)

Changed 8 years ago by SmileyChris

comment:1 Changed 8 years ago by SmileyChris

Here's a test I put together to make sure this wasn't going to consume any more time. Looks like it takes about double the time as a normal .split(), but that's acceptable for the added functionality I think.

import re

tests = ['ifchanged',
         'if printable',
         "if 'always true'",
         'ifequal user.id comment.user_id',
         'ifequal user.name "Robert Jones"',
         'regroup people by gender as grouped',]

split_token_re = re.compile(r'(?:\'[^\']+\'|"[^"]+"|[^ ]+)(?= )*')
check_for_quotes = re.compile(r'[\'"]')

def smart_split(token):
    if check_for_quotes.search(token):
        return split_token_re.findall(token)
    else:
        return token.split()

if __name__ == '__main__':
    for test in tests:
        print 'testing "%s"' % test
        setup_re = '''
from __main__ import smart_split
test='%s'
''' % test.replace("'", r"\'")
        setup_split = '''
test='%s'
''' % test.replace("'", r"\'")

        re_time = timeit.Timer('x = smart_split(test)', setup_re).timeit()
        print 're     takes %.2fusec to return %s' % (re_time, str(smart_split(test)))
        split_time = timeit.Timer('x = test.split()', setup_split).timeit()
        print 'split  takes %.2fusec to return %s' % (split_time, str(test.split()))
        inaccurate = ''
        if check_for_quotes.search(test):
            inaccurate = ' (but is inaccurate)'
        print 'split is %.2fx faster%s' % (re_time / split_time, inaccurate) 
        print

comment:2 Changed 8 years ago by lukeplant

+1 - this is a great patch! I was needing this functionality just the other day (especially for creating my own custom tags, but for built-in tags it is just as useful).

comment:3 Changed 8 years ago by django@…

See also #1400.

comment:4 Changed 8 years ago by SmileyChris

Good work, Kieran! I still feel this patch has some benefits: it works efficiently to create minimal overheads on the current code and is focussed on solving a specific problem people are having. Your patch obviously takes it to the next level though ;)

Rather than making people read the diff to understand this patch: what this patch does is give the Token class a new function, split_contents. So rather than a simple string split on spaces, it will handle quoted bits. Eg:

{% ifequal test "test 2" %}
    foo...
{% endifequal %}

Here's the function added to the Token class:

def split_contents(self): 
    if check_for_quotes.search(self.contents): 
        return split_token_re.findall(self.contents) 
    else: 
        return self.contents.split() 

comment:5 Changed 8 years ago by django@…

I agree. I was not proposing #1400 as an alternative - it doesn't include fixes for builtin tags and your light-weight solution is better for that anyway. Probably #1400 streches the attention span a little too far at the moment...I might need to break it down a little.

comment:6 Changed 8 years ago by adrian

This ticket is pretty nice, but the regex doesn't support escaped strings. For example, I'd like to be able to do this:

{% ifequal 'the "person\'s hat"' 'the person\'s hat' %}

Surely a perfect string-splitting regular expression is available on the Web somewhere...

comment:7 Changed 8 years ago by adrian

Added django.utils.text.smart_split in [3101].

comment:8 Changed 8 years ago by adrian

In [3112] -- Added django.template.Token.split_contents() and used it to add support for strings with spaces in {% ifchanged %}

comment:9 Changed 8 years ago by mattimustang@…

Here's a patch to use this in decorators like include_tag etc:

Index: django/template/__init__.py
===================================================================
--- django/template/__init__.py (revision 3116)
+++ django/template/__init__.py (working copy)
@@ -749,7 +749,7 @@

 def generic_tag_compiler(params, defaults, name, node_class, parser, token):
     "Returns a template.Node subclass."
-    bits = token.contents.split()[1:]
+    bits = list(token.split_contents())[1:]
     bmax = len(params)
     def_len = defaults and len(defaults) or 0
     bmin = bmax - def_len

This makes it much easier to pass strings with spaces and/or template variables to tags like so:

{% my_tag "hello {{ world }}" %}

regards

Matthew Flanagan

comment:10 Changed 7 years ago by Simon G. <dev@…>

  • Triage Stage changed from Unreviewed to Accepted

Does this work for more than ifchanged?

comment:11 Changed 7 years ago by (removed)

  • Cc ferringb@… added

comment:12 Changed 6 years ago by emulbreh

Why is this still open?

comment:13 Changed 6 years ago by emulbreh

  • Resolution set to fixed
  • Status changed from new to closed

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.