Opened 18 years ago

Closed 16 years ago

#1522 closed defect (fixed)

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

Reported by: Chris Beaven 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: no UI/UX: no

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 Chris Beaven 18 years ago.

Download all attachments as: .zip

Change History (14)

by Chris Beaven, 18 years ago

Attachment: smart_token_splitting.diff added

comment:1 by Chris Beaven, 18 years ago

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 by Luke Plant, 18 years ago

+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 by django@…, 18 years ago

See also #1400.

comment:4 by Chris Beaven, 18 years ago

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 by django@…, 18 years ago

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 by Adrian Holovaty, 18 years ago

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 by Adrian Holovaty, 18 years ago

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

comment:8 by Adrian Holovaty, 18 years ago

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

comment:9 by mattimustang@…, 18 years ago

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

  • django/template/__init__.py

     
    749749
    750750def generic_tag_compiler(params, defaults, name, node_class, parser, token):
    751751    "Returns a template.Node subclass."
    752     bits = token.contents.split()[1:]
     752    bits = list(token.split_contents())[1:]
    753753    bmax = len(params)
    754754    def_len = defaults and len(defaults) or 0
    755755    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 by Simon G. <dev@…>, 17 years ago

Triage Stage: UnreviewedAccepted

Does this work for more than ifchanged?

comment:11 by (removed), 17 years ago

Cc: ferringb@… added

comment:12 by Johannes Dollinger, 16 years ago

Why is this still open?

comment:13 by Johannes Dollinger, 16 years ago

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.
Back to Top