Opened 8 years ago

Closed 5 years ago

#9483 closed Bug (wontfix)

Title template filter is broken

Reported by: Henk de Vries Owned by: Henk de Vries
Component: Template system Version: master
Severity: Normal Keywords: title template filter
Cc: Triage Stage: Design decision needed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

There's a bug in Django ( trunk ) |title template filter:

This:

'foo;bar'|title

Gets converted into this:

Foo;Bar

This isn't the behavior it should have.

The correct behavior should be:

Foo;bar

Attachments (4)

patch_1_1.0.X.diff (2.2 KB) - added by Henk de Vries 8 years ago.
Patch against 1.0.X branch
patch_1.diff (2.2 KB) - added by Henk de Vries 8 years ago.
Patch against trunk
patch_2_trunk.diff (2.4 KB) - added by Henk de Vries 8 years ago.
New patch with string.capwords(value) against trunk.
patch_2_1.0.X.diff (2.4 KB) - added by Henk de Vries 8 years ago.
New patch with string.capwords(value) against 1.0.X.

Download all attachments as: .zip

Change History (20)

comment:1 Changed 8 years ago by Henk de Vries

Addendum:

I discovered this is a 'feature' of the Python string.title function. The above behavior also occurs with strings that have a '&' or "'" or '"'.

This would be a good solution:

def titlecase(words):
     return ' '.join([x.capitalize() for x in words.split()])

comment:2 Changed 8 years ago by Henk de Vries

Owner: changed from nobody to Henk de Vries
Status: newassigned

comment:3 Changed 8 years ago by Henk de Vries

Has patch: set

comment:4 Changed 8 years ago by Henk de Vries

milestone: post-1.0

comment:5 Changed 8 years ago by Henk de Vries

patch_1.diff is a patch against trunk. I will post a patch soon that is for the 1.0.X branch.

Changed 8 years ago by Henk de Vries

Attachment: patch_1_1.0.X.diff added

Patch against 1.0.X branch

Changed 8 years ago by Henk de Vries

Attachment: patch_1.diff added

Patch against trunk

comment:6 Changed 8 years ago by Malcolm Tredinnick

Why is there a great need to change this. We do what Python at does at the moment, which isn't unreasonable. This is an edge-case and whilst it doesn't necessarily do what you personally expect, it isn't de-facto wrong (normal written text won't have a semi-colon without a space following it).

So, what is the use-case that doesn't work without this change being made?

comment:7 Changed 8 years ago by Aaron Adams

Well, normal text will often contain apostrophes, and I see Python most definitely gets this wrong, too:

>>> m = "something ain't right here"
>>> m.title()
"Something Ain'T Right Here"

The str.title() documentation says the method should "return a titlecased version of the string: words start with uppercase characters, all remaining cased characters are lowercase." If you speak strictly in regular expression terms, that's exactly what it's doing; but it's clearly not the intent.

With this post being my first foray into the Django community, I don't know what SOP is when Python appears to be broken. But it does seem sensible to accept the patch, so that title-casing works as expected for Django template authors.

(Related Python bug report: Issue 995422: title case bug — strangely rejected four years ago on the basis that "capitalization conventions vary from language to language," accompanied by an example from French... whose capitalization conventions are the same as ours. Might be worth trying to re-open the issue.)

comment:8 Changed 8 years ago by Julian Bez

Patch needs improvement: set

Maybe we are using the wrong Python function here:

>>> import string
>>> m = "something ain't right here"
>>> string.capwords(m)
"Something Ain't Right Here"

That would do what we want and it would perfectly be Python.

Changed 8 years ago by Henk de Vries

Attachment: patch_2_trunk.diff added

New patch with string.capwords(value) against trunk.

comment:9 Changed 8 years ago by Henk de Vries

Kanuck54: Thanks for your support regarding this patch. I couldn't come up with a usecase that fast :-).

Changed 8 years ago by Henk de Vries

Attachment: patch_2_1.0.X.diff added

New patch with string.capwords(value) against 1.0.X.

comment:10 Changed 8 years ago by Henk de Vries

These patches introduce dependencies against the 'string' module. I'm not a Python guru, so I don't know if this is a problem.

comment:11 Changed 8 years ago by Henk de Vries

Patch needs improvement: unset

comment:12 in reply to:  11 Changed 8 years ago by jfw

Replying to hdevries:
There's also another side effect here: string.capwords() uses split(), which means that runs of multiple spaces will get converted to one space. That likely won't be a problem for most, as HTML tends not to care about such things, but it's not backwards-compatible.

comment:13 Changed 8 years ago by Chris Beaven

Triage Stage: UnreviewedDesign decision needed

Out of interest:

>>> import timeit

>>> # Don'T Do It, Harold!  It'S A Hyper-Trap!
>>> timeit.Timer("s.title()", 's="Don\'t do it, Harold!  It\'s a hyper-trap!"').timeit()
1.9782196480278917

>>> # Don't Do It, Harold! It's A Hyper-trap!
>>> timeit.Timer("string.capwords(s)", 's="Don\'t do it, Harold!  It\'s a hyper-trap!";import string').timeit()
6.9016400129066682

>>> # Don't Do It, Harold!  It's A Hyper-trap!
>>> timeit.Timer("r.sub(cap,s)", 's="Don\'t do it, Harold!  It\'s a hyper-trap!"; import re;r=re.compile(r"(\s|^)\w");cap=lambda x: x.group().upper()').timeit()
13.67118104395945

comment:14 Changed 8 years ago by (none)

milestone: post-1.0

Milestone post-1.0 deleted

comment:15 Changed 6 years ago by Luke Plant

Severity: Normal
Type: Bug

comment:16 Changed 5 years ago by Aymeric Augustin

Easy pickings: unset
Resolution: wontfix
Status: assignedclosed
UI/UX: unset

As pointed out by Malcolm, the initial example is rather artificial and doesn't warrant a change.

Furthermore Django handles correctly the example given in comment 7:

>>> from django.template.defaultfilters import title
>>> m = "something ain't right here"
>>> title(m)
u"Something Ain't Right Here"

(There's some specific code to handle the apostrophe, and it appears in the patches uploaded above, which means it already existed when this ticket was opened.)


Absent a sufficiently convincing use case to break backwards compatibility, and given the lack of interest for this ticket over the last three years, I think we'd better keep the current implementation and I'm going to close this ticket.

It's easy enough to implement your own title tag if the builtin doesn't match exactly your requirements.

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