Re: [Django] #14831: Django Template Style Guide
Django
-------------------------------------+-------------------------------------
Reporter: DrMeers | Owner: nobody
Type: New | Status: new
feature | Component: Documentation
Milestone: | Severity: Normal
Version: 1.2 | Keywords: template, style,
Resolution: | format
Triage Stage: Accepted | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Changes (by DrMeers):
* ui_ux: => 0
* easy: => 0
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/14831#comment:4>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: DrMeers | Owner: nobody
Type: New | Status: new
feature | Component: Documentation
Milestone: | Severity: Normal
Version: 1.2 | Keywords: template, style,
Resolution: | format
Triage Stage: Accepted | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Comment (by DrMeers):
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).
--
Ticket URL: <https://code.djangoproject.com/ticket/14831#comment:5>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: new
Component: Documentation | Version: 1.2
Severity: Normal | Resolution:
Keywords: template, style, | Triage Stage: Accepted
format |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Tobias Kunze (added)
Comment:
By now, there is a
[https://docs.djangoproject.com/en/dev/internals/contributing/writing-code
/coding-style/#template-style 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
[https://code.djangoproject.com/ticket/14831#comment:4 Simon].
--
Ticket URL: <https://code.djangoproject.com/ticket/14831#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Simon Meers | Owner: nobody
Type: New feature | Status: new
Component: Documentation | Version: 1.2
Severity: Normal | Resolution:
Keywords: template, style, | Triage Stage: Accepted
format |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Ryan Cheley (added)
Comment:
It looks like there hasn't been much movement on this ticket for a while,
and the docs located
[https://docs.djangoproject.com/en/dev/internals/contributing/writing-code
/coding-style/#template-style 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?
--
Ticket URL: <https://code.djangoproject.com/ticket/14831#comment:7>
Django
-------------------------------------+-------------------------------------
| Cheley
Type: New feature | Status: assigned
Component: Documentation | Version: 1.2
Severity: Normal | Resolution:
Keywords: template, style, | Triage Stage: Accepted
format |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Ryan Cheley):
* owner: nobody => Ryan Cheley
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/14831#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Simon Meers | Owner: Ryan
| Cheley
Type: New feature | Status: assigned
Component: Documentation | Version: 1.2
Severity: Normal | Resolution:
Keywords: template, style, | Triage Stage: Accepted
format |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Natalia Bidart):
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:
1. An overview of the various styles found in the current templates,
2. A draft patch based on whatever the most common/best practice seems to
be,
3. A thread on the Django Forum to let folks debate the options.
Looking forward to your forum post!
--
Ticket URL: <https://code.djangoproject.com/ticket/14831#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: Simon Meers | Owner: Ryan
| Cheley
Type: New feature | Status: assigned
Component: Documentation | Version: 1.2
Severity: Normal | Resolution:
Keywords: template, style, | Triage Stage: Accepted
format |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Ryan Cheley):
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
--
Ticket URL: <https://code.djangoproject.com/ticket/14831#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: Simon Meers | Owner: Ryan
| Cheley
Type: New feature | Status: assigned
Component: Documentation | Version: 1.2
Severity: Normal | Resolution:
Keywords: template, style, | Triage Stage: Accepted
format |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Ryan Cheley):
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` line ` has `{%-`.
Per the [https://jinja.palletsprojects.com/en/3.1.x/templates/#whitespace-
control 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:
- review `extends` and `load` tag placement
- placement of HTML in `block`
- indentation practices for code inside of `block`
--
Ticket URL: <https://code.djangoproject.com/ticket/14831#comment:11>
Django
-------------------------------------+-------------------------------------
Reporter: Simon Meers | Owner: Ryan
| Cheley
Type: New feature | Status: assigned
Component: Documentation | Version: 1.2
Severity: Normal | Resolution:
Keywords: template, style, | Triage Stage: Accepted
format |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Ryan Cheley):
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
--
Ticket URL: <https://code.djangoproject.com/ticket/14831#comment:12>
Django
-------------------------------------+-------------------------------------
Reporter: Simon Meers | Owner: Ryan
| Cheley
Type: New feature | Status: assigned
Component: Documentation | Version: 1.2
Severity: Normal | Resolution:
Keywords: template, style, | Triage Stage: Accepted
format |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Ryan Cheley):
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
[https://github.com/django/django/blob/52f5c949e9b50ae3b0034d67f2d385219588aaf1/templates/admin/index.html
52f5c9] so it's always been lower in the file.
Next step is to determine if moving the line up will break anything
--
Ticket URL: <https://code.djangoproject.com/ticket/14831#comment:13>
Django
-------------------------------------+-------------------------------------
Reporter: Simon Meers | Owner: Ryan
| Cheley
Type: New feature | Status: assigned
Component: Documentation | Version: 1.2
Severity: Normal | Resolution:
Keywords: template, style, | Triage Stage: Accepted
format |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by 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
--
Ticket URL: <https://code.djangoproject.com/ticket/14831#comment:14>
Django
-------------------------------------+-------------------------------------
Reporter: Simon Meers | Owner: Ryan
| Cheley
Type: New feature | Status: assigned
Component: Documentation | Version: 1.2
Severity: Normal | Resolution:
Keywords: template, style, | Triage Stage: Accepted
format |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Ryan Cheley):
Replying to [comment:14 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 %}`
--
Ticket URL: <https://code.djangoproject.com/ticket/14831#comment:15>
Django
-------------------------------------+-------------------------------------
Reporter: Simon Meers | Owner: Ryan
| Cheley
Type: New feature | Status: assigned
Component: Documentation | Version: 1.2
Severity: Normal | Resolution:
Keywords: template, style, | Triage Stage: Accepted
format |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Ryan Cheley):
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
--
Ticket URL: <https://code.djangoproject.com/ticket/14831#comment:16>
Django
-------------------------------------+-------------------------------------
Reporter: Simon Meers | Owner: Ryan
| Cheley
Type: New feature | Status: assigned
Component: Documentation | Version: 1.2
Severity: Normal | Resolution:
Keywords: template, style, | Triage Stage: Accepted
format |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Ryan Cheley):
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:
1. the `<link ...>` tends to be on a single line, see for example
`/django/contrib/admin/templates/admin/base.html`
[https://github.com/django/django/blob/9c6d7b4a678b7bbc6a1a14420f686162ba9016f5/django/contrib/admin/templates/admin/base.html#L6
Line 6]
2. Simple logic for html element classes tends to be on a single line, see
for example `./docs/_theme/djangodocs/layout.html`
[https://github.com/django/django/blob/9c6d7b4a678b7bbc6a1a14420f686162ba9016f5/docs/_theme/djangodocs/layout.html#L80
Line 80]
3. Single elements can are on a single line, see for example
`./django/contrib/admin/templates/admin/base.html`
[https://github.com/django/django/blob/9c6d7b4a678b7bbc6a1a14420f686162ba9016f5/django/contrib/admin/templates/admin/base.html#L102
Line 102]
There are two files which **may** fall outside of the standard:
* `./django/contrib/admin/templates/admin/auth/user/change_password.html`
[https://github.com/django/django/blob/9c6d7b4a678b7bbc6a1a14420f686162ba9016f5/django/contrib/admin/templates/admin/auth/user/change_password.html#L19
Line 19]
* `./django/contrib/admin/templates/admin/change_form.html`
[https://github.com/django/django/blob/9c6d7b4a678b7bbc6a1a14420f686162ba9016f5/django/contrib/admin/templates/admin/change_form.html#L36
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
--
Ticket URL: <https://code.djangoproject.com/ticket/14831#comment:17>
Django
-------------------------------------+-------------------------------------
Reporter: Simon Meers | Owner: Ryan
| Cheley
Type: New feature | Status: assigned
Component: Documentation | Version: 1.2
Severity: Normal | Resolution:
Keywords: template, style, | Triage Stage: Accepted
format |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Ryan Cheley):
style-guide-ticket-14831/28177
--
Ticket URL: <https://code.djangoproject.com/ticket/14831#comment:18>
Django
-------------------------------------+-------------------------------------
Reporter: Simon Meers | Owner: Ryan
| Cheley
Type: New feature | Status: assigned
Component: Documentation | Version: 1.2
Severity: Normal | Resolution:
Keywords: template, style, | Triage Stage: Accepted
format |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
> 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
> http://docs.djangoproject.com/en/dev/internals/contributing/#coding-style
New description:
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
--
--
Ticket URL: <https://code.djangoproject.com/ticket/14831#comment:19>
Django
-------------------------------------+-------------------------------------
Reporter: Simon Meers | Owner: Ryan
| Cheley
Type: New feature | Status: assigned
Component: Documentation | Version: 1.2
Severity: Normal | Resolution:
Keywords: template, style, | Triage Stage: Accepted
format |
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/14831#comment:20>
Django
-------------------------------------+-------------------------------------
Reporter: Simon Meers | Owner: Ryan
| Cheley
Type: New feature | Status: assigned
Component: Documentation | Version: 1.2
Severity: Normal | Resolution:
Keywords: template, style, | Triage Stage: Accepted
format |
Has patch: 1 | Needs documentation: 0
-------------------------------------+-------------------------------------
Changes (by Ryan Cheley):
--
Ticket URL: <https://code.djangoproject.com/ticket/14831#comment:21>
Django
-------------------------------------+-------------------------------------
Reporter: Simon Meers | Owner: Ryan
| Cheley
Type: New feature | Status: assigned
Component: Documentation | Version: 1.2
Severity: Normal | Resolution:
format | checkin
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/14831#comment:22>
Django
-------------------------------------+-------------------------------------
Reporter: Simon Meers | Owner: Ryan
| Cheley
Component: Documentation | Version: 1.2
Severity: Normal | Resolution: fixed
format | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* resolution: => fixed
* status: assigned => closed
Comment:
In [changeset:"f2c35249591d202ee544eadcaa9de4c255045692" f2c35249]:
{{{#!CommitTicketReference repository=""
revision="f2c35249591d202ee544eadcaa9de4c255045692"
Fixed #14831 -- Extended template style guide in docs.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/14831#comment:23>