Opened 14 years ago
Closed 9 months ago
#14831 closed New feature (fixed)
Django Template Style Guide
Reported by: | Simon Meers | Owned by: | Ryan Cheley |
---|---|---|---|
Component: | Documentation | Version: | 1.2 |
Severity: | Normal | Keywords: | template, style, format |
Cc: | Tobias Kunze, Ryan Cheley | 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 (last modified by )
It would be nice to have a standard template style that should be followed. For example, I like to indent my Django templates by two additional spaces every time I enter a new HTML or template block. The Django core templates vary greatly in regards to style and indentation. Much of the time template readability is sacrificed in favour of attempting to preserve the "appearance" of the generated HTML code.
Regardless of what is decided, it would be great to have at least some guidelines for templates added to https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/coding-style/#template-style
Change History (23)
comment:1 by , 14 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 14 years ago
I would like to see such a standards guide as well, especially regarding indentation. In addition to whether all code inside a block should be indented, there seems to be a variety of approaches to indentation around other template tags such as conditionals and loops. My preference would be to indent inside of all template tags that span multiple lines.
comment:3 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → New feature |
comment:4 by , 13 years ago
Easy pickings: | unset |
---|---|
UI/UX: | unset |
I think much of Gabriel's suggestions here make good sense. Though:
- I think
{% load A B C %}
is OK; loading in templates is simpler than importing in python since there are no relative imports, from x import y, etc. - I think 4-spaces is too much; in python, nesting can (and should) be avoided; in HTML, deep nesting is unavoidable. I'd therefore advocate 2-space indentation or tabs (the width of which can be configured by your editor)
I personally also indent template tags and HTML tags alike; e.g.
<ul> {% for x in y %} <li>{{ x }}</li> {% endfor %} </ul>
(though this obviously doesn't produce terribly pretty HTML, if anyone cares. I prefer pretty templates.)
This is dangerous bikeshedding territory, but it seems sensible to have a standard within core code, and would also allow the production of django-template emacs modes, TextMate bundles, etc.
comment:5 by , 13 years ago
Actually you can use "from" syntax in the load tag, can't you; in those cases it certainly makes sense to keep them on separate lines. Though perhaps that isn't what Gabriel was advocating in the first place; on re-reading he may have been simply saying it shouldn't be on the same line as {% extends %}
(which of course must be at the top).
comment:6 by , 6 years ago
Cc: | added |
---|
By now, there is a place to add further template style guides (currently it only says to add one space between {{
and the variable name. So we'd need a consensus on what a good template style would mean: Indented for template readability or for meaningful HTML indentation? Indented by two spaces or four?
I'd be all for two spaces and indentation as proposed by Simon.
comment:7 by , 11 months ago
Cc: | added |
---|
It looks like there hasn't been much movement on this ticket for a while, and the docs located here haven't made any changes to the Template Section. I'm wondering if this is actually something that is still needed or not?
If it is, I'm happy to try and gather consensus on what the docs should recommend.
If not, perhaps we just mark the ticket as closed?
comment:8 by , 11 months ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:9 by , 11 months ago
Hello Ryan, thanks for going thru these old tickets to try to make a decision about them.
Given that the ticket is already accepted, I would suggest to push this ticket forward by pursuing the 3 tasks detailed in the first comment (with the only caveat that the Django Forum would be preferred instead of the Django Developers list), which I agree are the first steps before making any docs changes:
- An overview of the various styles found in the current templates,
- A draft patch based on whatever the most common/best practice seems to be,
- A thread on the Django Forum to let folks debate the options.
Looking forward to your forum post!
comment:10 by , 11 months ago
I ran the following bash command (which is returning some false positives, for example {al
is finding an inline javascrpt alert) to get a sense of what is current in the django/django
directory
find django/django -type f -name "*.html" -exec grep -E "(\{\{[^}]*\}\})|(\{%[^%]*%\})" {} + | grep -o -E "\{.." | sort | uniq -c
This gives the following usages
{{
Count | Symbol |
---|---|
724 | {{ |
2 | {"| |
2 | {al |
1 | {if |
1 | {re |
1 | {vi |
1 | {x= |
1 | {{s |
{%
Count | Symbol |
---|---|
2121 | {% |
1 | {%- |
For the items that don't conform to {{
or {%
I'll look more deeply at the occurrences and provide more context for them, but on initial review it seems that the preference / standard seems to be {{
or {%
followed by a space
comment:11 by , 11 months ago
After reviewing the html files this is what I've found:
False positives from the script above
./contrib/admindocs/templates/admin_doc/bookmarklets.html
has inline javascript on line 22 which is the source of:
Count | Symbol |
---|---|
2 | {al |
1 | {if |
1 | {re |
1 | {vi |
1 | {x= |
./contrib/admindocs/templates/admin_doc/template_filter_index.html
line 23 which is triggering the {"|. This seems to fall outside of the requirements for this ticket./contrib/admindocs/templates/admin_doc/template_tag_index.html
line 23 which is triggering the {"|. This seems to fall outside of the requirements for this ticket./forms/jinja2/django/forms/widgets/multiwidget.html
linehas
{%-`. Per the jinja2 docs this is in place for whitespace control
Files where the standard isn't being followed
./views/templates/technical_500.html
on line 157 has<td>{{server_time|date:"r"}}</td>
Based on these findings it seems that the docs that are currently written are being enforced.
Next steps:
- [X] review
extends
andload
tag placement - [ ] placement of HTML in
block
- [ ]indentation practices for code inside of
block
comment:12 by , 11 months ago
Checking the {% extends}
using this bash command
`
find . -name "*.html" ! -path "./venv/*" -exec grep -n "% extends" {} + | awk -F: '$2 > 1 {print "Filename: " $1 ", Line number: " $2 ", Matched string: " $3}'
`
returns no results. It looks like the {{% extends}}
is always at the top line of the html
files
comment:13 by , 11 months ago
Checking the {% loads
using this bash command
find . -name "*.html" ! -path "./venv/*" -exec grep -En "\{% load" {} + | awk -F: '$2 > 2 {print "Filename: " $1 ", Line number: " $2 ", Matched string: " $3}'
returns the following results:
- Filename: ./django/contrib/admin/templates/admin/auth/user/change_password.html, Line number: 3, Matched string: {% load admin_urls %}
- Filename: ./django/contrib/admin/templates/admin/index.html, Line number: 25, Matched string: {% load log %}
The first item is actually OK upon inspection as there are 2 {% load ... }
statements in the file
The second item ./django/contrib/admin/templates/admin/index.html
has {% load log %}
on line 25. This was first added in commit 52f5c9 so it's always been lower in the file.
Next step is to determine if moving the line up will break anything
follow-up: 15 comment:14 by , 11 months ago
I moved the {% load log %}
from line 25 to line 3 and there weren't any NEW test failures. Once a standard is decided on this could potentially be moved to the top and it doesn't look like any negative impact would be introduced (from a testing perspective anyway).
Next step is to create a Django app locally and see what happens
comment:15 by , 11 months ago
Replying to Ryan Cheley:
I moved the
{% load log %}
from line 25 to line 3 and there weren't any NEW test failures. Once a standard is decided on this could potentially be moved to the top and it doesn't look like any negative impact would be introduced (from a testing perspective anyway).
Next step is to create a Django app locally and see what happens
App locally seems to be behaving as expected.
Also, in reading the docs I was reminded that a {% load ... %}
can have more than one template tag added, so the two outliers here can be moved up into a single {% load ... }
command instead of multiple lines
This means that
- /django/contrib/admin/templates/admin/auth/user/change_password.html can have line 2 updated to
{% load i18n static admin_urls %}
- ./django/contrib/admin/templates/admin/index.html can have line 2 updated to be
{% load i18n static log %}
comment:16 by , 11 months ago
Looking at the {% block ... %}
I have this to locate the occurences
find . -name "*.html" ! -path "./venv/*" -exec grep -En "\{%\s*block\s+\w+\s*%\}" {} + | awk -F: '$2 > 2 {print "Filename: " $1 ", Line number: " $2 ", Matched string: " $3}'
This returns 365 files in 37 directories.
Further investigation will have to wait until next time though
comment:17 by , 10 months ago
I made a slight tweak to the script above
find . -name "*.html" ! -path "./venv/*" ! -path "*/build/*" -exec grep -En "\{%\s*block\s+\w+\s*%\}" {} + | awk -F: 'length($3) > 80 && $2 > 2 {print "\n# Filename: " $1 "\nLine number: " $2 "\nMatched string: \"" $3,"\""}' > results.md
There were 23 matches.
This will only get lines that are longer than 80 characters and then writes it to a file called results.md
. Initial visual inspection of that file finds the following:
- the
<link ...>
tends to be on a single line, see for example/django/contrib/admin/templates/admin/base.html
Line 6 - Simple logic for html element classes tends to be on a single line, see for example
./docs/_theme/djangodocs/layout.html
Line 80 - Single elements can are on a single line, see for example
./django/contrib/admin/templates/admin/base.html
Line 102
There are two files which may fall outside of the standard:
./django/contrib/admin/templates/admin/auth/user/change_password.html
Line 19./django/contrib/admin/templates/admin/change_form.html
Line 36
In each of those cases there are block elements at the end of the line so these seem more like false positives from my script that actual issues
The next step is going to be to post to the Forum and the Discord server for next steps
comment:19 by , 9 months ago
Description: | modified (diff) |
---|
comment:20 by , 9 months ago
Has patch: | set |
---|
comment:21 by , 9 months ago
Patch needs improvement: | set |
---|
comment:22 by , 9 months ago
Patch needs improvement: | unset |
---|---|
Triage Stage: | Accepted → Ready for checkin |
I'm in favor of standards in general. What will probably bring this issue to a consensus most quickly would be the following:
A few points that should be specified in a style guide for templates:
extends
andload
tags (at the top seems pretty standard).block
should always be on its own line or if short blocks should be kept on one line. See the admin base_site template for examples of both.block
should be indented. (commonly the answer seems to be "no")There are probably others, but this is what springs to mind so far.
Just to toss my two cents into the ring (how's that for a mixed metaphor?), here's what I think:
Even though there's no guiding document like PEP 8 for HTML, I think applying as much of PEP 8 as makes sense is going to be the simplest answer. It's an established and understood document that Python developers are accustomed to. A couple of items that would tend to apply:
load
andextends
) should be at the top of the file, on separate lines.<span class="timestamp hidden">
not<span class = " timestamp hidden" >
)I think overall readability and proper indentation of each individual template file is more important than what the HTML output looks like. I wrap my base templates with the
spaceless
tag though, so it matters less to me.Use 4 spaces for indentation. (I know 2 spaces is popular for HTML among many developers, but PEP 8 recommends 4 and changing to 2 for templates doesn't offer any substantive improvement while it decreases the visual separation).