Opened 13 years ago

Closed 11 years ago

#15093 closed Bug (worksforme)

smart_split behaviour is surprising

Reported by: Stephen Kelly Owned by: nobody
Component: Template system Version: 1.2
Severity: Normal Keywords:
Cc: Triage Stage: Design decision needed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I attempted to fix #15092 with a smart_split and noticed that it does not split on unescaped " not surrounded by whitespace.

If this is intentional it should be documented. I don't think any other unit test for templates tries to use a 'corrupt' string like that.
It seems that FilterExpression doesn't accept strings like that anyway, so it should either be documented that they don't work in the template system, or smart_split should be changed.

In [1]: from django.utils.text import smart_split

In [2]: smart_split(' now "j "n" Y"'
   ...: )
Out[2]: <generator object smart_split at 0xa0f5d74>

In [3]: for a in smart_split(' now "j "n" Y"'): print a
   ...: 
now
"j "n" Y"

In [4]: for a in smart_split(' now "j " n " Y"'): print a
   ...: 
now
"j "
n
" Y"

Attachments (1)

15093.diff (855 bytes ) - added by ersame 13 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 by Russell Keith-Magee, 13 years ago

Triage Stage: UnreviewedAccepted

This looks like it's an oversight in the parsing algorithm; I can't imagine it would be intentional.

by ersame, 13 years ago

Attachment: 15093.diff added

comment:2 by ersame, 13 years ago

Has patch: set

Hi there,

I have solved the problem. I have almost read a book about regular expression to solve the problem but it was quite interesting :)

I have made some test using steveire report:

In [3]: list(smart_split(r'This is "a person\'s" test.')) == [u'This', u'is', u'"a person\\\'s"', u'test.']
Out[3]: True

In [4]: list(smart_split(r"Another 'person\'s' test.")) == [u'Another', u"'person\\'s'", u'test.']
Out[4]: True

In [5]: list(smart_split(r'A "\"funky\" style" test.')) == [u'A', u'"\\"funky\\" style"', u'test.']
Out[5]: True

In [6]: list(smart_split(r' now "j "n" Y"')) == [u'now', u'"j "', u'n', u'" Y"']
Out[6]: True

In [7]: list(smart_split(r' now "j "n " Y"')) == [u'now', u'"j "', u'n', u'" Y"']
Out[7]: True

In [8]: list(smart_split(r' now "j " n" Y"')) == [u'now', u'"j "', u'n', u'" Y"']
Out[8]: True

In [9]: list(smart_split(r' now "j " n " Y"')) == [u'now', u'"j "', u'n', u'" Y"']
Out[9]: True

Thank you for the report steveire.

Regards,

Juan Antonio Infantes.

Version 0, edited 13 years ago by ersame (next)

comment:3 by ersame, 13 years ago

Has patch: unset

comment:4 by ersame, 13 years ago

Triage Stage: AcceptedDesign decision needed

I think it is not a bug, in the comment of smart_split definition it appears:
"Generator that splits a string by spaces".

If we look to the test case in django.utils.text we can see:

self.assertEquals(list(smart_split(u'url search_page words="something else"')),
            [u'url', u'search_page', u'words="something else"'])


This test won't pass if you patch smart_split in the way you report.

We would need to decide how we want smart_split works. In my opinion
smart_split should split by spaces just what we have.

Anyway if someone wants the other behaviour could use my patch to solve
the problem.

What do you think?

Regards,

Juan Antonio Infantes.

Last edited 13 years ago by ersame (previous) (diff)

comment:5 by James Addison, 13 years ago

Severity: Normal
Type: Bug

comment:6 by Carl Meyer, 13 years ago

In [16108]:

Refs #15093 -- Fixed another get_models call missed in r16053. Thanks Luke for catching it.

comment:7 by Carl Meyer, 13 years ago

Easy pickings: unset

Oops, fat-fingered that commit message - should have referred to #15903.

comment:8 by Aymeric Augustin, 12 years ago

UI/UX: unset

Change UI/UX from NULL to False.

comment:9 by Aymeric Augustin, 11 years ago

Resolution: worksforme
Status: newclosed

The behavior is intentional and it's described in the docstring: "Generator that splits a string by spaces, leaving quoted phrases together."

This is a private API, and such APIs are only documented in their docstrings.

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