Opened 10 years ago

Closed 10 years ago

#24372 closed Cleanup/optimization (fixed)

Remove django.template.base.TokenParser?

Reported by: Preston Timmons Owned by: nobody
Component: Template system Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

TokenParser is only used by the {% trans %} tag in django.templatetags.i18n.do_translate. I don't see the advantage over traditional parsing. Should we update that? Or at least move TokenParser out of django.template and into django.templatetags.i18n?.

Change History (6)

comment:1 by Preston Timmons, 10 years ago

Here's a branch that replaces TokenParser with traditional parsing:

https://github.com/prestontimmons/django/compare/remove-token-parser

If this is worthwhile, I'll add some additional tests to the trans tag for the syntax error cases.

comment:2 by Claude Paroz, 10 years ago

Has patch: set
Triage Stage: UnreviewedAccepted

If the tests are happy, so do I :-)

comment:3 by Tim Graham, 10 years ago

Triage Stage: AcceptedReady for checkin

Patch looks good pending a couple cosmetic tweaks.

comment:4 by Aymeric Augustin, 10 years ago

Before merging this, could you check when TokenParser was introduced and possibly why?

If it predates the "traditional parsing" — or at least the reasonably robust version we now have — it can be removed safely.

If it was added later, we should make sure this doesn't introduce regressions.

comment:5 by Preston Timmons, 10 years ago

Yes.

TokenParser was added as part of the original i18n branch:

https://github.com/django/django/commit/5cf8f684237ab5addaf3549b2347c3adf107c0a7

At that time, it was used by both the trans and blocktrans tags.

The blocktrans tag was changed to use traditional parsing when the with syntax was updated:

https://github.com/django/django/commit/3ae9117c467f9fabed8736949dee209d40293b8d

This left the trans tag as the only one using TokenParser.

The unit tests for the trans tag in template_tests and i18n are fairly comprehensive. I added additional tests in this patch for the syntax error cases.

comment:6 by Tim Graham <timograham@…>, 10 years ago

Resolution: fixed
Status: newclosed

In 358850781f8fd3d79aba5b1a9a0b8d28f544bf8a:

Fixed #24372 - Replaced TokenParser usage with traditional parsing.

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