Code

Opened 3 years ago

Closed 16 months ago

#15093 closed Bug (worksforme)

smart_split behaviour is surprising

Reported by: steveire 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 3 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 3 years ago by russellm

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

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

Changed 3 years ago by ersame

comment:2 Changed 3 years ago by ersame

  • Has patch set

I am working on it.

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

comment:3 Changed 3 years ago by ersame

  • Has patch unset

comment:4 Changed 3 years ago by ersame

  • Triage Stage changed from Accepted to Design 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?

Regards,

Juan Antonio Infantes.

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

comment:5 Changed 3 years ago by jaddison

  • Severity set to Normal
  • Type set to Bug

comment:6 Changed 3 years ago by carljm

In [16108]:

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

comment:7 Changed 3 years ago by carljm

  • Easy pickings unset

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

comment:8 Changed 2 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:9 Changed 16 months ago by aaugustin

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

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.

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.