Opened 6 years ago

Closed 4 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


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
"j "n" Y"

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

Attachments (1)

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

Download all attachments as: .zip

Change History (10)

comment:1 Changed 6 years ago by Russell Keith-Magee

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedAccepted

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

Changed 6 years ago by ersame

Attachment: 15093.diff added

comment:2 Changed 6 years ago by ersame

Has patch: set

I am working on it.

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

comment:3 Changed 6 years ago by ersame

Has patch: unset

comment:4 Changed 6 years ago by ersame

Triage Stage: AcceptedDesign decision needed

I think it is not a bug, in the comment of smart_split definition it appear:
"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 thing?


Juan Antonio Infantes.

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

comment:5 Changed 6 years ago by James Addison

Severity: Normal
Type: Bug

comment:6 Changed 5 years ago by Carl Meyer

In [16108]:

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

comment:7 Changed 5 years ago by Carl Meyer

Easy pickings: unset

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

comment:8 Changed 5 years ago by Aymeric Augustin

UI/UX: unset

Change UI/UX from NULL to False.

comment:9 Changed 4 years ago by Aymeric Augustin

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