Re: Template-based widget rendering

2016-12-23 Thread Tim Graham
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!

https://github.com/django/django/pull/6498

On Tuesday, December 20, 2016 at 6:01:58 PM UTC-5, Tim Graham wrote:
>
> TemplatesSetting seems okay to me (open to other consensus though). 
>
> PR is updated: https://github.com/django/django/pull/6498
>
> On Tuesday, December 20, 2016 at 5:19:32 PM UTC-5, Carl Meyer wrote:
>>
>>
>> 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"? 
>>
>> Yeah... I guess I thought ProjectTemplates got reasonably close to that, 
>> 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 
>>
>>

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/e1dbc1f9-e9b4-4a25-b1e4-c33747d91530%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Template-based widget rendering

2016-12-20 Thread Tim Graham
TemplatesSetting seems okay to me (open to other consensus though). 

PR is updated: https://github.com/django/django/pull/6498

On Tuesday, December 20, 2016 at 5:19:32 PM UTC-5, Carl Meyer wrote:
>
>
> 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"? 
>
> Yeah... I guess I thought ProjectTemplates got reasonably close to that, 
> 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 
>
>

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/959261a9-1533-4ff0-8cf7-36b9fe691b64%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Template-based widget rendering

2016-12-20 Thread Carl Meyer

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"?

Yeah... I guess I thought ProjectTemplates got reasonably close to that,
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

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/f7b131b4-910a-ec72-a045-badd4cba74ad%40oddbird.net.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: OpenPGP digital signature


Re: Template-based widget rendering

2016-12-20 Thread Tim Graham
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"?

On Tuesday, December 20, 2016 at 4:35:45 PM UTC-5, Carl Meyer wrote:
>
> On 12/20/2016 01:26 PM, Tim Graham wrote: 
> > 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. 
>
> Yes, it would mean adding django.forms to INSTALLED_APPS as well. 
>
> 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. 
>
> +1. The first two names are fine with me, not sure about 
> TemplateEngines. It seems kinda generic and unclear: all of the 
> renderers use "template engines" in one way or another. What about 
> "ProjectTemplates"? "ConfiguredTemplates"? 
>
> 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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/71680f34-b22f-4812-8b82-bcf49489824d%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Template-based widget rendering

2016-12-20 Thread Carl Meyer
On 12/20/2016 01:26 PM, Tim Graham wrote:
> 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.

Yes, it would mean adding django.forms to INSTALLED_APPS as well.

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.

+1. The first two names are fine with me, not sure about
TemplateEngines. It seems kinda generic and unclear: all of the
renderers use "template engines" in one way or another. What about
"ProjectTemplates"? "ConfiguredTemplates"?

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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/5d2bed65-88cf-5ab8-4f75-cdc61ef2b7b6%40oddbird.net.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: OpenPGP digital signature


Re: Template-based widget rendering

2016-12-20 Thread Tim Graham
> 
> > > 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) 
> > > 
> > > [0] 
> > > 
> > https://github.com/django/django/pull/6498#issuecomment-268120711 
> > <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=D=1=AFQjCNH6pYlfIAgKUN59fcVGOTnZ7FikCw
>  
> > <
> https://www.google.com/url?q=https%3A%2F%2Fgithub.com%2Fdjango%2Fdjango%2Fpull%2F6498%23issuecomment-268120711=D=1=AFQjCNH6pYlfIAgKUN59fcVGOTnZ7FikCw>>
>  
>
> > 
> > > 
> > > On Monday, December 19, 2016 at 5:53:23 PM UTC-5, Tim Graham 
> > wrote: 
> > > 
> > > It's ready for review: 
> > > 
> > > Add 'POST_APP_DIRS' TEMPLATES option. 
> > > https://github.com/django/django/pull/7693 
> > <https://github.com/django/django/pull/7693> 
> > > <https://github.com/django/django/pull/7693 
> > <https://github.com/django/django/pull/7693>> 
> > > Template based widget rendering: 
> > > https://github.com/django/django/pull/6498 
> > <https://github.com/django/django/pull/6498> 
> > > <https://github.com/django/django/pull/6498 
> > <https://github.com/django/django/pull/6498>> 
> > > 
> > > On Wednesday, December 14, 2016 at 6:59:16 PM UTC-5, Tim 
> > Graham 
> > > wrote: 
> > > 
> > > 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. 
> > > 
> > > On Wednesday, December 14, 2016 at 6:40:36 PM UTC-5, 
> Carl 
> > > Meyer wrote: 
> > > 
> > > 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 
> > > > the app_directories loader eases setup but adds 
> a 
> > > cognitive overhead of 
> > > > figuring out where the template overrides are 
> > coming 
> > > from. 
> > > 
> > > All of these seem like generic objections to 
> > APP_DIRS / the 
> > > app_directories loader, which has been around in 
> some 
> > > form (and been the 
> > > default behavior) effectively forever. For better 
> or 
> > > worse, we have a 
> > > template system that layers multiple loaders in 
> > the same 
> > > template 
> > > namespace and allows them to stomp on each other, 
> and 
> > > leaves it up to 

Re: Template-based widget rendering

2016-12-20 Thread Carl Meyer
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

On 12/20/2016 11:55 AM, Tim Graham wrote:
> 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.
> 
> On Tuesday, December 20, 2016 at 2:26:33 PM UTC-5, Carl Meyer wrote:
> 
> 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
> 
> On 12/20/2016 11:16 AM, Florian Apolloner wrote:
> > 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
> >
> > On Tuesday, December 20, 2016 at 7:34:21 PM UTC+1, Tim Graham wrote:
> >
> > 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)
> >
> > [0]
> >
> https://github.com/django/django/pull/6498#issuecomment-268120711
> <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=D=1=AFQjCNH6pYlfIAgKUN59fcVGOTnZ7FikCw
> 
> <https://www.google.com/url?q=https%3A%2F%2Fgithub.com%2Fdjango%2Fdjango%2Fpull%2F6498%23issuecomment-268120711=D=1=AFQjCNH6pYlfIAgKUN59fcVGOTnZ7FikCw>>
> 
> >
> > On Monday, December 19, 2016 at 5:53:23 PM UTC-5, Tim Graham
> wrote:
> >
> > It's ready for review:
> >
> > Add 'POST_APP_DIRS' TEMPLATES option.
> > https://github.com/django/django/pull/7693
> <https://github.com/django/django/pull/7693>
> > <https://github.com/django/django/pull/7693
> <https://github.com/django/django/pull/7693>>
> > Template based widget rendering:
> > https://github.com/django/django/pull/6498
> <https://github.com/django/django/pull/6498>
> > <https://github.com/django/django/pull/6498
> <https://github.com/django/django/pull/6498>>
> >
> > On Wednesday, December 14, 2016 at 6:59:16 PM UTC-5, Tim
> Graham
> > wrote:
> >
> > I don'

Re: Template-based widget rendering

2016-12-20 Thread Tim Graham
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.

On Tuesday, December 20, 2016 at 2:26:33 PM UTC-5, Carl Meyer wrote:
>
> 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 
>
> On 12/20/2016 11:16 AM, Florian Apolloner wrote: 
> > 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 
> > 
> > On Tuesday, December 20, 2016 at 7:34:21 PM UTC+1, Tim Graham wrote: 
> > 
> > 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) 
> > 
> > [0] 
> > 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=D=1=AFQjCNH6pYlfIAgKUN59fcVGOTnZ7FikCw>
>  
>
> > 
> > On Monday, December 19, 2016 at 5:53:23 PM UTC-5, Tim Graham wrote: 
> > 
> > It's ready for review: 
> > 
> > Add 'POST_APP_DIRS' TEMPLATES option. 
> > https://github.com/django/django/pull/7693 
> > <https://github.com/django/django/pull/7693> 
> > Template based widget rendering: 
> > https://github.com/django/django/pull/6498 
> > <https://github.com/django/django/pull/6498> 
> > 
> > On Wednesday, December 14, 2016 at 6:59:16 PM UTC-5, Tim Graham 
> > wrote: 
> > 
> > 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. 
> > 
> > On Wednesday, December 14, 2016 at 6:40:36 PM UTC-5, Carl 
> > Meyer wrote: 
> > 
> > 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 
> &

Re: Template-based widget rendering

2016-12-20 Thread Carl Meyer
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

On 12/20/2016 11:16 AM, Florian Apolloner wrote:
> 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
> 
> On Tuesday, December 20, 2016 at 7:34:21 PM UTC+1, Tim Graham wrote:
> 
> 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)
> 
> [0]
> 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=D=1=AFQjCNH6pYlfIAgKUN59fcVGOTnZ7FikCw>
> 
> On Monday, December 19, 2016 at 5:53:23 PM UTC-5, Tim Graham wrote:
> 
> It's ready for review:
> 
> Add 'POST_APP_DIRS' TEMPLATES option.
> https://github.com/django/django/pull/7693
> <https://github.com/django/django/pull/7693>
> Template based widget rendering:
> https://github.com/django/django/pull/6498
> <https://github.com/django/django/pull/6498>
> 
> On Wednesday, December 14, 2016 at 6:59:16 PM UTC-5, Tim Graham
> wrote:
> 
> 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.
> 
> On Wednesday, December 14, 2016 at 6:40:36 PM UTC-5, Carl
> Meyer wrote:
> 
> 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
> > the app_directories loader eases setup but adds a
> cognitive overhead of
> > figuring out where the template overrides are coming
> from.
> 
> All of these seem like generic objections to APP_DIRS / the
> app_directories loader, which has been around in some
> form (and been the
> default behavior) effectively forever. For better or
> worse, we have a
> template system that layers multiple loaders in the same
> template
> namespace and allows them to stomp on each other, and
> leaves it up to
>  

Re: Template-based widget rendering

2016-12-20 Thread Florian Apolloner
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

On Tuesday, December 20, 2016 at 7:34:21 PM UTC+1, Tim Graham wrote:
>
> 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)
>
> [0] 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=D=1=AFQjCNH6pYlfIAgKUN59fcVGOTnZ7FikCw>
>
> On Monday, December 19, 2016 at 5:53:23 PM UTC-5, Tim Graham wrote:
>>
>> 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
>>
>> On Wednesday, December 14, 2016 at 6:59:16 PM UTC-5, Tim Graham wrote:
>>>
>>> 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.
>>>
>>> On Wednesday, December 14, 2016 at 6:40:36 PM UTC-5, Carl Meyer wrote:
>>>>
>>>> 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 
>>>> > the app_directories loader eases setup but adds a cognitive overhead 
>>>> of 
>>>> > figuring out where the template overrides are coming from. 
>>>>
>>>> All of these seem like generic objections to APP_DIRS / the 
>>>> app_directories loader, which has been around in some form (and been 
>>>> the 
>>>> default behavior) effectively forever. For better or worse, we have a 
>>>> template system that layers multiple loaders in the same template 
>>>> namespace and allows them to stomp on each other, and leaves it up to 
>>>> you to figure it out if they do. This does give people some rope to 
>>>> create confusion, but it's also powerful and flexible, and so far we've 
>>>> decided that tradeoff is worth it. How is any of this specific to form 
>>>> templates? 
>>>>
>>>> > 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 de

Re: Template-based widget rendering

2016-12-19 Thread Tim Graham
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

On Wednesday, December 14, 2016 at 6:59:16 PM UTC-5, Tim Graham wrote:
>
> 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.
>
> On Wednesday, December 14, 2016 at 6:40:36 PM UTC-5, Carl Meyer wrote:
>>
>> 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 
>> > the app_directories loader eases setup but adds a cognitive overhead of 
>> > figuring out where the template overrides are coming from. 
>>
>> All of these seem like generic objections to APP_DIRS / the 
>> app_directories loader, which has been around in some form (and been the 
>> default behavior) effectively forever. For better or worse, we have a 
>> template system that layers multiple loaders in the same template 
>> namespace and allows them to stomp on each other, and leaves it up to 
>> you to figure it out if they do. This does give people some rope to 
>> create confusion, but it's also powerful and flexible, and so far we've 
>> decided that tradeoff is worth it. How is any of this specific to form 
>> templates? 
>>
>> > 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 
>>
>> > 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': 'djangoform

Re: Template-based widget rendering

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

On Wednesday, December 14, 2016 at 6:40:36 PM UTC-5, Carl Meyer wrote:
>
> 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 
> > the app_directories loader eases setup but adds a cognitive overhead of 
> > figuring out where the template overrides are coming from. 
>
> All of these seem like generic objections to APP_DIRS / the 
> app_directories loader, which has been around in some form (and been the 
> default behavior) effectively forever. For better or worse, we have a 
> template system that layers multiple loaders in the same template 
> namespace and allows them to stomp on each other, and leaves it up to 
> you to figure it out if they do. This does give people some rope to 
> create confusion, but it's also powerful and flexible, and so far we've 
> decided that tradeoff is worth it. How is any of this specific to form 
> templates? 
>
> > 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 
>
> > 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. 
>
>
>

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 

Re: Template-based widget rendering

2016-12-14 Thread Carl Meyer
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
> the app_directories loader eases setup but adds a cognitive overhead of
> figuring out where the template overrides are coming from.

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

> 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

> 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.


-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/cac98bf1-4f8c-be8a-8f7b-1ff95d627c24%40oddbird.net.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: OpenPGP digital signature


Re: Template-based widget rendering

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

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

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

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

On Wednesday, December 14, 2016 at 2:27:20 PM UTC-5, Carl Meyer wrote:
>
> 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 
> 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 
>
>

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/7eb0fe16-e926-449e-b747-63fd5b683201%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Template-based widget rendering

2016-12-14 Thread Carl Meyer
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
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

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/77412aeb-5c89-d62a-10e5-dbe1c5b604fc%40oddbird.net.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: OpenPGP digital signature


Re: Template-based widget rendering

2016-12-14 Thread Carl Meyer
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
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

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/f64544f7-ce0e-f58a-da5d-7ffe709d38d1%40oddbird.net.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: OpenPGP digital signature


Re: Template-based widget rendering

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

On Wednesday, December 7, 2016 at 12:00:57 PM UTC-5, Carl Meyer wrote:
>
> 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 
>
>

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/a0231f5f-5ce4-449e-bab8-2edf7bf05b05%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Template-based widget rendering

2016-12-07 Thread Carl Meyer
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

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/6768a102-edc7-ae69-ceec-32aab4beedbb%40oddbird.net.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: OpenPGP digital signature


Re: Template-based widget rendering

2016-12-07 Thread Tim Graham
This scheme seems to be working well so far.

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

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

On Saturday, December 3, 2016 at 12:41:13 PM UTC-5, Preston Timmons wrote:
>
> 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
>

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/6fd84f7b-3f8a-474d-bc9f-50919d21b5dd%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Template-based widget rendering

2016-12-03 Thread Preston Timmons
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

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/952f690d-bfc9-4aba-ba46-0501fd7f27bb%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Template-based widget rendering

2016-12-02 Thread Carl Meyer
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

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/7b4d51f5-6f48-acb5-a3bf-802cf7a8ea1f%40oddbird.net.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: OpenPGP digital signature


Re: Template-based widget rendering

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

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

On Wednesday, May 18, 2016 at 2:49:52 AM UTC-4, Carl Meyer wrote:
>
> 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 
> 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 
>
>
>
>

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/ed0c891a-33f9-4b28-80aa-7a90f2d097e8%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Template-based widget rendering

2016-05-18 Thread Carl Meyer
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
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



-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/573C1083.8030304%40oddbird.net.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: OpenPGP digital signature


Re: Template-based widget rendering

2016-05-18 Thread Carl Meyer
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

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/573C0FF5.5040801%40oddbird.net.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: OpenPGP digital signature


Re: Template-based widget rendering

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

On Tuesday, May 17, 2016 at 9:28:59 AM UTC-4, 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',
> ]
> },
> },
> }
>
> Simon
>
> Le samedi 14 mai 2016 10:03:57 UTC-4, Tim Graham a écrit :
>>
>> 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?
>>
>> On Thursday, May 12, 2016 at 1:17:23 PM UTC-4, Carl Meyer wrote:
>>>
>>> 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. 
>>>
>>> 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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/35252aa9-ae85-4c5f-9718-58da4530dbad%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Template-based widget rendering

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

Something along theese lines make more sense to me:

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

Simon

Le samedi 14 mai 2016 10:03:57 UTC-4, Tim Graham a écrit :
>
> 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?
>
> On Thursday, May 12, 2016 at 1:17:23 PM UTC-4, Carl Meyer wrote:
>>
>> 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. 
>>
>> 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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/b9de7eac-9412-482f-9e79-809d1293f6eb%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Template-based widget rendering

2016-05-17 Thread Claude Paroz


Le mardi 17 mai 2016 08:03:52 UTC+2, Carl Meyer a écrit :
>
> On 05/16/2016 01:48 AM, Claude Paroz wrote: 
>
> > I'm still answering with my naive hat: isn't it possible to simply 
> > always consider forms in django.forms without requiring anything new in 
> > INSTALLED_APPS? What would be the problem? (Sorry if I missed a 
> > discussion about that). 
>
> This is certainly possible, it's just (as Tim says) not clear where 
> exactly one would implement it without introducing an ugly bidirectional 
> coupling between forms and templates. It's one thing for form rendering 
> to now depend on templates, but for template engines (which currently 
> are configured explicitly and only know about the template directories 
> you tell them about) to also automatically and unconditionally know 
> about django.forms (even if you maybe aren't using forms at all -- maybe 
> you're using the template engine standalone for something else entirely) 
> doesn't seem right at all. 
>

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

Claude 

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/f30d6a06-7139-4ca0-bb9b-7714d50771a7%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Template-based widget rendering

2016-05-17 Thread Carl Meyer
On 05/16/2016 01:48 AM, Claude Paroz wrote:
> 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).

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

Carl

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/573AB437.6020607%40oddbird.net.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: OpenPGP digital signature


Re: Template-based widget rendering

2016-05-16 Thread Carl Meyer
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?

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

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/57394559.70508%40gmail.com.
For more options, visit https://groups.google.com/d/optout.


Re: Template-based widget rendering

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

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

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

On Monday, May 16, 2016 at 12:18:44 PM UTC-4, Claude Paroz wrote:
>
> ...always consider templates (not forms, sorry ) in django.forms ...
>

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/ba5cea4b-8d15-49ca-bc71-ea040a4653e7%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Template-based widget rendering

2016-05-16 Thread Claude Paroz
...always consider templates (not forms, sorry ) in django.forms ...

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/9c69034d-c12e-4c05-a7ea-e78fd52a6b88%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Template-based widget rendering

2016-05-16 Thread Claude Paroz
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

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/65f588d1-fd03-4bb4-aad5-733e680b5541%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Template-based widget rendering

2016-05-15 Thread Curtis Maloney

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 
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.)


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


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


--
Curtis

--
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/573958C0.6070102%40tinbrain.net.
For more options, visit https://groups.google.com/d/optout.


Re: Template-based widget rendering

2016-05-15 Thread Carl Meyer
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.

(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

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/5739564A.6010207%40oddbird.net.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: OpenPGP digital signature


Re: Template-based widget rendering

2016-05-15 Thread Curtis Maloney

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...

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__()`.


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


--
Curtis

--
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/5739541C.9040301%40tinbrain.net.
For more options, visit https://groups.google.com/d/optout.


Re: Template-based widget rendering

2016-05-15 Thread Carl Meyer
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

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/5739528E.3010909%40oddbird.net.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: OpenPGP digital signature


Re: Template-based widget rendering

2016-05-14 Thread Tim Graham
While testing this, I ran into a shortcoming with the fallback strategy for 
backwards-compatibility. 

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

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

On Thursday, May 12, 2016 at 1:17:23 PM UTC-4, Carl Meyer wrote:
>
> 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. 
>
> 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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/2fa7c726-3549-4a02-a820-0aae79802a2c%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Template-based widget rendering

2016-05-12 Thread Carl Meyer
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.

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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/5734BA8F.4000506%40oddbird.net.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: OpenPGP digital signature


Re: Template-based widget rendering

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

On Wednesday, May 11, 2016 at 2:08:31 PM UTC-4, Carl Meyer wrote:
>
> 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 
> 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 
>
>

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/4c27261f-5c08-4ac6-a6e3-4128a49f52d9%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Template-based widget rendering

2016-05-11 Thread Carl Meyer
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
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

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/57337508.7060708%40oddbird.net.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: OpenPGP digital signature


Re: Template-based widget rendering

2016-05-11 Thread Carl Meyer
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.
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?

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.

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

Ok, will do.

Carl

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/57337178.5040003%40oddbird.net.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: OpenPGP digital signature


Re: Template-based widget rendering

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

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

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

On Tuesday, May 10, 2016 at 2:22:14 PM UTC-4, Carl Meyer wrote:
>
> 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 
>
>

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/18c63161-f3de-44d5-aa4e-c91e3b63ec8b%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Template-based widget rendering

2016-05-11 Thread Preston Timmons
Hey Curtis,

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

For example, compare:

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

to the template version:

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

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

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

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

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

Preston



On Tuesday, May 10, 2016 at 10:32:30 PM UTC-5, Curtis Maloney wrote:
>
> 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 
>

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/856af7b0-9d1e-485b-ac57-e37f4d4cee04%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Template-based widget rendering

2016-05-10 Thread Curtis Maloney
Sorry for the late entry to the discussion, but I was looking over the 
code and wondered about something.


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


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


Is this correct?

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


--
Curtis

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-developers+unsubscr...@googlegroups.com
.
To post to this group, send email to django-developers@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
.
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/5732A7B7.7090403%40tinbrain.net.
For more options, visit https://groups.google.com/d/optout.


Re: Template-based widget rendering

2016-05-10 Thread Preston Timmons
+1. I like the simpler fallback solution Carl has suggested also.

Preston

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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.
For more options, visit https://groups.google.com/d/optout.


Re: Template-based widget rendering

2016-05-10 Thread Carl Meyer
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

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/573226C2.9050807%40oddbird.net.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: OpenPGP digital signature


Re: Template-based widget rendering

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

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

On Tuesday, May 10, 2016 at 2:35:50 AM UTC-4, Aymeric Augustin wrote:
>
> I agree with Carl’s arguments about the fallback strategy (specifically, 
> the four paragraphs quoted below). 
>
> -- 
> Aymeric. 
>
> > On 10 May 2016, at 00:28, Carl Meyer  
> wrote: 
> > 
> > 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. 
>
>

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/8e52e3c9-f7dd-4b78-9ba8-66fcc0a7f9e4%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Template-based widget rendering

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

-- 
Aymeric.

> On 10 May 2016, at 00:28, Carl Meyer  wrote:
> 
> 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.

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/EFF3F90A-F111-4E1E-94FF-654008BBDD0A%40polytechnique.org.
For more options, visit https://groups.google.com/d/optout.


Re: Template-based widget rendering

2016-05-10 Thread Claude Paroz
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

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/2176e758-864e-4d25-bb08-e51d17d4664d%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Template-based widget rendering

2016-05-09 Thread Carl Meyer
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'.

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

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/57310F15.2060401%40oddbird.net.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: OpenPGP digital signature


Re: Template-based widget rendering

2016-05-09 Thread Tim Graham
I've been working on testing and writing documentation for Preston's PR: 
https://github.com/django/django/pull/6498

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

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

On Wednesday, June 10, 2015 at 6:14:11 PM UTC-4, Preston Timmons wrote:
>
> 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
>

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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/378b1cbd-e2a2-4ee9-b088-29c21f9cfd61%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Template-based widget rendering

2015-06-10 Thread Preston Timmons
Hi Carl,

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

I'll update and convert this to a PR.

Preston

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/7e2e0274-aea5-42c0-92ba-f4b54346dd6c%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Template-based widget rendering

2015-06-10 Thread Carl Meyer
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 WI

Template-based widget rendering

2015-06-10 Thread Preston Timmons
Hi all,

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

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

This problem has two sides:

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

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

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

In addition, a WIP branch is available here:

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

Hopefully, we can finally get this in. :)

Preston

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/c0ea2593-05c2-4307-86ea-1b34c05d0f7a%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Proposal: template-based widget rendering

2011-03-23 Thread Carl Meyer
Hi Bruno,

On 03/23/2011 08:56 AM, Bruno Renié wrote:
> I'm not sure, however, how django's backwards-compatibility applies in
> this case. The widgets API has never been publicly documented but the
> official documentation encourages people to look at the code and
> create their own widgets based on it. Is it fine to touch some
> internal methods like build_attrs()? There will be some minor
> differences in the rendered code, with things like the order of
> attributes (name='foo' id='bar' versus id='bar' name='foo') and
> linebreaks (I have an EOL at the end of every template so each widget
> gets a "\n" at its end).

Aesthetic differences in the rendered HTML (like attribute order or
linebreaks) is not a backwards-compatibility issue, IMO - though the
default templates should still be functionally identical to the current
output.

I think widgets should maintain the same external API (i.e. the way they
are used by Django's forms code should not change), or else follow the
normal deprecation path for any well-justified changes there. As far as
I can tell from a quick scan, the external API consists only of the
signatures of the __init__() and render() methods, and the .attrs
attribute (which is tweaked directly by BoundField). Any methods or
attributes that are a) not documented, and b) not used by Django's form
code outside of widgets.py, are IMO fair game for change (though
obviously still only with good reason).

The area where this is most likely to break user code is if people have
subclassed some of the more complex widgets and are depending on
internal implementation details in their subclass. My feeling here is
that this is just like any other use of an internal API; all bets are
off. If it turns out that this breaks a lot of code, the other option
would be to introduce the new widgets in a new namespace, make them the
default ones, and leave the old ones alone. I'd really like to not do
this unless we absolutely have to.

> Currently the base `Widget` class has a render() method that raises
> NotImplemented. I think the rendering logic should happen here, the
> base Widget class should accept a template_name attribute and a
> render() method that renders the template. This way there is (almost)
> no need to touch any widget's render() method since template_name and
> get_context_data provide the needed extension points.

This sounds reasonable to me.

> I'll post some updates here and on the ticket, in the meantime I'm
> open to comments and suggestions.

Great! Thanks for working on this.

Carl

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Proposal: template-based widget rendering

2011-03-23 Thread Bruno Renié
Hi Carl,

On Tue, Mar 15, 2011 at 6:59 AM, Carl Meyer  wrote:
>
> As we've already discussed here at PyCon, I'm +1 on this change. It
> makes forms far more flexible and usable by template authors, and I
> think that will benefit almost all Django users. It's more consistent
> with Django's philosophy that presentation issues should be accessible
> to template designers who are not necessarily Python programmers.
>
> There is the speed tradeoff, and I sure hate to do anything that we know
> is slower. But I feel like this is a case of "do it right, then make it
> fast" - and I just think this is clearly the right way to do it.
> Probably the right way to make it fast is to make the template engine
> fast (Alex Gaynor had some interesting proposals for that for last
> year's GSoC, though it got dropped in favor of the no-SQL stuff). In
> absolute terms, I think the speed difference is already small enough
> that only a small fraction of users will be impacted by it, and they
> would always have the option to replace the default widgets in their
> forms with their own faster versions.
>
> The auto-escaping is another significant advantage - it's quite easy to
> forget to escape something that should be escaped in a custom widget (we
> had such a bug in Django itself until the latest security release), and
> I think it can only be good for security to discourage people ever
> building HTML strings in Python code.
>
> I already use floppyforms on all projects, and don't have any issues
> with the widget API it provides.
>
>> This could be part of a broader change related to form rendering (e.g.
>> fieldsets, looping over fields, etc) but I haven't thought about this
>> enough to have a concrete proposal. For more information, see that
>> discussion that happened at Djangocon:
>>
>> https://github.com/SmileyChris/django-forms/wiki/Django-form-API-ideas
>
> Although these proposals are related, and will involve some similar
> changes (i.e. the need to be able to load default form templates), I
> think the broader questions about form-rendering tags still need more
> work, and converting the built-in widgets to use templates doesn't need
> to be delayed while we hammer all of that out.

With 1.3 out (yay!), I just created a ticket to track the progress on this:

http://code.djangoproject.com/ticket/15667

I'm working on a patch, I'll attach it to the ticket as soon as I'm
happy with the widgets code and the regression tests pass again.

I'm not sure, however, how django's backwards-compatibility applies in
this case. The widgets API has never been publicly documented but the
official documentation encourages people to look at the code and
create their own widgets based on it. Is it fine to touch some
internal methods like build_attrs()? There will be some minor
differences in the rendered code, with things like the order of
attributes (name='foo' id='bar' versus id='bar' name='foo') and
linebreaks (I have an EOL at the end of every template so each widget
gets a "\n" at its end).

Currently the base `Widget` class has a render() method that raises
NotImplemented. I think the rendering logic should happen here, the
base Widget class should accept a template_name attribute and a
render() method that renders the template. This way there is (almost)
no need to touch any widget's render() method since template_name and
get_context_data provide the needed extension points.

I'll post some updates here and on the ticket, in the meantime I'm
open to comments and suggestions.

Regards,
Bruno

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Proposal: template-based widget rendering

2011-03-15 Thread Carl Meyer
Hi Bruno,

On 03/14/2011 06:33 PM, Bruno Renié wrote:
> Although Django 1.3 is not released yet I'd like to take advantage of
> the pycon sprints to discuss a proposal for 1.4: render form widgets
> using Django templates instead of python code.
> 
> This approach is implemented in django-floppyforms [0] (I'm the
> author): each widget gets a template_name argument, and the render
> method of each widget renders this template. Floppyforms also provides
> hooks to inject data in the template context, making it quite easy to
> implement custom widgets: add some context data, define a custom
> template and you're done.
> 
> This approach has a couple of advantages and drawbacks.
> 
> Advantages:
> 
>  - Templates give you escaping for free, giving us better protection against 
> XSS
>  - Template authors can alter widget rendering more easily
>  - This would also fix the "how do I specify my own doctype?" issue:
> Django can provide XHTML-style  elements and people can switch
> to html4-style s by providing their own set of templates.
> 
> Drawbacks:
> 
>  - Template rendering is slower than string interpolation. There is a
> benchmark in the floppyforms source tree, for a simple form the
> rendering takes 9 milliseconds instead of 1.5 milliseconds. With
> template caching enabled, it drops to 3 milliseconds. It still takes
> no time but I can see it being an issue.
>  - We need to add a template loader to provide the default form
> templates without asking the users to add 'django.forms' to their
> INSTALLED_APPS.
> 
> Backwards-compatibility is not going to be a big issue: the default
> templates can implement django's current behaviour.
> 
> We've been discussing it with Jannis and Carl and I'm trying to start
> a broader discussion. A side-effect of such a change would be to make
> the widgets API public and provide hooks for customization. In
> floppyforms I've been trying to follow the same conventions as the
> class-based views, using template_name and get_context_data as
> extension points.
> 
> I'm happy to work on the patch and convert the admin widgets but I'd
> like to hear people's voices about:
> 
> 1) If such a change is desired, and how we can make it faster,
> 2) how the API should differ from floppyforms,
> 3) make sure no significant use case is forgotten.

As we've already discussed here at PyCon, I'm +1 on this change. It
makes forms far more flexible and usable by template authors, and I
think that will benefit almost all Django users. It's more consistent
with Django's philosophy that presentation issues should be accessible
to template designers who are not necessarily Python programmers.

There is the speed tradeoff, and I sure hate to do anything that we know
is slower. But I feel like this is a case of "do it right, then make it
fast" - and I just think this is clearly the right way to do it.
Probably the right way to make it fast is to make the template engine
fast (Alex Gaynor had some interesting proposals for that for last
year's GSoC, though it got dropped in favor of the no-SQL stuff). In
absolute terms, I think the speed difference is already small enough
that only a small fraction of users will be impacted by it, and they
would always have the option to replace the default widgets in their
forms with their own faster versions.

The auto-escaping is another significant advantage - it's quite easy to
forget to escape something that should be escaped in a custom widget (we
had such a bug in Django itself until the latest security release), and
I think it can only be good for security to discourage people ever
building HTML strings in Python code.

I already use floppyforms on all projects, and don't have any issues
with the widget API it provides.

> This could be part of a broader change related to form rendering (e.g.
> fieldsets, looping over fields, etc) but I haven't thought about this
> enough to have a concrete proposal. For more information, see that
> discussion that happened at Djangocon:
> 
> https://github.com/SmileyChris/django-forms/wiki/Django-form-API-ideas

Although these proposals are related, and will involve some similar
changes (i.e. the need to be able to load default form templates), I
think the broader questions about form-rendering tags still need more
work, and converting the built-in widgets to use templates doesn't need
to be delayed while we hammer all of that out.

Carl

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Proposal: template-based widget rendering

2011-03-14 Thread Bruno Renié
Hi django devs,

Although Django 1.3 is not released yet I'd like to take advantage of
the pycon sprints to discuss a proposal for 1.4: render form widgets
using Django templates instead of python code.

This approach is implemented in django-floppyforms [0] (I'm the
author): each widget gets a template_name argument, and the render
method of each widget renders this template. Floppyforms also provides
hooks to inject data in the template context, making it quite easy to
implement custom widgets: add some context data, define a custom
template and you're done.

This approach has a couple of advantages and drawbacks.

Advantages:

 - Templates give you escaping for free, giving us better protection against XSS
 - Template authors can alter widget rendering more easily
 - This would also fix the "how do I specify my own doctype?" issue:
Django can provide XHTML-style  elements and people can switch
to html4-style s by providing their own set of templates.

Drawbacks:

 - Template rendering is slower than string interpolation. There is a
benchmark in the floppyforms source tree, for a simple form the
rendering takes 9 milliseconds instead of 1.5 milliseconds. With
template caching enabled, it drops to 3 milliseconds. It still takes
no time but I can see it being an issue.
 - We need to add a template loader to provide the default form
templates without asking the users to add 'django.forms' to their
INSTALLED_APPS.

Backwards-compatibility is not going to be a big issue: the default
templates can implement django's current behaviour.

We've been discussing it with Jannis and Carl and I'm trying to start
a broader discussion. A side-effect of such a change would be to make
the widgets API public and provide hooks for customization. In
floppyforms I've been trying to follow the same conventions as the
class-based views, using template_name and get_context_data as
extension points.

I'm happy to work on the patch and convert the admin widgets but I'd
like to hear people's voices about:

1) If such a change is desired, and how we can make it faster,
2) how the API should differ from floppyforms,
3) make sure no significant use case is forgotten.

This could be part of a broader change related to form rendering (e.g.
fieldsets, looping over fields, etc) but I haven't thought about this
enough to have a concrete proposal. For more information, see that
discussion that happened at Djangocon:

https://github.com/SmileyChris/django-forms/wiki/Django-form-API-ideas

Regards,
Bruno

[0] https://github.com/brutasse/django-floppyforms

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.