Template-based widget rendering
1,089 views
Skip to first unread message
Preston Timmons
Jun 10, 2015, 1:46:00 PM6/10/15
to django-d...@googlegroups.com
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:
In addition, a WIP branch is available here:
Hopefully, we can finally get this in. :)
Preston
Carl Meyer
Jun 10, 2015, 5:16:49 PM6/10/15
to django-d...@googlegroups.com
Hi Preston,
On 06/10/2015 11:46 AM, Preston Timmons wrote:
> 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!
> 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
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.)
> In addition, a WIP branch is available here:
>
> 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.
> Hopefully, we can finally get this in. :)
+1!
Carl
On 06/10/2015 11:46 AM, Preston Timmons wrote:
> 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
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.)
> In addition, a WIP branch is available here:
>
> https://github.com/prestontimmons/django/tree/ticket-15667
Having a PR, even if its not merge-ready yet, makes code review and
commenting much easier.
> Hopefully, we can finally get this in. :)
Carl
Preston Timmons
Jun 10, 2015, 6:14:11 PM6/10/15
to django-d...@googlegroups.com
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
May 9, 2016, 10:03:29 AM5/9/16
to Django developers (Contributions to Django itself)
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'.
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'.
Carl Meyer
May 9, 2016, 6:28:57 PM5/9/16
to django-d...@googlegroups.com
Hi Tim,
On 05/09/2016 08:03 AM, Tim Graham wrote:
> 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.
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
> 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
On 05/09/2016 08:03 AM, Tim Graham wrote:
> 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.
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
> 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
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
May 10, 2016, 2:19:56 AM5/10/16
to Django developers (Contributions to Django itself)
Le mardi 10 mai 2016 00:28:57 UTC+2, Carl Meyer a écrit :
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
May 10, 2016, 2:35:50 AM5/10/16
to django-d...@googlegroups.com
I agree with Carl’s arguments about the fallback strategy (specifically,
the four paragraphs quoted below).
--
Aymeric.
the four paragraphs quoted below).
--
Aymeric.
Tim Graham
May 10, 2016, 9:10:38 AM5/10/16
to Django developers (Contributions to Django itself)
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).
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).
Carl Meyer
May 10, 2016, 2:22:14 PM5/10/16
to django-d...@googlegroups.com
Hi Tim,
On 05/10/2016 07:10 AM, Tim Graham wrote:
> 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).
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
On 05/10/2016 07:10 AM, Tim Graham wrote:
> 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).
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
May 10, 2016, 11:21:30 PM5/10/16
to Django developers (Contributions to Django itself)
+1. I like the simpler fallback solution Carl has suggested also.
Preston
Curtis Maloney
May 10, 2016, 11:32:30 PM5/10/16
to django-d...@googlegroups.com
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
On 11/05/16 13:21, Preston Timmons wrote:
> +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
> an email to django-develop...@googlegroups.com
> <mailto:django-develop...@googlegroups.com>.
> To post to this group, send email to django-d...@googlegroups.com
> <mailto:django-d...@googlegroups.com>.
> 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.
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
On 11/05/16 13:21, Preston Timmons wrote:
> +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
> an email to django-develop...@googlegroups.com
> <mailto:django-develop...@googlegroups.com>.
> To post to this group, send email to django-d...@googlegroups.com
> <mailto:django-d...@googlegroups.com>.
> 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
May 11, 2016, 11:04:03 AM5/11/16
to Django developers (Contributions to Django itself)
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:
to the template version:
It also enables easier custom widgets, like the admin clearable file input:
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
Tim Graham
May 11, 2016, 1:30:33 PM5/11/16
to Django developers (Contributions to Django itself)
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.
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.
Carl Meyer
May 11, 2016, 1:53:17 PM5/11/16
to django-d...@googlegroups.com
On 05/11/2016 11:30 AM, Tim Graham wrote:
> 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.
> 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.
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?
> What's your proposal for changing the default TEMPLATES? Using Jinja2 or
> DTL?
then, the default startproject template can't use it, so I think DTL is
the only real option.
> 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.
Carl
Carl Meyer
May 11, 2016, 2:08:31 PM5/11/16
to django-d...@googlegroups.com
On 05/11/2016 11:52 AM, Carl Meyer wrote:
> On 05/11/2016 11:30 AM, Tim Graham wrote:
>> 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
> On 05/11/2016 11:30 AM, Tim Graham wrote:
>> 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.
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
May 12, 2016, 11:39:36 AM5/12/16
to Django developers (Contributions to Django itself)
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.
Carl Meyer
May 12, 2016, 1:17:23 PM5/12/16
to django-d...@googlegroups.com
On 05/12/2016 09:39 AM, Tim Graham wrote:
> 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.
> 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.
Carl
Tim Graham
May 14, 2016, 10:03:57 AM5/14/16
to Django developers (Contributions to Django itself)
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?
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?
Carl Meyer
May 16, 2016, 12:55:06 AM5/16/16
to django-d...@googlegroups.com
On 05/15/2016 09:58 PM, Carl Meyer wrote:
> 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
> 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
Curtis Maloney
May 16, 2016, 1:01:39 AM5/16/16
to django-d...@googlegroups.com
On 12/05/16 01:04, Preston Timmons wrote:
> 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...
> 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.
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?
> 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__()`.
from the form code -- your widgets can be templated, but your form code
still needs to decide which widgets...
--
Curtis
Carl Meyer
May 16, 2016, 1:10:53 AM5/16/16
to django-d...@googlegroups.com
On 05/15/2016 11:01 PM, Curtis Maloney wrote:
> 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
> 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...
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
May 16, 2016, 1:21:19 AM5/16/16
to django-d...@googlegroups.com
On 16/05/16 15:10, Carl Meyer wrote:
> On 05/15/2016 11:01 PM, Curtis Maloney wrote:
>> 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
> On 05/15/2016 11:01 PM, Curtis Maloney wrote:
>> 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.
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.
> (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.)
"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
Claude Paroz
May 16, 2016, 3:48:08 AM5/16/16
to Django developers (Contributions to Django itself)
Le samedi 14 mai 2016 16:03:57 UTC+2, Tim Graham a écrit :
(...)
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
May 16, 2016, 12:18:44 PM5/16/16
to Django developers (Contributions to Django itself)
...always consider templates (not forms, sorry ) in django.forms ...
Tim Graham
May 16, 2016, 12:43:04 PM5/16/16
to Django developers (Contributions to Django itself)
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
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
Carl Meyer
May 16, 2016, 8:07:54 PM5/16/16
to django-d...@googlegroups.com
Hi Tim,
On 05/14/2016 08:03 AM, Tim Graham wrote:
> 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?
I think it's better than giving up on seamless backward compatibility.
Then obviously we need to add a note to the documentation clarifying
that if you have any custom form templates that need to include or
extend any of the default templates, you should put 'django.forms' in
INSTALLED_APPS so all of the templates can be found by the same template
engine.
This might be another argument in favor of deprecating the fallback and
actively pushing people to just get 'django.forms' into their
INSTALLED_APPS. But we can wait on that, like you said.
Carl
On 05/14/2016 08:03 AM, Tim Graham wrote:
> 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?
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. Obviously that's not ideal, but
so, I think we would maybe be better off just duplicating that one
I think it's better than giving up on seamless backward compatibility.
Then obviously we need to add a note to the documentation clarifying
that if you have any custom form templates that need to include or
extend any of the default templates, you should put 'django.forms' in
INSTALLED_APPS so all of the templates can be found by the same template
engine.
This might be another argument in favor of deprecating the fallback and
actively pushing people to just get 'django.forms' into their
INSTALLED_APPS. But we can wait on that, like you said.
Carl
Carl Meyer
May 17, 2016, 2:03:52 AM5/17/16
to django-d...@googlegroups.com
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
May 17, 2016, 3:33:48 AM5/17/16
to Django developers (Contributions to Django itself)
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
charettes
May 17, 2016, 9:28:59 AM5/17/16
to Django developers (Contributions to Django itself)
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
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
Tim Graham
May 17, 2016, 11:15:02 AM5/17/16
to Django developers (Contributions to Django itself)
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.
Carl Meyer
May 18, 2016, 2:47:36 AM5/18/16
to django-d...@googlegroups.com
Hi Simon,
On 05/17/2016 07:28 AM, charettes wrote:
> 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',
> ]
> },
> },
> }
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
On 05/17/2016 07:28 AM, charettes wrote:
> 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',
> ]
> },
> },
> }
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
Carl Meyer
May 18, 2016, 2:49:52 AM5/18/16
to django-d...@googlegroups.com
On 05/17/2016 01:33 AM, Claude Paroz wrote:
> 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
> 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?
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
Nov 16, 2016, 9:23:03 AM11/16/16
to Django developers (Contributions to Django itself)
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
https://github.com/django/django/pull/6498/commits/1eba57277c7168bd491277a14554635bf4efbfc8
Carl Meyer
Dec 3, 2016, 2:13:11 AM12/3/16
to django-d...@googlegroups.com
Hi Tim,
On 11/16/2016 06:23 AM, Tim Graham wrote:
> 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 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
> 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
On 11/16/2016 06:23 AM, Tim Graham wrote:
> 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?
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
> 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.
Carl
Preston Timmons
Dec 3, 2016, 12:41:13 PM12/3/16
to Django developers (Contributions to Django itself)
Carl,
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.
I like this idea. It's explicit and predictable. A lot simpler than our current approach.
Preston
Tim Graham
Dec 7, 2016, 11:41:04 AM12/7/16
to Django developers (Contributions to Django itself)
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'
Carl Meyer
Dec 7, 2016, 12:00:57 PM12/7/16
to django-d...@googlegroups.com
Hi Tim,
On 12/07/2016 08:41 AM, Tim Graham wrote:
> 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 :-)
> 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'
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
On 12/07/2016 08:41 AM, Tim Graham wrote:
> This scheme seems to be working well so far.
putting together the implementation once I'd gotten your feedback on the
design, but I won't complain if you've already done it :-)
> 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'
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
Dec 14, 2016, 1:27:49 PM12/14/16
to Django developers (Contributions to Django itself)
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.
Carl Meyer
Dec 14, 2016, 2:09:11 PM12/14/16
to django-d...@googlegroups.com
On 12/14/2016 10:27 AM, Tim Graham wrote:
> 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
> 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.
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
Dec 14, 2016, 2:27:20 PM12/14/16
to django-d...@googlegroups.com
On 12/14/2016 11:08 AM, Carl Meyer wrote:
> 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
> 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.
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
Dec 14, 2016, 3:50:04 PM12/14/16
to Django developers (Contributions to Django itself)
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.
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.
Carl Meyer
Dec 14, 2016, 6:40:36 PM12/14/16
to django-d...@googlegroups.com
Hi Tim,
On 12/14/2016 12:50 PM, Tim Graham wrote:
> 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
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?
> 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
On 12/14/2016 12:50 PM, Tim Graham wrote:
> 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
> 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
> figuring out where the template overrides are coming from.
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?
> 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.
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
Tim Graham
Dec 14, 2016, 6:59:16 PM12/14/16
to Django developers (Contributions to Django itself)
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.
Message has been deleted
Tim Graham
Dec 19, 2016, 5:53:23 PM12/19/16
to Django developers (Contributions to Django itself)
It's ready for review:
Add 'POST_APP_DIRS' TEMPLATES option. https://github.com/django/django/pull/7693
Template based widget rendering: https://github.com/django/django/pull/6498
Add 'POST_APP_DIRS' TEMPLATES option. https://github.com/django/django/pull/7693
Template based widget rendering: https://github.com/django/django/pull/6498
Tim Graham
Dec 20, 2016, 1:34:21 PM12/20/16
to Django developers (Contributions to Django itself)
Florian started a long discussion [0] on the pull request and concluded in him saying, "If we are going to promote the usage of
ProjectTemplateRenderer (which I think we should), we probably should bite the dust and get rid of POST_APP_DIRS and in the same breath of the jinja renderer -- ie provide the Django renderer really only as backwards compat shim."One concern I have is that if you want to ues ProjectTemplateRenderer and render Jinja2 widget templates but use Django templates elsewhere, then you have to put a Jinja2 backend first in TEMPLATES which means all your non-form widget template lookups have an additional cost of checking if the Jinja2 template exists. Does anyone have a sense if that would be a noticeable performance penalty? (and perhaps an argument against ProjectTemplateRenderer for a use case like that)
Florian Apolloner
Dec 20, 2016, 2:16:43 PM12/20/16
to Django developers (Contributions to Django itself)
One option would be to introduce a new PREFIXES option in the template engine settings which ignores template paths not starting with one of those prefixes (if they are set). That said I didn't bother checking for performance issues, I know that the cached loader in Django will also cache if it doesn't find anything, ie it will not do extra I/O for non-existing templates (after the initial round). As far as I understand the Jinja2 code, non-existing file are extra I/O every time, so that might be something to consider.
Cheers,
Florian
Cheers,
Florian
Carl Meyer
Dec 20, 2016, 2:26:33 PM12/20/16
to django-d...@googlegroups.com
I think that a) wanting render forms via Jinja and everything else via
DTL, and b) caring about the perf impact of checking two engines is an
edge case, and a great reason to have the full flexibility of the form
renderers system. When we say "promote the usage of
ProjectTemplateRenderer", I think we mean as the most flexible and least
surprising option for most projects that just want Things To Work, not
that it will always meet everyone's needs. If you have more specific
needs, you can always write a form renderer that meets them.
Carl
> <https://www.google.com/url?q=https%3A%2F%2Fgithub.com%2Fdjango%2Fdjango%2Fpull%2F6498%23issuecomment-268120711&sa=D&sntz=1&usg=AFQjCNH6pYlfIAgKUN59fcVGOTnZ7FikCw>
> --
> 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
> an email to django-develop...@googlegroups.com
> <mailto:django-develop...@googlegroups.com>.
> To post to this group, send email to django-d...@googlegroups.com
> <mailto:django-d...@googlegroups.com>.
> 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/6e011e39-2e25-4d61-975f-866bf4a6a2a3%40googlegroups.com
> <https://groups.google.com/d/msgid/django-developers/6e011e39-2e25-4d61-975f-866bf4a6a2a3%40googlegroups.com?utm_medium=email&utm_source=footer>.
> For more options, visit https://groups.google.com/d/optout.
DTL, and b) caring about the perf impact of checking two engines is an
edge case, and a great reason to have the full flexibility of the form
renderers system. When we say "promote the usage of
ProjectTemplateRenderer", I think we mean as the most flexible and least
surprising option for most projects that just want Things To Work, not
that it will always meet everyone's needs. If you have more specific
needs, you can always write a form renderer that meets them.
Carl
> 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
> an email to django-develop...@googlegroups.com
> <mailto:django-develop...@googlegroups.com>.
> To post to this group, send email to django-d...@googlegroups.com
> <mailto:django-d...@googlegroups.com>.
> 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/6e011e39-2e25-4d61-975f-866bf4a6a2a3%40googlegroups.com
> <https://groups.google.com/d/msgid/django-developers/6e011e39-2e25-4d61-975f-866bf4a6a2a3%40googlegroups.com?utm_medium=email&utm_source=footer>.
> For more options, visit https://groups.google.com/d/optout.
Tim Graham
Dec 20, 2016, 2:55:43 PM12/20/16
to Django developers (Contributions to Django itself)
I thought maybe that use case could be an argument for keeping the Jinja2TemplateRenderer -- we need it for testing the Jinja2 templates anyway, so it's not adding any maintenance overhead, it's just a matter of whether it's public with docs or if it just lives in the tests.
About the fate of 'POST_APP_DIRS' -- it seems the consensus among you and Florian is to drop it. Fine with me.
About the fate of 'POST_APP_DIRS' -- it seems the consensus among you and Florian is to drop it. Fine with me.
> <mailto:django-developers+unsub...@googlegroups.com>.
Carl Meyer
Dec 20, 2016, 3:00:11 PM12/20/16
to django-d...@googlegroups.com
Sure, I guess Florian mentioned dropping Jinja2TemplateRenderer, but I
don't really see a strong argument against keeping and documenting it.
As you say, cost is low.
Carl
> <https://github.com/django/django/pull/6498#issuecomment-268120711>
> >
> <https://www.google.com/url?q=https%3A%2F%2Fgithub.com%2Fdjango%2Fdjango%2Fpull%2F6498%23issuecomment-268120711&sa=D&sntz=1&usg=AFQjCNH6pYlfIAgKUN59fcVGOTnZ7FikCw
> > an email to django-develop...@googlegroups.com <javascript:>
> > <mailto:django-develop...@googlegroups.com
> <javascript:>>.
> > <mailto:django-d...@googlegroups.com <javascript:>>.
> <https://groups.google.com/group/django-developers>.
> <https://groups.google.com/d/msgid/django-developers/2aa831a2-d435-49ba-9872-85422ea00e28%40googlegroups.com?utm_medium=email&utm_source=footer>.
don't really see a strong argument against keeping and documenting it.
As you say, cost is low.
Carl
> >
> <https://www.google.com/url?q=https%3A%2F%2Fgithub.com%2Fdjango%2Fdjango%2Fpull%2F6498%23issuecomment-268120711&sa=D&sntz=1&usg=AFQjCNH6pYlfIAgKUN59fcVGOTnZ7FikCw
> > <mailto:django-develop...@googlegroups.com
> <javascript:>>.
> > To post to this group, send email to django-d...@googlegroups.com
> <javascript:>
> > <mailto:django-d...@googlegroups.com <javascript:>>.
> <https://groups.google.com/group/django-developers>.
> > To view this discussion on the web visit
> >
> https://groups.google.com/d/msgid/django-developers/6e011e39-2e25-4d61-975f-866bf4a6a2a3%40googlegroups.com
> <https://groups.google.com/d/msgid/django-developers/6e011e39-2e25-4d61-975f-866bf4a6a2a3%40googlegroups.com>
>
> >
> <https://groups.google.com/d/msgid/django-developers/6e011e39-2e25-4d61-975f-866bf4a6a2a3%40googlegroups.com?utm_medium=email&utm_source=footer
> >
> https://groups.google.com/d/msgid/django-developers/6e011e39-2e25-4d61-975f-866bf4a6a2a3%40googlegroups.com
> <https://groups.google.com/d/msgid/django-developers/6e011e39-2e25-4d61-975f-866bf4a6a2a3%40googlegroups.com>
>
> >
> <https://groups.google.com/d/msgid/django-developers/6e011e39-2e25-4d61-975f-866bf4a6a2a3%40googlegroups.com?utm_medium=email&utm_source=footer>>.
>
> > For more options, visit https://groups.google.com/d/optout
> <https://groups.google.com/d/optout>.
>
> > For more options, visit https://groups.google.com/d/optout
>
> --
> 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
> an email to django-develop...@googlegroups.com
> <mailto:django-develop...@googlegroups.com>.
> --
> 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
> an email to django-develop...@googlegroups.com
> To post to this group, send email to django-d...@googlegroups.com
> <mailto:django-d...@googlegroups.com>.
> 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/2aa831a2-d435-49ba-9872-85422ea00e28%40googlegroups.com
> <mailto:django-d...@googlegroups.com>.
> 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/2aa831a2-d435-49ba-9872-85422ea00e28%40googlegroups.com?utm_medium=email&utm_source=footer>.
Tim Graham
Dec 20, 2016, 4:26:35 PM12/20/16
to Django developers (Contributions to Django itself)
Okay I removed 'POST_APP_DIRS' from the PR.
Next, we should discuss whether or not to make the ProjectTemplate renderer the default FORM_RENDERER in the startproject template. (For backwards compatibility, DjangoTemplateRenderer has to be the default in global_settings.py.)
Florian said:
the first question in IRC I (and everyone else in IRC) will have to answer for weeks will be:
I have a nice context_processor where I specify the theme (color) of my material design template, why is the variable not in the templates for widgets, after all it is in the same template directory.
I am already using django-sniplates via libraries in the TEMPLATE config, as migration step I want to reuse that for the widgets, why are they not getting picked up.
Carl said:
"the best we can do to address the support concern that may arise once people start to use custom widgets and wonder why their TEMPLATES config doesn't apply, is to clearly document that if you're creating custom widgets that need custom template features, you should upgrade to ProjectTemplateRenderer. (And we should update startproject to use ProjectTemplateRenderer, so this support concern is a temporary thing during the upgrade timeframe only.)"
I feel these concerns may be overstated. I think custom template widget rendering won't be high on the list of things that Django beginners are looking to do and different values betwen "setting default" and "startproject default" causes confusion too. If we do make the addition, I guess 'django.forms' would be added somewhere in INSTALLED_APPS also.
Finally, I'd like if the renderer names were less verbose. Proposal:
django.forms.renderers.DjangoTemplates, Jinja2, TemplateEngines.
Next, we should discuss whether or not to make the ProjectTemplate renderer the default FORM_RENDERER in the startproject template. (For backwards compatibility, DjangoTemplateRenderer has to be the default in global_settings.py.)
Florian said:
the first question in IRC I (and everyone else in IRC) will have to answer for weeks will be:
I have a nice context_processor where I specify the theme (color) of my material design template, why is the variable not in the templates for widgets, after all it is in the same template directory.
I am already using django-sniplates via libraries in the TEMPLATE config, as migration step I want to reuse that for the widgets, why are they not getting picked up.
Carl said:
"the best we can do to address the support concern that may arise once people start to use custom widgets and wonder why their TEMPLATES config doesn't apply, is to clearly document that if you're creating custom widgets that need custom template features, you should upgrade to ProjectTemplateRenderer. (And we should update startproject to use ProjectTemplateRenderer, so this support concern is a temporary thing during the upgrade timeframe only.)"
I feel these concerns may be overstated. I think custom template widget rendering won't be high on the list of things that Django beginners are looking to do and different values betwen "setting default" and "startproject default" causes confusion too. If we do make the addition, I guess 'django.forms' would be added somewhere in INSTALLED_APPS also.
Finally, I'd like if the renderer names were less verbose. Proposal:
django.forms.renderers.DjangoTemplates, Jinja2, TemplateEngines.
> > <mailto:django-developers+unsub...@googlegroups.com
> <javascript:>>.
> > To post to this group, send email to django-d...@googlegroups.com
> <javascript:>
> > <mailto:django-d...@googlegroups.com <javascript:>>.
> > Visit this group at
> https://groups.google.com/group/django-developers
> <https://groups.google.com/group/django-developers>.
> > To view this discussion on the web visit
> >
> https://groups.google.com/d/msgid/django-developers/6e011e39-2e25-4d61-975f-866bf4a6a2a3%40googlegroups.com
> <https://groups.google.com/d/msgid/django-developers/6e011e39-2e25-4d61-975f-866bf4a6a2a3%40googlegroups.com>
>
> >
> <https://groups.google.com/d/msgid/django-developers/6e011e39-2e25-4d61-975f-866bf4a6a2a3%40googlegroups.com?utm_medium=email&utm_source=footer
> <https://groups.google.com/d/msgid/django-developers/6e011e39-2e25-4d61-975f-866bf4a6a2a3%40googlegroups.com?utm_medium=email&utm_source=footer>>.
>
> > For more options, visit https://groups.google.com/d/optout
> <https://groups.google.com/d/optout>.
>
> --
> 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
> an email to django-develop...@googlegroups.com
> <mailto:django-developers+unsub...@googlegroups.com>.
Carl Meyer
Dec 20, 2016, 4:35:45 PM12/20/16
to django-d...@googlegroups.com
Personally, I'm totally fine with deferring this decision until we see
how the default DTL renderer plays out in practice, and whether it does
cause confusion with people trying to build custom widgets.
I do think it'd be good to add to the DjangoTemplates renderer docs,
noting specifically that if you are using this renderer your custom
widget templates will use a "stock" DTL engine and won't have access to
any template engine customizations in your TEMPLATES setting, and
recommend ProjectTemplateRenderer if you need the latter.
> Finally, I'd like if the renderer names were less verbose. Proposal:
> django.forms.renderers.DjangoTemplates, Jinja2, TemplateEngines.
TemplateEngines. It seems kinda generic and unclear: all of the
renderers use "template engines" in one way or another. What about
"ProjectTemplates"? "ConfiguredTemplates"?
Carl
Tim Graham
Dec 20, 2016, 5:04:07 PM12/20/16
to Django developers (Contributions to Django itself)
I'll try to enhance the docs as you suggested.
I think it would be nice to be able to look at the name of the "project" renderer and understand that it's connected to settings.TEMPLATES. I'm not sure if the term "project" does that well. Maybe "TemplatesSettingEngines"?
I think it would be nice to be able to look at the name of the "project" renderer and understand that it's connected to settings.TEMPLATES. I'm not sure if the term "project" does that well. Maybe "TemplatesSettingEngines"?
Carl Meyer
Dec 20, 2016, 5:19:32 PM12/20/16
to django-d...@googlegroups.com
On 12/20/2016 02:04 PM, Tim Graham wrote:
> I think it would be nice to be able to look at the name of the "project"
> renderer and understand that it's connected to settings.TEMPLATES. I'm
> not sure if the term "project" does that well. Maybe
> "TemplatesSettingEngines"?
since settings.TEMPLATES is the template configuration for your project.
I guess "TemplatesSettingEngines" could be OK, it just fails to roll off
the tongue. Not sure why we'd tack on "Engines" when we don't to any of
the other names (even though they all use a template engine or engines);
maybe just "django.forms.renderers.TemplatesSetting"?
I still slightly prefer "ProjectTemplates", but you're painting the
bikeshed, feel free to choose the color :-)
Carl
Tim Graham
Dec 20, 2016, 6:01:58 PM12/20/16
to Django developers (Contributions to Django itself)
TemplatesSetting seems okay to me (open to other consensus though).
PR is updated: https://github.com/django/django/pull/6498
PR is updated: https://github.com/django/django/pull/6498
Tim Graham
Dec 23, 2016, 1:26:19 PM12/23/16
to Django developers (Contributions to Django itself)
I noticed that the GIS widgets didn't use the new API so I added a commit for that. I tested the widgets in the geodjango tutorial and they looked okay. I'm not sure how well those widgets are tested in Django's test suite -- if any GIS users want to review the changes and/or test the branch with their own project, that would be welcome.
I also did a little polish on the documentation. If there's no further feedback, I think we could merge this sometime next week. If you would like to review it and don't have time until a certain day, just let me know and I'll delay the merge as needed.
Thanks!
Reply all
Reply to author
Forward
0 new messages