Code

Opened 6 years ago

Closed 7 months 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

Description

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@…> 6 years ago.

Download all attachments as: .zip

Change History (14)

comment:1 Changed 6 years ago by ubernostrum

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to invalid
  • Status changed from new to closed

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 6 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 6 years ago by Bastian Kleineidam <calvin@…>

comment:3 Changed 6 years ago by mtredinnick

  • Resolution invalid deleted
  • Status changed from closed to reopened
  • Triage Stage changed from Unreviewed to Accepted

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 6 years ago by mtredinnick

  • Owner changed from nobody to mtredinnick
  • Patch needs improvement set
  • Status changed from reopened to new

comment:5 Changed 6 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 6 years ago by mtredinnick

  • Owner mtredinnick deleted

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

comment:7 Changed 3 years ago by julien

  • Type set to Bug

comment:8 Changed 3 years ago by julien

  • Severity set to Normal

comment:9 Changed 2 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:10 Changed 2 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

comment:11 Changed 7 months 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 7 months ago by FunkyBob

  • Cc FunkyBob added

comment:13 Changed 7 months ago by FunkyBob

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

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.

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.