Opened 7 years ago

Closed 4 years ago

#9483 closed Bug (wontfix)

Title template filter is broken

Reported by: hdevries Owned by: hdevries
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 hdevries 7 years ago.
Patch against 1.0.X branch
patch_1.diff (2.2 KB) - added by hdevries 7 years ago.
Patch against trunk
patch_2_trunk.diff (2.4 KB) - added by hdevries 7 years ago.
New patch with string.capwords(value) against trunk.
patch_2_1.0.X.diff (2.4 KB) - added by hdevries 7 years ago.
New patch with string.capwords(value) against 1.0.X.

Download all attachments as: .zip

Change History (20)

comment:1 Changed 7 years ago by hdevries

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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 7 years ago by hdevries

  • Owner changed from nobody to hdevries
  • Status changed from new to assigned

comment:3 Changed 7 years ago by hdevries

  • Has patch set

comment:4 Changed 7 years ago by hdevries

  • milestone set to post-1.0

comment:5 Changed 7 years ago by hdevries

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

Changed 7 years ago by hdevries

Patch against 1.0.X branch

Changed 7 years ago by hdevries

Patch against trunk

comment:6 Changed 7 years ago by mtredinnick

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 7 years ago by Kanuck54

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 7 years ago by julianb

  • 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 7 years ago by hdevries

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

comment:9 Changed 7 years ago by hdevries

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

Changed 7 years ago by hdevries

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

comment:10 Changed 7 years ago by hdevries

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 follow-up: Changed 7 years ago by hdevries

  • Patch needs improvement unset

comment:12 in reply to: ↑ 11 Changed 7 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 6 years ago by SmileyChris

  • Triage Stage changed from Unreviewed to Design 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 6 years ago by anonymous

  • milestone post-1.0 deleted

Milestone post-1.0 deleted

comment:15 Changed 4 years ago by lukeplant

  • Severity set to Normal
  • Type set to Bug

comment:16 Changed 4 years ago by aaugustin

  • Easy pickings unset
  • Resolution set to wontfix
  • Status changed from assigned to closed
  • 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