Template-based widget rendering

[Preston Timmons]

Hi all,

I've been working through solutions for #15667 -- template based widget
rendering. This is a problem that was close to a solution at one time, but
stalled out due to performance concerns and difficulties with defining a
workable API to create configurable template loaders.

Now that Jinja2 is supported, performance isn't as much a concern. So,
I'd like to tackle the API portion of this.

This problem has two sides:

1) Converting individual widgets to be rendered with templates.
2) Deciding how to instantiate a user-customizable template engine.

The first is easy to accomplish, but the second isn't. I've written a
proposal for the second problem available here:

https://gist.github.com/prestontimmons/24a2a835bea590afb70b

In addition, a WIP branch is available here:

https://github.com/prestontimmons/django/tree/ticket-15667

Hopefully, we can finally get this in. :)

Preston

[Carl Meyer]

Hi Preston,

Post by Preston Timmons
I've been working through solutions for #15667 -- template based widget
rendering. This is a problem that was close to a solution at one time, but
stalled out due to performance concerns and difficulties with defining a
workable API to create configurable template loaders.
Now that Jinja2 is supported, performance isn't as much a concern. So,
I'd like to tackle the API portion of this.

Thanks for picking this up again!

Post by Preston Timmons
1) Converting individual widgets to be rendered with templates.
2) Deciding how to instantiate a user-customizable template engine.
The first is easy to accomplish, but the second isn't. I've written a
https://gist.github.com/prestontimmons/24a2a835bea590afb70b

This proposal looks excellent. A few comments:

1) I think the new `renderer` argument to `Widget.render()` should be
kept optional, and BoundField should have a fallback to not pass it in
if not accepted, in order to avoid requiring immediate
backwards-incompatible updates to custom widgets' render() methods. The
Widget.render() method's signature is documented, so it is covered by
the backwards-compatibility policy. The BoundField calling fallback can
have a deprecation path; keeping the renderer arg optional forever (with
fallback to default renderer from settings) doesn't seem like a terrible
idea.

2) I don't think that looking up the magic name 'forms' in `TEMPLATES`
is a good idea for overriding the forms template loader. I think it
would be better to just require using a custom renderer (which can be a
simple subclass of the built-in TemplateRenderer) for that case.

3) Regarding the template-precedence conundrum you mentioned: IMO the
best end-result here is to simply add 'django.forms' to
`INSTALLED_APPS`. This provides full flexibility (you can order
`INSTALLED_APPS` however you like to override with templates from a
different app, or of course override with templates in `DIRS`) without
requiring any new concepts. So then the question is just a
backwards-compatibility path for getting 'django.forms' into
`INSTALLED_APPS`. Here's what I'd propose:

The default `TemplateRenderer` has an `engine` cached-property, with the
following behavior:

a) If 'django.forms' is in `INSTALLED_APPS` and there is at least one
configured template engine with `APP_DIRS: True`, return `None`.

b) Otherwise, fallback to instantiating and returning a simple Jinja2
backend (similar to the `default_backend` in your gist, but I think even
more stripped down, with `APP_DIRS: False`). Raise a (pending)
deprecation warning.

If `TemplateRenderer.engine` is `None`, then `TemplateRenderer.render`
would use `django.template.loader.get_template` (thus using the same
engine fallback strategy as normal template rendering from a view).

Document in the form-rendering docs (and in the release upgrade notes,
which are linked to by the above deprecation warning) that if you want
to override the default form widget templates, you need to include
'django.forms' in `INSTALLED_APPS` and make sure you have at least one
template engine with `APP_DIRS: True`. Overriding widget templates is a
new feature, so it's OK if it requires explicit opt-in.

Once the deprecation warning runs its course, the default
`TemplateRenderer.engine` property would simply return `None` in all
cases (unless we add an `engine_name` class attr; see below), thus
deferring to normal template rendering, and there would be no
automatically-generated special template engine for forms.

I think this provides a full backwards-compatible path for the simplest
case, while still preserving all the flexibility you could want. If you
don't want the built-in templates at all, and want to provide all your
own, you don't need `APP_DIRS: True` or 'django.forms' in
`INSTALLED_APPS`. If you want to only use a specific template engine for
form widgets, you can subclass `TemplateRenderer` and override the
`engine` property to return your engine (which could be instantiated
right there in the renderer, or could be from the `TEMPLATES` setting
and fetched in the renderer using `get_engine`).

To make the latter customization even simpler, we could add an
`engine_name` attribute on `TemplateRenderer`, defaulting to `None`, but
if set to anything other than `None`, the `engine` property would call
`get_engine` with that name. Then the subclass only has to define the
`engine_name` attribute to enforce use of a specific engine.

Or if that type of customization turns out to be very common, we could
even go one step further and add a `FORM_RENDERER_TEMPLATE_ENGINE`
setting which can be set to a template engine name, to avoid needing to
subclass `TemplateRenderer` at all.

(My feeling is we should probably do the `TemplateRenderer.engine_name`
thing, but not the setting.)

Post by Preston Timmons
https://github.com/prestontimmons/django/tree/ticket-15667

Can you turn this into a PR marked [WIP] and link it from the ticket?
Having a PR, even if its not merge-ready yet, makes code review and
commenting much easier.

Post by Preston Timmons
Hopefully, we can finally get this in. :)

+1!

Carl

[Preston Timmons]

Hi Carl,

Thanks for the feedback. I agree with your suggestions. I didn't think about
running a check for a combination of INSTALLED_APPS and a loader with
APP_DIRS. That would be sufficient for customizing loading.

I'll update and convert this to a PR.

Preston

[Tim Graham]

I've been working on testing and writing documentation for Preston's PR:
https://github.com/django/django/pull/6498

About backwards compatibility, the current implementation which seems to be
based on Carl's proposal requires either a) 'django.forms' in
INSTALLED_APPS (so the APP_DIRS loader can find the DTL widget templates)
or b) Jinja2 to be installed so that the TemplateRenderer.default_engine()
initialization of Jinja2 doesn't fail. If these items were accepted and
acknowledged as upgrade requirements, it wasn't clear to me.

In trying to make the Django test suite pass without Jinja2 installed, I
noticed that the 'django.forms' in INSTALLED_APPS requirement is a bit
annoying because this requires any use of
@override_settings(INSTALLED_APPS=[...]) that does widget rendering to now
include 'django.forms'. <https://github.com/django/django/pull/6498>

Post by Preston Timmons
Hi Carl,
Thanks for the feedback. I agree with your suggestions. I didn't think about
running a check for a combination of INSTALLED_APPS and a loader with
APP_DIRS. That would be sufficient for customizing loading.
I'll update and convert this to a PR.
Preston

[Carl Meyer]

Hi Tim,

Post by Tim Graham
https://github.com/django/django/pull/6498
About backwards compatibility, the current implementation which seems to
be based on Carl's proposal requires either a) 'django.forms' in
INSTALLED_APPS (so the APP_DIRS loader can find the DTL widget
templates) or b) Jinja2 to be installed so that the
TemplateRenderer.default_engine() initialization of Jinja2 doesn't fail.
If these items were accepted and acknowledged as upgrade requirements,
it wasn't clear to me.

Nope, I just overlooked the "Jinja2 is not installed" case there. In
that case we should just use a DjangoTemplates engine instead of a
Jinja2 engine as the fallback, which is what you've done in
https://github.com/django/django/pull/6498/commits/83e0138a85ada2a8139af8a35629b78ec5e539d8

Post by Tim Graham
In trying to make the Django test suite pass without Jinja2 installed, I
noticed that the 'django.forms' in INSTALLED_APPS requirement is a bit
annoying because this requires any use of
@override_settings(INSTALLED_APPS=[...]) that does widget rendering to
now include 'django.forms'.<https://github.com/django/django/pull/6498>

Yeah... so relatedly, you also mentioned in IRC and on GH that you don't
see why we should deprecate the automatic fallback in case you don't
have django.forms in INSTALLED_APPS and a loader with APP_DIRS=True. The
answer to that, IMO, is that checking specifically for "django.forms in
INSTALLED_APPS and APP_DIRS=True" is janky and inadequate, and I really
don't think it belongs in there permanently. In fact I'm wondering now
whether it even belongs in there temporarily as a deprecation path.

The problem with that check is that there are various legitimate ways
one could use the TemplateRenderer, and putting django.forms in
INSTALLED_APPS and using a loader with APP_DIRS=True is only one of
them. If you don't want to use APP_DIRS=True, it's also reasonable to
put a separate engine in TEMPLATES pointing to the form templates (this
might be a nicer solution for the Django test suite, for instance). Some
people might even want to supply all their own form templates and not
use any of the default ones, and that should be supported, too. By
automatically overriding with our own fallback template engine if we
don't detect "django.forms in INSTALLED_APPS and APP_DIRS=True", we
effectively prohibit any other approach. I thought that might be OK as a
temporary state of affairs during a deprecation path, but I definitely
don't think it's OK permanently.

I pushed an alternative approach in
https://github.com/carljm/django/commit/7d734cfb9da2f64e4bf59c55167c70748b3bd092
that removes the INSTALLED_APPS checking. Instead, it has the `render`
method unconditionally fallback to the built-in templates if a template
is not found via the engines configured in TEMPLATES.

I still think that in the long run, it's simpler and more predictable if
the developer is simply responsible to either set up TEMPLATES such that
the form templates needed are findable (startproject would set this up
by default), or use a custom subclass of TemplateRenderer that does
something entirely different. So I would still favor deprecating the
fallback (in which case we should also update the startproject template
so the fallback isn't needed for new projects). But I think this
fallback is more flexible and simple enough that we could consider
keeping it in place permanently, if you and others prefer that.

(I also removed the whole engine_name thing in that commit; it doesn't
really seem worth it. If you're subclassing TemplateRenderer, it's easy
enough to just override get_template and do whatever you like.)

If you like that approach, I can re-add some TemplateRenderer tests and
update the docs and push it to the main `15667` branch.

Carl

[Claude Paroz]

Post by Carl Meyer
I pushed an alternative approach in
https://github.com/carljm/django/commit/7d734cfb9da2f64e4bf59c55167c70748b3bd092
that removes the INSTALLED_APPS checking. Instead, it has the `render`
method unconditionally fallback to the built-in templates if a template
is not found via the engines configured in TEMPLATES.

I didn't follow closely this work, but a definite +1 to a sane fallback
(and keeping it). Having to fiddle with INSTALLED_APPS just to get forms
rendered as before doesn't look nice to me. Sorry if I missed the point.

Claude

[Aymeric Augustin]

I agree with Carl’s arguments about the fallback strategy (specifically,
the four paragraphs quoted below).
--
Aymeric.

Post by Carl Meyer
Yeah... so relatedly, you also mentioned in IRC and on GH that you don't
see why we should deprecate the automatic fallback in case you don't
have django.forms in INSTALLED_APPS and a loader with APP_DIRS=True. The
answer to that, IMO, is that checking specifically for "django.forms in
INSTALLED_APPS and APP_DIRS=True" is janky and inadequate, and I really
don't think it belongs in there permanently. In fact I'm wondering now
whether it even belongs in there temporarily as a deprecation path.
The problem with that check is that there are various legitimate ways
one could use the TemplateRenderer, and putting django.forms in
INSTALLED_APPS and using a loader with APP_DIRS=True is only one of
them. If you don't want to use APP_DIRS=True, it's also reasonable to
put a separate engine in TEMPLATES pointing to the form templates (this
might be a nicer solution for the Django test suite, for instance). Some
people might even want to supply all their own form templates and not
use any of the default ones, and that should be supported, too. By
automatically overriding with our own fallback template engine if we
don't detect "django.forms in INSTALLED_APPS and APP_DIRS=True", we
effectively prohibit any other approach. I thought that might be OK as a
temporary state of affairs during a deprecation path, but I definitely
don't think it's OK permanently.
I pushed an alternative approach in
https://github.com/carljm/django/commit/7d734cfb9da2f64e4bf59c55167c70748b3bd092
that removes the INSTALLED_APPS checking. Instead, it has the `render`
method unconditionally fallback to the built-in templates if a template
is not found via the engines configured in TEMPLATES.
I still think that in the long run, it's simpler and more predictable if
the developer is simply responsible to either set up TEMPLATES such that
the form templates needed are findable (startproject would set this up
by default), or use a custom subclass of TemplateRenderer that does
something entirely different. So I would still favor deprecating the
fallback (in which case we should also update the startproject template
so the fallback isn't needed for new projects). But I think this
fallback is more flexible and simple enough that we could consider
keeping it in place permanently, if you and others prefer that.

[Tim Graham]

About the fallback engines, the main use case I have in mind (as Claude
alluded to) is if you want to use django.forms "standalone" without the
rest of Django. In that case, it seems like it would be nice not to require
someone to configure settings.TEMPLATES. Here's an alternate proposal:

Creating a "django.forms.renderers.templates.DefaultTemplateRenderer"
(exact name to be discussed) which uses the fallback engines and ignores
settings.TEMPLATES. This could be the default renderer for the FORM_RENDERER
setting, for backwards-compatibility and to allow django.forms standalone
usage by default. For more advanced uses, set the setting: FORM_RENDERER =
'django.forms.renderers.templates.TemplateRenderer' (which uses
django.template.loader.get_template and doesn't have any fallback engines).

I agree with Carl’s arguments about the fallback strategy (specifically,
the four paragraphs quoted below).
--
Aymeric.

Post by Carl Meyer
Yeah... so relatedly, you also mentioned in IRC and on GH that you don't
see why we should deprecate the automatic fallback in case you don't
have django.forms in INSTALLED_APPS and a loader with APP_DIRS=True. The
answer to that, IMO, is that checking specifically for "django.forms in
INSTALLED_APPS and APP_DIRS=True" is janky and inadequate, and I really
don't think it belongs in there permanently. In fact I'm wondering now
whether it even belongs in there temporarily as a deprecation path.
The problem with that check is that there are various legitimate ways
one could use the TemplateRenderer, and putting django.forms in
INSTALLED_APPS and using a loader with APP_DIRS=True is only one of
them. If you don't want to use APP_DIRS=True, it's also reasonable to
put a separate engine in TEMPLATES pointing to the form templates (this
might be a nicer solution for the Django test suite, for instance). Some
people might even want to supply all their own form templates and not
use any of the default ones, and that should be supported, too. By
automatically overriding with our own fallback template engine if we
don't detect "django.forms in INSTALLED_APPS and APP_DIRS=True", we
effectively prohibit any other approach. I thought that might be OK as a
temporary state of affairs during a deprecation path, but I definitely
don't think it's OK permanently.
I pushed an alternative approach in

https://github.com/carljm/django/commit/7d734cfb9da2f64e4bf59c55167c70748b3bd092

Post by Carl Meyer
that removes the INSTALLED_APPS checking. Instead, it has the `render`
method unconditionally fallback to the built-in templates if a template
is not found via the engines configured in TEMPLATES.
I still think that in the long run, it's simpler and more predictable if
the developer is simply responsible to either set up TEMPLATES such that
the form templates needed are findable (startproject would set this up
by default), or use a custom subclass of TemplateRenderer that does
something entirely different. So I would still favor deprecating the
fallback (in which case we should also update the startproject template
so the fallback isn't needed for new projects). But I think this
fallback is more flexible and simple enough that we could consider
keeping it in place permanently, if you and others prefer that.

[Carl Meyer]

Hi Tim,

Post by Tim Graham
About the fallback engines, the main use case I have in mind (as Claude
alluded to) is if you want to use django.forms "standalone" without the
rest of Django. In that case, it seems like it would be nice not to
require someone to configure settings.TEMPLATES. Here's an alternate
Creating a "django.forms.renderers.templates.DefaultTemplateRenderer"
(exact name to be discussed) which uses the fallback engines and ignores
settings.TEMPLATES. This could be the default renderer for the
FORM_RENDERER setting, for backwards-compatibility and to allow
django.forms standalone usage by default. For more advanced uses, set
the setting: FORM_RENDERER =
'django.forms.renderers.templates.TemplateRenderer' (which uses
django.template.loader.get_template and doesn't have any fallback engines).

Yeah, I considered this (my first version of my commit actually had two
different renderer classes like this). My concern is that I think this
proposal has the default backwards for what will actually be typical
usage. In my experience of using templated widgets for the last several
years (via django-floppyforms), the biggest value is the ability to
override specific widget templates with your own templates. So I think
overriding templates (within a normal Django project with TEMPLATES
configured) is the "basic usage" and standalone use of the forms library
is an "advanced use," not the other way around.

The proposed "DefaultTemplateRenderer" doesn't allow any template
overriding at all, because it can _only_ load the built-in templates. I
think in the long run it would be a mistake to have the default
FORM_RENDERER setting be a renderer that doesn't allow easily overriding
templates, and I don't think that we should allow the transition
concerns to override reaching the right long-term solution after a
transition path.

Carl

[Preston Timmons]

+1. I like the simpler fallback solution Carl has suggested also.

Preston

[Curtis Maloney]

Sorry for the late entry to the discussion, but I was looking over the
code and wondered about something.

In projects where I've used my django-sniplates for form rendering, it's
been helpful that I can have several different form widget sets within
the one project -- for instance, for side-by-side labels, or top-labels,
etc.

From what I can see so far of the code/docs, there's no way to override
which set of form widgets get used on a per-form basis... let alone
per-field.

Is this correct?

The only possible avenue I see is a custom renderer class that somehow
mangles the widget template paths...

--
Curtis

Post by Preston Timmons
+1. I like the simpler fallback solution Carl has suggested also.
Preston
--
You received this message because you are subscribed to the Google
Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-developers/a5f5ad63-71f0-4ba5-bd21-028d79b0a3b4%40googlegroups.com
<https://groups.google.com/d/msgid/django-developers/a5f5ad63-71f0-4ba5-bd21-028d79b0a3b4%40googlegroups.com?utm_medium=email&utm_source=footer>.
For more options, visit https://groups.google.com/d/optout.

[Preston Timmons]

Hey Curtis,

I think you're asking how this patch will help with form and field layouts?
If so, not that much. It only addresses moving the widget HTML that
currently is hardcoded in Python into templates.

For example, compare:

https://github.com/django/django/blob/master/django/forms/widgets.py#L272

to the template version:

https://github.com/django/django/blob/15667/django/forms/templates/django/forms/widgets/input.html

It also enables easier custom widgets, like the admin clearable file input:

https://github.com/django/django/blob/15667/django/contrib/admin/templates/admin/widgets/clearable_file_input.html

There's nothing in this patch that would hinder further development to
convert the form rendering methods, like `Form.as_p()` to be template
based also, or providing better rendering methods altogether.

With that said, yes the renderer class is able to be set per form class
and as an argument to `Form.__init__()`.

Preston

Post by Curtis Maloney
Sorry for the late entry to the discussion, but I was looking over the
code and wondered about something.
In projects where I've used my django-sniplates for form rendering, it's
been helpful that I can have several different form widget sets within
the one project -- for instance, for side-by-side labels, or top-labels,
etc.
From what I can see so far of the code/docs, there's no way to override
which set of form widgets get used on a per-form basis... let alone
per-field.
Is this correct?
The only possible avenue I see is a custom renderer class that somehow
mangles the widget template paths...
--
Curtis

[Curtis Maloney]

Post by Preston Timmons
Hey Curtis,
I think you're asking how this patch will help with form and field layouts?
If so, not that much. It only addresses moving the widget HTML that
currently is hardcoded in Python into templates.

No... was all to do with widget rendering...

Currently when you render a form field, its label comes along with it.

Does this patch _only_ changes the rendering of the input field, and not
the label?

Post by Preston Timmons
There's nothing in this patch that would hinder further development to
convert the form rendering methods, like `Form.as_p()` to be template
based also, or providing better rendering methods altogether.
With that said, yes the renderer class is able to be set per form class
and as an argument to `Form.__init__()`.

So this seems to leave us only part way to removing rendering decisions
from the form code -- your widgets can be templated, but your form code
still needs to decide which widgets...

--
Curtis

[Carl Meyer]

Post by Curtis Maloney
So this seems to leave us only part way to removing rendering decisions
from the form code -- your widgets can be templated, but your form code
still needs to decide which widgets...

Yep, one step at a time. With django-floppyforms templated widgets I've
used a template filter that allows switching a widget's template path in
the template that's rendering the form, placing more control into
templates. I think integrating something like that could make sense as a
next step.

(Since widgets are Python classes that control not only markup
generation but also data retrieval, control over widgets themselves
needs to remain in Python code. But the rendering template could be
given the option to customize the template used to render the widget.)

Carl

[Curtis Maloney]

Post by Carl Meyer

Post by Curtis Maloney
So this seems to leave us only part way to removing rendering decisions
from the form code -- your widgets can be templated, but your form code
still needs to decide which widgets...

Yep, one step at a time. With django-floppyforms templated widgets I've
used a template filter that allows switching a widget's template path in
the template that's rendering the form, placing more control into
templates. I think integrating something like that could make sense as a
next step.

Sniplates works by pulling {% blocks %} from a nominated template as
widgets -- these are widgets in the "macro" sense, rather than
specifically the form field sense.

It lets you load multiple widget templates, each in its own namespace.

Post by Carl Meyer
(Since widgets are Python classes that control not only markup
generation but also data retrieval, control over widgets themselves
needs to remain in Python code. But the rendering template could be
given the option to customize the template used to render the widget.)

The way I handle this in sniplates is the {% form_field %} tag
"explodes" all the relevant details from the BoundField into the context.

Since the field tag can select the widget set to use, as well as the
specific widget template, and other context overrides, it puts pretty
much all the rendering decisions into the template.

--
Curtis

[Tim Graham]

I'm not sure about how common the need for custom widget templates are.
Speaking for djangoproject.com and a few other small projects I maintain, I
don't think these projects would make use of them but maybe if the feature
is there, I might realize it would help in some places.

What's your proposal for changing the default TEMPLATES? Using Jinja2 or
DTL?

Since others have supported the idea, feel free to push your ideas to the
branch -- I won't be working on that branch for the rest of today.

Post by Carl Meyer
Hi Tim,

Post by Tim Graham
About the fallback engines, the main use case I have in mind (as Claude
alluded to) is if you want to use django.forms "standalone" without the
rest of Django. In that case, it seems like it would be nice not to
require someone to configure settings.TEMPLATES. Here's an alternate
Creating a "django.forms.renderers.templates.DefaultTemplateRenderer"
(exact name to be discussed) which uses the fallback engines and ignores
settings.TEMPLATES. This could be the default renderer for the
FORM_RENDERER setting, for backwards-compatibility and to allow
django.forms standalone usage by default. For more advanced uses, set
the setting: FORM_RENDERER =
'django.forms.renderers.templates.TemplateRenderer' (which uses
django.template.loader.get_template and doesn't have any fallback

engines).
Yeah, I considered this (my first version of my commit actually had two
different renderer classes like this). My concern is that I think this
proposal has the default backwards for what will actually be typical
usage. In my experience of using templated widgets for the last several
years (via django-floppyforms), the biggest value is the ability to
override specific widget templates with your own templates. So I think
overriding templates (within a normal Django project with TEMPLATES
configured) is the "basic usage" and standalone use of the forms library
is an "advanced use," not the other way around.
The proposed "DefaultTemplateRenderer" doesn't allow any template
overriding at all, because it can _only_ load the built-in templates. I
think in the long run it would be a mistake to have the default
FORM_RENDERER setting be a renderer that doesn't allow easily overriding
templates, and I don't think that we should allow the transition
concerns to override reaching the right long-term solution after a
transition path.
Carl

[Carl Meyer]

Post by Tim Graham
I'm not sure about how common the need for custom widget templates are.
Speaking for djangoproject.com and a few other small projects I
maintain, I don't think these projects would make use of them but maybe
if the feature is there, I might realize it would help in some places.

It's certainly not always needed; maybe "basic" is too strong a word.
Better to say that it's a very useful intermediate technique, especially
for more complex widgets with detailed UI needs.

We're not only talking about overriding templates for built-in widgets,
we're also talking about the ability to define your own custom widgets
with their own templates. If only the built-in widget templates are
available, you can't do that either. So given that templating is now the
norm for how to build widgets, we'd effectively be making it impossible
to create your own widget classes without changing the default form
renderer.

I'd flip it around and make the comparison with "standalone use of the
forms library without a configured TEMPLATES setting," which seems to be
the competing feature: is that really _more_ common than custom form
widgets? How many projects do you maintain that need that?

Post by Tim Graham
What's your proposal for changing the default TEMPLATES? Using Jinja2 or
DTL?

At some point maybe we can adopt Jinja2 as a required dependency. Until
then, the default startproject template can't use it, so I think DTL is
the only real option.

Post by Tim Graham
Since others have supported the idea, feel free to push your ideas to
the branch -- I won't be working on that branch for the rest of today.

Ok, will do.

Carl

[Carl Meyer]

Post by Carl Meyer

Post by Tim Graham
What's your proposal for changing the default TEMPLATES? Using Jinja2 or
DTL?

At some point maybe we can adopt Jinja2 as a required dependency. Until
then, the default startproject template can't use it, so I think DTL is
the only real option.

Oops, I should clarify here that I don't actually plan to change the
default TEMPLATES setting at all; we don't need to. The default
startproject TEMPLATES already includes an engine with APP_DIRS: True; I
would just add 'django.forms' to the default INSTALLED_APPS.

But even that only really makes sense if we're planning to deprecate the
automatic fallback to the built-in templates, so we want to push people
to configure their settings to explicitly include them. If we plan to
leave the fallback around permanently, there's no need to change the
startproject template at all.

Here are the pros and cons as I see them for deprecating the fallback
(all the pros apply only after the deprecation process would complete):

- pro: cleaner and simpler default TemplateRenderer, with less complex
code and tests to maintain.

- pro: simpler mental model of form template rendering, where the form
templates work just like any other kind of template, they aren't
magically always available.

- con: requires an update to settings (for most projects, just adding
'django.forms' to INSTALLED_APPS) to keep forms rendering as they do now
(once the deprecation process completes).

I like conceptual simplicity, and I care more about where we end up than
about what the deprecation path requires (IMO the "con" in that list
disappears once everyone has upgraded through the deprecation process,
whereas the "pros" are permanent), so I lean towards deprecating the
fallback, but I really don't feel strongly about it at all. Claude
prefers keeping the fallback around permanently. What do others think?

Carl

[Tim Graham]

So this discussion doesn't stall the rest of the patch, I suggest keeping
the fallbacks for now and deprecation them later if they cause confusion or
other problems.

Post by Carl Meyer

Post by Carl Meyer

Post by Tim Graham
What's your proposal for changing the default TEMPLATES? Using Jinja2

or

Post by Carl Meyer

Post by Tim Graham
DTL?

At some point maybe we can adopt Jinja2 as a required dependency. Until
then, the default startproject template can't use it, so I think DTL is
the only real option.

Oops, I should clarify here that I don't actually plan to change the
default TEMPLATES setting at all; we don't need to. The default
startproject TEMPLATES already includes an engine with APP_DIRS: True; I
would just add 'django.forms' to the default INSTALLED_APPS.
But even that only really makes sense if we're planning to deprecate the
automatic fallback to the built-in templates, so we want to push people
to configure their settings to explicitly include them. If we plan to
leave the fallback around permanently, there's no need to change the
startproject template at all.
Here are the pros and cons as I see them for deprecating the fallback
- pro: cleaner and simpler default TemplateRenderer, with less complex
code and tests to maintain.
- pro: simpler mental model of form template rendering, where the form
templates work just like any other kind of template, they aren't
magically always available.
- con: requires an update to settings (for most projects, just adding
'django.forms' to INSTALLED_APPS) to keep forms rendering as they do now
(once the deprecation process completes).
I like conceptual simplicity, and I care more about where we end up than
about what the deprecation path requires (IMO the "con" in that list
disappears once everyone has upgraded through the deprecation process,
whereas the "pros" are permanent), so I lean towards deprecating the
fallback, but I really don't feel strongly about it at all. Claude
prefers keeping the fallback around permanently. What do others think?
Carl

[Carl Meyer]

Post by Tim Graham
So this discussion doesn't stall the rest of the patch, I suggest
keeping the fallbacks for now and deprecation them later if they cause
confusion or other problems.

Yes, I think that makes sense.

Carl

[Tim Graham]

While testing this, I ran into a shortcoming with the fallback strategy for
backwards-compatibility.

If you have a DjangoTemplates backend configured with 'APP_DIRS': True (as
in the tutorial) and you try to visit /admin/auth/user/#/change/ which
renders the ReadOnlyPasswordHashWidget, the template system will crash with
TemplateDoesNotExist because it can find the widget's template using the
app_directories loader but not the 'django/forms/widgets/attrs.html'
template that the first template includes. Since the first template is
found using what's configured in TEMPLATES (which doesn't know about any of
the built-in form templates), the standalone engine needed to find the
built-in templates is ignored.

I guess it will affect every project that uses the admin. I can't think of
a simple solution other than adding a system check upgrade warning to
detect this situation ('django.contrib.admin' in INSTALLED_APPS but not
'django.forms') and advise the user to add 'django.forms' to
INSTALLED_APPS. Thoughts?

Post by Carl Meyer

Post by Tim Graham
So this discussion doesn't stall the rest of the patch, I suggest
keeping the fallbacks for now and deprecation them later if they cause
confusion or other problems.

Yes, I think that makes sense.
Carl

[Carl Meyer]

Yuck. Is there only one admin widget that includes a built-in widget? If
so, I think we would maybe be better off just duplicating that one
built-in widget in the admin templates.

That should read "built-in template," not "built-in widget."

Carl

[Claude Paroz]

Post by Tim Graham
(...)
I guess it will affect every project that uses the admin. I can't think
of a simple solution other than adding a system check upgrade warning to
detect this situation ('django.contrib.admin' in INSTALLED_APPS but not
'django.forms') and advise the user to add 'django.forms' to
INSTALLED_APPS. Thoughts?

I'm still answering with my naive hat: isn't it possible to simply always
consider forms in django.forms without requiring anything new in
INSTALLED_APPS? What would be the problem? (Sorry if I missed a discussion
about that).

Claude

[Claude Paroz]

...always consider templates (not forms, sorry ) in django.forms ...

[Tim Graham]

Carl, there are other widgets such as
admin/widgets/related_widget_wrapper.html which include the built-in widget
templates. It's actually a variable include, {% include
widget.template_name %}, so copying the included template isn't feasible in
this case.

Claude, I'm not quite sure what the implementation of your idea would look
like. One possibility would be automatically add the form template
directories in each template engine's 'DIRS' [0]. This doesn't seem like a
good separation of concerns though.

[0]
https://github.com/django/django/blob/aa69f36984adb6cba5763604150d99e89aa1ee71/django/template/backends/django.py#L32

Post by Claude Paroz
...always consider templates (not forms, sorry ) in django.forms ...

[Carl Meyer]

Post by Tim Graham
(...)
I guess it will affect every project that uses the admin. I can't
think of a simple solution other than adding a system check upgrade
warning to detect this situation ('django.contrib.admin' in
INSTALLED_APPS but not 'django.forms') and advise the user to add
'django.forms' to INSTALLED_APPS. Thoughts?
I'm still answering with my naive hat: isn't it possible to simply
always consider forms in django.forms without requiring anything new in
INSTALLED_APPS? What would be the problem? (Sorry if I missed a
discussion about that).

This is certainly possible, it's just (as Tim says) not clear where
exactly one would implement it without introducing an ugly bidirectional
coupling between forms and templates. It's one thing for form rendering
to now depend on templates, but for template engines (which currently
are configured explicitly and only know about the template directories
you tell them about) to also automatically and unconditionally know
about django.forms (even if you maybe aren't using forms at all -- maybe
you're using the template engine standalone for something else entirely)
doesn't seem right at all.

Carl

[Claude Paroz]

Post by Carl Meyer

Post by Claude Paroz
I'm still answering with my naive hat: isn't it possible to simply
always consider forms in django.forms without requiring anything new in
INSTALLED_APPS? What would be the problem? (Sorry if I missed a
discussion about that).

This is certainly possible, it's just (as Tim says) not clear where
exactly one would implement it without introducing an ugly bidirectional
coupling between forms and templates. It's one thing for form rendering
to now depend on templates, but for template engines (which currently
are configured explicitly and only know about the template directories
you tell them about) to also automatically and unconditionally know
about django.forms (even if you maybe aren't using forms at all -- maybe
you're using the template engine standalone for something else entirely)
doesn't seem right at all.

I can imagine that django.forms would then be responsible to tell the
template engine to add the template source. Maybe this would need a new API
in the template code?
It may be time for me to dive into the code to stop saying stupid things :-/

Claude

[Carl Meyer]

Post by Claude Paroz
I can imagine that django.forms would then be responsible to tell the
template engine to add the template source. Maybe this would need a new
API in the template code?

Yes, it's certainly possible that we could address this with a new API
in the template engines. That's a more significant change than I had
thought we needed, but it may be the best option.

I'm happy to work on this issue more, but it's not likely to be until
the PyCon sprints.

Carl

[Tim Graham]

I took a stab at backwards-compatibility with respect to allowing the admin
to work without adding 'django.forms' to INSTALLED_APPS. It seems to work
for at least the simple TEMPLATES = 'BACKEND':
'django.template.backends.django.DjangoTemplates', ...} case, although I'm
not proud of the code and I'm nervous it's a bit fragile. What do you think
of the approach? I thought I'd try something to get the conversation going
again. I've been reacting to some tickets based on the assumption that this
will get into 1.11 so I'd like to make it happen.

https://github.com/django/django/pull/6498/commits/1eba57277c7168bd491277a14554635bf4efbfc8

Post by Carl Meyer

Post by Claude Paroz
I can imagine that django.forms would then be responsible to tell the
template engine to add the template source. Maybe this would need a new
API in the template code?

Yes, it's certainly possible that we could address this with a new API
in the template engines. That's a more significant change than I had
thought we needed, but it may be the best option.
I'm happy to work on this issue more, but it's not likely to be until
the PyCon sprints.
Carl

[Carl Meyer]

Hi Tim,

Post by Tim Graham
I took a stab at backwards-compatibility with respect to allowing the
admin to work without adding 'django.forms' to INSTALLED_APPS. It seems
'django.template.backends.django.DjangoTemplates', ...} case, although
I'm not proud of the code and I'm nervous it's a bit fragile. What do
you think of the approach?

I have similar concerns about fragility, as well as the behavior being
complex and hard to keep a clear mental model of.

I went back and re-read this thread and took a fresh approach to the
problem, trying to adhere to "keep the easy things easy, make the hard
things possible" and keep the behavior as explicit as possible. Here's
what I came up with, let me know what you think:

We provide three built-in template renderer classes:

1. The default FORM_RENDERER is DjangoTemplateRenderer. It does not use
settings.TEMPLATES/get_template(), but constructs its own DTL template
engine. This engine has APP_DIRS=True, and automatically inserts the
built-in form templates directory _after_ the app dirs. (This will
require a subclass of the normal DTL Engine class that overrides the
template_dirs property, because DIRS takes priority over APP_DIRS, so
the default Engine has no way to specify a directory that has lower
priority than APP_DIRS. But I don't think needing this subclass is a
problem, and I do think it's important that out of the box you can
override the default form templates, as well as adding your own; this
latter of course is needed for the admin.)

2. We also provide JinjaTemplateRenderer, which is just like
DjangoTemplateRenderer, except it uses Jinja. (I.e. we get rid of the
"automatically use Jinja2 if it's installed" behavior of the
StandaloneTemplateRenderer in the current PR, and instead split into two
different renderers for DTL vs Jinja. Magically changing behavior just
because something is installed is bad. The choice to use Jinja should be
explicitly configured, not automatic when you install it.)

3. Lastly, we provide ProjectTemplateRenderer (name up for
bikeshedding), for those who want form template rendering to use their
overall project templating settings, as defined in TEMPLATES. This
renderer just uses the stock get_template() function. If you choose to
switch to this renderer, you will need to either put 'django.forms' in
INSTALLED_APPS (and have at least one engine with APP_DIRS=True), or add
the forms directory in DIRS of one of our engines, or supply all the
form templates you need yourself, or whatever. You've chosen to take
full control, so it's your responsibility to make the form templates you
need are available.

I think this meets all the requirements:

The default renderer is backwards-compatible with no settings changes,
and does not require Jinja2, but it still allows overrides/additions of
widget templates. It's also standalone-compatible, in that it is
self-contained and has no dependency on TEMPLATES config.

Switching to Jinja2 is just a six-character change to your FORM_RENDERER
setting.

If you want complete control of how your widget templates are sourced,
switch to ProjectTemplateRenderer.

And of course you can always write your own renderer class if you need
even more control than that.

Anything I missed?

Carl

I thought I'd try something to get the

Post by Tim Graham
conversation going again. I've been reacting to some tickets based on
the assumption that this will get into 1.11 so I'd like to make it happen.

That would be great.

Carl

[Preston Timmons]

Carl,

The default renderer is backwards-compatible with no settings changes,

Post by Carl Meyer
and does not require Jinja2, but it still allows overrides/additions of
widget templates. It's also standalone-compatible, in that it is
self-contained and has no dependency on TEMPLATES config.

I like this idea. It's explicit and predictable. A lot simpler than our
current approach.

Preston

[Tim Graham]

This scheme seems to be working well so far.

One thing you may not have thought of is that switching
to JinjaTemplateRenderer is incompatible with the admin because jinja2
templates aren't provided for those widgets. I think the reasoning was that
they're complicated to convert due to the use of the i18n and static
template tags and (under the old rendering scheme) a DjangoTemplates
backend had to be available anyway for rendering the main admin templates.
So I think JinjaTemplateRenderer may not be that useful in practice as it
requires your project and all its third-party apps to provide Jinja2
templates for all widgets.

I used these steps to use Jinja2 and allow the admin to continue using
DjangoTemplates:
1. Prepend this to the default settings.TEMPLATES:
{
'BACKEND': 'django.template.backends.jinja2.Jinja2',
'APP_DIRS': True,
}
2. Add 'django.forms' to INSTALLED_APPS.
3. Add settings.FORM_RENDERER =
'django.forms.renderers.templates.ProjectTemplateRenderer'

Post by Carl Meyer
Carl,
The default renderer is backwards-compatible with no settings changes,

Post by Carl Meyer
and does not require Jinja2, but it still allows overrides/additions of
widget templates. It's also standalone-compatible, in that it is
self-contained and has no dependency on TEMPLATES config.

I like this idea. It's explicit and predictable. A lot simpler than our
current approach.
Preston

[Carl Meyer]

Hi Tim,

Post by Tim Graham
This scheme seems to be working well so far.

Great, thanks for working on implementation. I did have intentions of
putting together the implementation once I'd gotten your feedback on the
design, but I won't complain if you've already done it :-)

Post by Tim Graham
One thing you may not have thought of is that switching
to JinjaTemplateRenderer is incompatible with the admin because jinja2
templates aren't provided for those widgets. I think the reasoning was
that they're complicated to convert due to the use of the i18n and
static template tags and (under the old rendering scheme) a
DjangoTemplates backend had to be available anyway for rendering the
main admin templates. So I think JinjaTemplateRenderer may not be that
useful in practice as it requires your project and all its third-party
apps to provide Jinja2 templates for all widgets.
I used these steps to use Jinja2 and allow the admin to continue using
{
'BACKEND': 'django.template.backends.jinja2.Jinja2',
'APP_DIRS': True,
}
2. Add 'django.forms' to INSTALLED_APPS.
3. Add settings.FORM_RENDERER =
'django.forms.renderers.templates.ProjectTemplateRenderer'

Makes sense. I still think we may as well provide JinjaTemplateRenderer,
for completeness, and because not all projects use the admin. But the
docs should be clear about the limitations, and point to
ProjectTemplateRenderer for more flexible setups.

Let me know once you have a PR that you feel is ready for review, happy
to review it.

Carl

[Tim Graham]

I'm not sure if having the default renders insert the built-in form
templates directory after the app dirs (as opposed to before, as usual with
'DIRS') is the best approach. On the PR, we discussed adding a new
TEMPLATES 'POST_APP_DIRS' option to accomplish this but it might be
unneeded complexity. I mentioned, "I'm concerned that it gives the idea
that third-party apps should be overriding templates to customize things
when that's really not appropriate since only the template in the earliest
installed app will be found." Carl said "Yeah, I agree that third-party
apps in most cases shouldn't be overriding form widgets (though I suppose I
could see some exceptions for e.g. something like a django-bootstrap-forms
app, if it clearly advertises that this is what it does)." If overriding
templates via app directories is the exception, should that behavior be
eased by default? I think using a custom renderer (perhaps even one
provided by the app itself) that uses DIRS would be a more explicit setup.

Post by Carl Meyer
Hi Tim,

Post by Tim Graham
This scheme seems to be working well so far.

Great, thanks for working on implementation. I did have intentions of
putting together the implementation once I'd gotten your feedback on the
design, but I won't complain if you've already done it :-)

Post by Tim Graham
One thing you may not have thought of is that switching
to JinjaTemplateRenderer is incompatible with the admin because jinja2
templates aren't provided for those widgets. I think the reasoning was
that they're complicated to convert due to the use of the i18n and
static template tags and (under the old rendering scheme) a
DjangoTemplates backend had to be available anyway for rendering the
main admin templates. So I think JinjaTemplateRenderer may not be that
useful in practice as it requires your project and all its third-party
apps to provide Jinja2 templates for all widgets.
I used these steps to use Jinja2 and allow the admin to continue using
{
'BACKEND': 'django.template.backends.jinja2.Jinja2',
'APP_DIRS': True,
}
2. Add 'django.forms' to INSTALLED_APPS.
3. Add settings.FORM_RENDERER =
'django.forms.renderers.templates.ProjectTemplateRenderer'

Makes sense. I still think we may as well provide JinjaTemplateRenderer,
for completeness, and because not all projects use the admin. But the
docs should be clear about the limitations, and point to
ProjectTemplateRenderer for more flexible setups.
Let me know once you have a PR that you feel is ready for review, happy
to review it.
Carl

[Carl Meyer]

Post by Tim Graham
I'm not sure if having the default renders insert the built-in form
templates directory after the app dirs (as opposed to before, as usual
with 'DIRS') is the best approach. On the PR, we discussed adding a new
TEMPLATES 'POST_APP_DIRS' option to accomplish this but it might be
unneeded complexity. I mentioned, "I'm concerned that it gives the idea
that third-party apps should be overriding templates to customize things
when that's really not appropriate since only the template in the
earliest installed app will be found." Carl said "Yeah, I agree that
third-party apps in most cases shouldn't be overriding form widgets
(though I suppose I could see some exceptions for e.g. something like a
django-bootstrap-forms app, if it clearly advertises that this is what
it does)." If overriding templates via app directories is the exception,
should that behavior be eased by default? I think using a custom
renderer (perhaps even one provided by the app itself) that uses DIRS
would be a more explicit setup.

There's a big difference between "apps" and "third-party apps" -- not
all apps are third-party; in fact I think most aren't. I've never seen a
project that didn't have installed "apps" that were part of the project
itself. In fact I've seen a number of people who put their main
"project" module itself in INSTALLED_APPS and get their "project-level"
templates rendered that way, and don't use template DIRS at all.

These "local" apps is the primary use case I'm thinking of for installed
apps overriding form widget templates, and I think it's a strong and
valuable use case that will be very commonly used once templated widgets
are in core. I've been using templated widgets (via django-floppyforms)
for many years, and my experience is that I've wanted to override
built-in widget templates to tweak their markup on every single project
I've used them on. Frankly, overriding widget templates is the primary
motivation for having templated widgets in the first place! I think its
important that the default setup make overriding easy.

As for whether a hypothetical third-party app that wants to override
form widgets should do it by just providing the template overrides and
clearly documenting that, or should do it by providing a custom form
renderer, that's a separate question. I wouldn't have a big problem with
either approach. There are already plenty of apps out there that
override e.g. the default admin templates; this isn't much different.

Carl

[Carl Meyer]

Post by Carl Meyer
As for whether a hypothetical third-party app that wants to override
form widgets should do it by just providing the template overrides and
clearly documenting that, or should do it by providing a custom form
renderer, that's a separate question. I wouldn't have a big problem with
either approach. There are already plenty of apps out there that
override e.g. the default admin templates; this isn't much different.

Actually, on further thought, I do have a strong opinion here -- such a
hypothetical third-party app should just provide the template overrides
in the expected location; it should not provide a custom form renderer.
The former is easier to use for the simple case (just add to
INSTALLED_APPS) and also more flexible and easier to
integrate/control/override at the project level (e.g. by modifying order
of INSTALLED_APPS, or using DIRS and ProjectTemplateRenderer, or using a
project-specific custom renderer) for advanced uses.

Whereas a custom app-specific renderer doesn't compose well with an
existing custom form renderer that a project might use, making advanced
use more difficult.

Carl

[Tim Graham]

My thinking is that there should typically be only one directory in each
project that overrides built-in templates, otherwise if multiple apps
provide overrides, they'll stomp on each other when using the
app_directories loader. Are your projects usually set up that way? Using
the app_directories loader eases setup but adds a cognitive overhead of
figuring out where the template overrides are coming from. If you feel the
ship has sailed and the pattern of putting the main "project" module in
INSTALLED_APPS to get "project-level" templates rendered should be
endorsed, then I guess we'll do that.

An alternative could be an additional setting to declare the
"project-level" templates directory. That would allow overriding built-in
and third-party app templates without create a "project" app. For example,
the engine for the DjangoTemplateRenderer would look like:

def engine(self):
dirs = [os.path.join(ROOT, 'templates')]
if settings.FORM_TEMPLATES_DIR:
dirs.insert(0, FORM_TEMPLATES_DIR)
return DjangoTemplates({
'APP_DIRS': True,
'DIRS': dirs,
'NAME': 'djangoforms',
'OPTIONS': {},
})

This loading order makes it impossible for apps to override the built-in
templates unless that app is pointed to by FORM_TEMPLATES_DIR. You couldn't
easily do further customizations of built-in templates without copying that
app's templates into a new directory and pointing FORM_TEMPLATES_DIR at it,
then editing those templates or adding new ones.

Post by Carl Meyer

Post by Carl Meyer
As for whether a hypothetical third-party app that wants to override
form widgets should do it by just providing the template overrides and
clearly documenting that, or should do it by providing a custom form
renderer, that's a separate question. I wouldn't have a big problem with
either approach. There are already plenty of apps out there that
override e.g. the default admin templates; this isn't much different.

Actually, on further thought, I do have a strong opinion here -- such a
hypothetical third-party app should just provide the template overrides
in the expected location; it should not provide a custom form renderer.
The former is easier to use for the simple case (just add to
INSTALLED_APPS) and also more flexible and easier to
integrate/control/override at the project level (e.g. by modifying order
of INSTALLED_APPS, or using DIRS and ProjectTemplateRenderer, or using a
project-specific custom renderer) for advanced uses.
Whereas a custom app-specific renderer doesn't compose well with an
existing custom form renderer that a project might use, making advanced
use more difficult.
Carl

[Carl Meyer]

Hi Tim,

Post by Tim Graham
My thinking is that there should typically be only one directory in each
project that overrides built-in templates, otherwise if multiple apps
provide overrides, they'll stomp on each other when using the
app_directories loader.Are your projects usually set up that way? Using
the app_directories loader eases setup but adds a cognitive overhead of
figuring out where the template overrides are coming from.

All of these seem like generic objections to APP_DIRS / the
app_directories loader, which has been around in some form (and been the
default behavior) effectively forever. For better or worse, we have a
template system that layers multiple loaders in the same template
namespace and allows them to stomp on each other, and leaves it up to
you to figure it out if they do. This does give people some rope to
create confusion, but it's also powerful and flexible, and so far we've
decided that tradeoff is worth it. How is any of this specific to form
templates?

Post by Tim Graham
If you feel
the ship has sailed and the pattern of putting the main "project" module
in INSTALLED_APPS to get "project-level" templates rendered should be
endorsed, then I guess we'll do that.

I think some kind of "project" or "core" app is in practice pretty
common (e.g. it also becomes necessary if you want "project-level"
management commands, or DTL template tags, or...). I'm sure it's not
universal; I don't think there's anything wrong with it, or anything
wrong with not doing it. And I don't think that any decision we make
here needs to imply an endorsement of one approach or the other.

I understand that in my proposed default setup, if a project relies on
DIRS for their project-level templates, they won't be able to override
form templates along with the rest of their project-level templates;
they'll either need to switch to the ProjectTemplateRenderer or put
their form override templates in an app. That's not ideal, but I think
it's still a reasonable set of options (I'd probably go for
ProjectTemplateRenderer in that situation -- "if you want project
templates to override form templates, use ProjectTemplateRenderer" seems
reasonable). While I agree this is a wrinkle, I don't think this wrinkle
is a good rationale for making it impossible to override built-in form
templates from an app in the default setup.

If you disagree with that, though, I could live with with just saying to
everyone "if you want to override form templates, use
ProjectTemplateRenderer" -- it's not that hard to switch to
ProjectTemplateRenderer.

I would not be in favor of the FORM_TEMPLATE_DIRS setting. It adds a
brand-new concept and setting, which would typically be set to the same
value as your template DIRS in the common case, without really gaining
any flexibility or much ease-of-use compared to just switching to
ProjectTemplateRenderer.

Carl

Post by Tim Graham
An alternative could be an additional setting to declare the
"project-level" templates directory. That would allow overriding
built-in and third-party app templates without create a "project" app.
dirs = [os.path.join(ROOT, 'templates')]
dirs.insert(0, FORM_TEMPLATES_DIR)
return DjangoTemplates({
'APP_DIRS': True,
'DIRS': dirs,
'NAME': 'djangoforms',
'OPTIONS': {},
})
This loading order makes it impossible for apps to override the built-in
templates unless that app is pointed to by FORM_TEMPLATES_DIR. You
couldn't easily do further customizations of built-in templates without
copying that app's templates into a new directory and pointing
FORM_TEMPLATES_DIR at it, then editing those templates or adding new ones.

[Tim Graham]

I don't have any strong objections at this point, just wanted to think it
through and offer possible alternatives. If there's no further input, I'll
finish adding tests and docs for the TEMPLATES 'POST_APP_DIRS' option
tomorrow, after which I think the widget rendering patch will be ready for
final reviews.

Post by Carl Meyer
Hi Tim,

Post by Tim Graham
My thinking is that there should typically be only one directory in each
project that overrides built-in templates, otherwise if multiple apps
provide overrides, they'll stomp on each other when using the
app_directories loader.Are your projects usually set up that way? Using
the app_directories loader eases setup but adds a cognitive overhead of
figuring out where the template overrides are coming from.

All of these seem like generic objections to APP_DIRS / the
app_directories loader, which has been around in some form (and been the
default behavior) effectively forever. For better or worse, we have a
template system that layers multiple loaders in the same template
namespace and allows them to stomp on each other, and leaves it up to
you to figure it out if they do. This does give people some rope to
create confusion, but it's also powerful and flexible, and so far we've
decided that tradeoff is worth it. How is any of this specific to form
templates?

Post by Tim Graham
If you feel
the ship has sailed and the pattern of putting the main "project" module
in INSTALLED_APPS to get "project-level" templates rendered should be
endorsed, then I guess we'll do that.

I think some kind of "project" or "core" app is in practice pretty
common (e.g. it also becomes necessary if you want "project-level"
management commands, or DTL template tags, or...). I'm sure it's not
universal; I don't think there's anything wrong with it, or anything
wrong with not doing it. And I don't think that any decision we make
here needs to imply an endorsement of one approach or the other.
I understand that in my proposed default setup, if a project relies on
DIRS for their project-level templates, they won't be able to override
form templates along with the rest of their project-level templates;
they'll either need to switch to the ProjectTemplateRenderer or put
their form override templates in an app. That's not ideal, but I think
it's still a reasonable set of options (I'd probably go for
ProjectTemplateRenderer in that situation -- "if you want project
templates to override form templates, use ProjectTemplateRenderer" seems
reasonable). While I agree this is a wrinkle, I don't think this wrinkle
is a good rationale for making it impossible to override built-in form
templates from an app in the default setup.
If you disagree with that, though, I could live with with just saying to
everyone "if you want to override form templates, use
ProjectTemplateRenderer" -- it's not that hard to switch to
ProjectTemplateRenderer.
I would not be in favor of the FORM_TEMPLATE_DIRS setting. It adds a
brand-new concept and setting, which would typically be set to the same
value as your template DIRS in the common case, without really gaining
any flexibility or much ease-of-use compared to just switching to
ProjectTemplateRenderer.
Carl

Post by Tim Graham
An alternative could be an additional setting to declare the
"project-level" templates directory. That would allow overriding
built-in and third-party app templates without create a "project" app.
dirs = [os.path.join(ROOT, 'templates')]
dirs.insert(0, FORM_TEMPLATES_DIR)
return DjangoTemplates({
'APP_DIRS': True,
'DIRS': dirs,
'NAME': 'djangoforms',
'OPTIONS': {},
})
This loading order makes it impossible for apps to override the built-in
templates unless that app is pointed to by FORM_TEMPLATES_DIR. You
couldn't easily do further customizations of built-in templates without
copying that app's templates into a new directory and pointing
FORM_TEMPLATES_DIR at it, then editing those templates or adding new

ones.

[charettes]

Did we consider defining a template loader that knows where to load the
widget
templates from instead of requiring 'django.forms' in INSTALLED_APPS with
'APP_DIRS': True in TEMPLATES?

Something along theese lines make more sense to me:

TEMPLATES = {
{
'BACKEND': 'django.template.backends.django.DjangoTemplates',
'DIRS': [],
'APP_DIRS': True,
'OPTIONS': {
'loaders': [
'django.forms.templates.loader.WidgetTemplateLoader',
]
},
},
}

Simon

Post by Tim Graham
While testing this, I ran into a shortcoming with the fallback strategy
for backwards-compatibility.
If you have a DjangoTemplates backend configured with 'APP_DIRS': True (as
in the tutorial) and you try to visit /admin/auth/user/#/change/ which
renders the ReadOnlyPasswordHashWidget, the template system will crash with
TemplateDoesNotExist because it can find the widget's template using the
app_directories loader but not the 'django/forms/widgets/attrs.html'
template that the first template includes. Since the first template is
found using what's configured in TEMPLATES (which doesn't know about any of
the built-in form templates), the standalone engine needed to find the
built-in templates is ignored.
I guess it will affect every project that uses the admin. I can't think
of a simple solution other than adding a system check upgrade warning to
detect this situation ('django.contrib.admin' in INSTALLED_APPS but not
'django.forms') and advise the user to add 'django.forms' to
INSTALLED_APPS. Thoughts?

Post by Carl Meyer

Post by Tim Graham
So this discussion doesn't stall the rest of the patch, I suggest
keeping the fallbacks for now and deprecation them later if they cause
confusion or other problems.

Yes, I think that makes sense.
Carl

[Tim Graham]

I'm not feeling particularly inspired about a solution for the
backwards-compatibility issue, and unless someone can work some magic today
in completing the patch, I think we'll defer this from 1.10.

Post by charettes
Did we consider defining a template loader that knows where to load the
widget
templates from instead of requiring 'django.forms' in INSTALLED_APPS with
'APP_DIRS': True in TEMPLATES?
TEMPLATES = {
{
'BACKEND': 'django.template.backends.django.DjangoTemplates',
'DIRS': [],
'APP_DIRS': True,
'OPTIONS': {
'loaders': [
'django.forms.templates.loader.WidgetTemplateLoader',
]
},
},
}
Simon

Post by Tim Graham
While testing this, I ran into a shortcoming with the fallback strategy
for backwards-compatibility.
If you have a DjangoTemplates backend configured with 'APP_DIRS': True
(as in the tutorial) and you try to visit /admin/auth/user/#/change/ which
renders the ReadOnlyPasswordHashWidget, the template system will crash with
TemplateDoesNotExist because it can find the widget's template using the
app_directories loader but not the 'django/forms/widgets/attrs.html'
template that the first template includes. Since the first template is
found using what's configured in TEMPLATES (which doesn't know about any of
the built-in form templates), the standalone engine needed to find the
built-in templates is ignored.
I guess it will affect every project that uses the admin. I can't think
of a simple solution other than adding a system check upgrade warning to
detect this situation ('django.contrib.admin' in INSTALLED_APPS but not
'django.forms') and advise the user to add 'django.forms' to
INSTALLED_APPS. Thoughts?

Post by Carl Meyer

Post by Tim Graham
So this discussion doesn't stall the rest of the patch, I suggest
keeping the fallbacks for now and deprecation them later if they cause
confusion or other problems.

Yes, I think that makes sense.
Carl

[Carl Meyer]

Hi Simon,

Post by charettes
Did we consider defining a template loader that knows where to load the
widget
templates from instead of requiring 'django.forms' in INSTALLED_APPS with
'APP_DIRS': True in TEMPLATES?
TEMPLATES = {
{
'BACKEND': 'django.template.backends.django.DjangoTemplates',
'DIRS': [],
'APP_DIRS': True,
'OPTIONS': {
'loaders': [
'django.forms.templates.loader.WidgetTemplateLoader',
]
},
},
}

I think the solution needs to be at a higher level than DTL loaders,
because it should work also for Jinja2.

And this approach doesn't really help solve the backwards-compatibility
path, unless we'd automatically insert such a loader if it's not listed
explicitly.

Carl