Opened 11 years ago

Closed 5 years ago

#6544 closed Bug (wontfix)

Regression with new templates extends checks

Reported by: Bastian Kleineidam <calvin@…> Owned by:
Component: Template system Version: master
Severity: Normal Keywords:
Cc: FunkyBob Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no


SVN r7084 breaks existing templates when a comment node is the first element in a template. So templates of the form

{% comment %}bla{% endcomment %}
{% extends ... %}

will throw a syntax error with the new behaviour. I suggest allowing comments before the extends tag.

Attachments (1)

django-allow-comments-before-extends.diff (1.8 KB) - added by Bastian Kleineidam <calvin@…> 11 years ago.

Download all attachments as: .zip

Change History (14)

comment:1 Changed 11 years ago by James Bennett

Resolution: invalid
Status: newclosed

The documentation states, and has stated since before this change, that "If you use {% extends %} in a template, it must be the first template tag in that template." If use of other tags before {% extends %} worked before, it shouldn't have, and if it doesn't now that's the way it ought to be.

comment:2 Changed 11 years ago by Bastian Kleineidam <calvin@…>

Has patch: set

Here is a patch that allows any number of whitespace or comments before an {% extends %} node. This is more convenient than having to edit all templates (which can be a sufficiently large number).

I'll leave the ticket as closed, but other users might be interested in the patch, so here goes. Note that I did not bother to included tests or documentation since this idea is already rejected.

Changed 11 years ago by Bastian Kleineidam <calvin@…>

comment:3 Changed 11 years ago by Malcolm Tredinnick

Resolution: invalid
Status: closedreopened
Triage Stage: UnreviewedAccepted

It's reasonable to allow comment template tags, since we allow the {#...#} form. However, I don't like the patch. It looks like it should be just a simple change to extend_nodelist in the current code (which admittedly has been modified since the patch was created).

I'll get around to fixing it next time I'm working on trunk.

comment:4 Changed 11 years ago by Malcolm Tredinnick

Owner: changed from nobody to Malcolm Tredinnick
Patch needs improvement: set
Status: reopenednew

comment:5 Changed 11 years ago by Bastian Kleineidam <calvin@…>

Needs documentation: set
Needs tests: set

Ah, thanks for accepting this. And yes, the patch needs definitely improvement (I am not very familiar with the template API), and unit tests, and documentation updates.

comment:6 Changed 10 years ago by Malcolm Tredinnick

Owner: Malcolm Tredinnick deleted

Reassigning back to the pool in case somebody wants to work up a better patch in one of the sprints.

comment:7 Changed 8 years ago by Julien Phalip

Type: Bug

comment:8 Changed 7 years ago by Julien Phalip

Severity: Normal

comment:9 Changed 7 years ago by Aymeric Augustin

UI/UX: unset

Change UI/UX from NULL to False.

comment:10 Changed 7 years ago by Aymeric Augustin

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:11 Changed 5 years ago by FunkyBob

Given that nothing outside of a {% block %} is rendered in a template that {% extends %} ... why even wrap your comments in a comment tag?

The template engine allows text nodes to precede the {% extends %} node, so why not just write your comments there, unadorned?

comment:12 Changed 5 years ago by FunkyBob

Cc: FunkyBob added

comment:13 Changed 5 years ago by FunkyBob

Resolution: wontfix
Status: newclosed

Clearly nobody has strong feelings about this any more.

Also, there are two simple solutions:

  • move your comment below the extends tag
  • don't wrap your comment in a tag.
Note: See TracTickets for help on using tickets.
Back to Top