Proposal: Improvements for django.forms
petr.marhoun
I would like to propose some improvements for django.forms. But it is
seven quite independent proposals - one mail would be to long, seven
emails would too many emails. So I have created wiki page - is it a
good procedure?
The wiki page:
http://code.djangoproject.com/wiki/ImprovementsForDjangoForms
Petr Marhoun
Harro
thing :)
Now about the "Model fields with not-default values".
With 1.2 something like that landed to override the widget for fields
on ModelForms, but I agree that sometimes you need to change a label
or help text for a specific form instance. So that's a +1.
About the others; I'm not sure,
Russell Keith-Magee
> Hello,
>
> I would like to propose some improvements for django.forms. But it is
> seven quite independent proposals - one mail would be to long, seven
> emails would too many emails. So I have created wiki page - is it a
> good procedure?
It's not really ideal, because it makes the initial phases of much
harder. Starting a wiki page is a good idea when you have a single,
very complex idea, and you want to track the evolution of planning.
However, if you have lots of little ideas, you would be better served
with a single email so that we have something to hang the discussion
on.
That said, here's some initial feedback:
1. Fieldsets
This is effectively just exposing an Admin capability into general
forms; I'm +1 to this general idea.
One complication I can see is how to handle iteration. "for field in
form" currently assumes that all the fields are ungrouped. How does
this change when fieldsets are introduced? Does iterating over the
form give you fieldsets, which are themselves iterable? Do you need to
iterate over "for fieldset in form.fieldsets", with "for field in
form" iterating over the raw fields independently of the fieldsets?
I'm also not entirely clear how we can/should implement this with BaseForm.
So - more detail required, but generally +1 from me.
I would also point out that one of the acceptance criteria for this
proposal should be backporting the changes into Admin. Admin should be
the best example of us eating our own dogfood; however, the tools in
admin have a habit of evolving on their own a little bit too much. If
this give us an opportunity to do some of this housekeeping, I'd call
that a big win.
2. Inlines
I'm not sure I see the benefit of this. Why does inline processing
need to be crammed into the base form?
3. Better API for formsets
Formsets are complex beasts, to be sure. However, I'm extremely wary
of proposals that advocate widespread changes to existing API.
Backwards compatibility will be a *huge* issue here - however broken
the current implementation may be, we can't change the conditions
under which forms validate at present.
That said, I'm sure there are validation edge cases that can be
legitmately called bugs, and that aren't tested at the moment. If
cleaning up the formset API allows us to fix those edge cases, all the
better.
So - a tentative +0 to this idea, but most of the effort will be in
proving that you're fixing bugs, not breaking features.
4. Model fields with not-default values
Isn't this mostly what the new widget Meta argument does?
5. Parameters "template" and "attrs" for forms, formsets, fieldsets,
forms, (bound) fields and widgets
6. Support for (X)HTML5 and legacy HTML
I strongly suspect that these two may be covered by something I
discussed with Jacob at DjangoCon. Broadly, the proposal is to
deprecate the {{ field }} and {{ form }} approach to template
rendering in favor of a template tag {% render field %} and {% render
form %}; these template tags would allow you to customize the way that
individual fields, fieldsets and forms are rendered, including
controlling doctype compliance.
If you want to override the way fields or forms are rendered, you will
be able to subclass the base render template tag, override the
rendering behavior, and re-register the tag (or register it under a
different name).
I've been tinkering with this since DjangoCon.eu; I hope to be able to
present something once I surface from jetlag and the backlog of mail
and other tasks in my inbox.
7. Requests in forms
I suspect the answer on this one will be no for the simple reason of
backwards compatibility. A request attribute is only really useful if
it is ubiquitous, and there's no way to make it ubiquitous without
breaking backwards compatibility. Truth be told, having the request
around would have made CSRF a lot easier, but it wasn't so we had to
fall back on {% csrf_token %}.
This is also something that can be implemented in user-space by
subclassing a form and enforcing the request argument. So, call this a
-0 from me.
There's some brief feedback. My one additional request is to try not
to be too ambitious. You've proposed a lot of work here; frankly, if
you only got (1) finished for 1.3 I would consider it to be a major
win.
Yours,
Russ Magee %-)
Henrik Genssen
what about adding one more layer on top of forms:
a page object
why?
1) because it would be nice to have other widgets besides forms, too (e.g. a (ajax-)table (like in the admin views)
2) having a page object could ease the mess with media files from forms and the rest of a page (e.g. avoid double include of jquery in a html page)
3) a very old idea is to have a html page with no html in it - just code (so a template is only needed for special cases) and than having different renderer engines (as_table functions like) to produce html or something else like XUL for the same page / form
regards
Henrik
petr.marhoun
wrote:
> > Hello,
>
> > I would like to propose some improvements for django.forms. But it is
> > seven quite independent proposals - one mail would be to long, seven
> > emails would too many emails. So I have created wiki page - is it a
> > good procedure?
>
> It's not really ideal, because it makes the initial phases of much
> harder. Starting a wiki page is a good idea when you have a single,
> very complex idea, and you want to track the evolution of planning.
> However, if you have lots of little ideas, you would be better served
> with a single email so that we have something to hang the discussion
> on.
>
> That said, here's some initial feedback:
>
> 1. Fieldsets
>
> This is effectively just exposing an Admin capability into general
> forms; I'm +1 to this general idea.
>
> One complication I can see is how to handle iteration. "for field in
> form" currently assumes that all the fields are ungrouped. How does
> this change when fieldsets are introduced? Does iterating over the
> form give you fieldsets, which are themselves iterable? Do you need to
> iterate over "for fieldset in form.fieldsets", with "for field in
> form" iterating over the raw fields independently of the fieldsets?
>
> I'm also not entirely clear how we can/should implement this with BaseForm.
>
> So - more detail required, but generally +1 from me.
concatenation of fields from individual fieldsets. So methods
as_table, as_ul and as_li and "for field in form" would work as now.
But if you want to really use fieldsets, you have to iterate fieldsets
in your templates.
I think I am able to implement it - but the first step is to decide
what to implement.
> I would also point out that one of the acceptance criteria for this
> proposal should be backporting the changes into Admin. Admin should be
> the best example of us eating our own dogfood; however, the tools in
> admin have a habit of evolving on their own a little bit too much. If
> this give us an opportunity to do some of this housekeeping, I'd call
> that a big win.
(not only forms, but also for example pagination, ordering or
filtering) in too many places (model admin, helper methods and
classes, templates, template tags ), it could be encapsulated in one
class. But I do not think that it is possible to change without
breaking backward-compatibility. So for forms and fieldsets I would
propose only extract meta attributes of forms as class attributes of
model admin.
> 2. Inlines
>
> I'm not sure I see the benefit of this. Why does inline processing
> need to be crammed into the base form?
object's form is valid if all inlines are valid. And saving the
objects means to save all inlines.
Yes, it is possible to iterate all inlines and decide if they are
valid and then save them once by once - but I thing it is very common
activity and it should be in core forms. And what if I want to edit my
objects with general views (for example class model views which could
be in 1.3)? How should I tell the general view I want to edit also
related objects?
> 3. Better API for formsets
>
> Formsets are complex beasts, to be sure. However, I'm extremely wary
> of proposals that advocate widespread changes to existing API.
> Backwards compatibility will be a *huge* issue here - however broken
> the current implementation may be, we can't change the conditions
> under which forms validate at present.
>
> That said, I'm sure there are validation edge cases that can be
> legitmately called bugs, and that aren't tested at the moment. If
> cleaning up the formset API allows us to fix those edge cases, all the
> better.
>
> So - a tentative +0 to this idea, but most of the effort will be in
> proving that you're fixing bugs, not breaking features.
- cleaned_data - I think that this property is broken by design
because valid formsets can have invalid forms without cleaned_data (if
they should be deleted).
- deleted_forms - it is useful as it is.
- ordered_forms - it is useful as it is but it would be redundant if
there would be more general property working also with unordered
formsets.
And there is no API for the my main requirement: give me all filled
and valid forms and nothing more, possible ordered (if can_order is
True). I propose new property which give me these forms.
So my proposal in other words:
1. Simplify formset validation with full backward-compatibility (minus
bugs).
2. Add new property "cleaned_forms" which do the right thing.
3. Possible deprecate "ordered_forms" (because it is redundant).
4. Possible deprecate "cleaned_data" (because it is broken by design -
there is note in code: "# Maybe this should just go away?").
> 4. Model fields with not-default values
>
> Isn't this mostly what the new widget Meta argument does?
proposal is general for all keyword arguments. It is possible to add
more and more meta arguments (help_texts and labels and so on) - but
is it a good idea? And there could be some arguments specific for only
one type of fields, they need general method for setting of not-
default value.
formsets would have last argument requiest with default value None. So
current code will continue to work.
But I fully agree that it could be solved by subclass which would save
the request. But if it will be quite common for init methods to accept
request arguments, new generic views could set this argument.
Otherwise everybody who would like to use request in forms and generic
views has to subclass forms and generic views, they could not be
enhanced independently.
Carl Meyer
On Jun 3, 11:32 am, "petr.marhoun" <petr.marh...@gmail.com> wrote:
> OK, more details: If fieldsets would be defined, fields would be
> concatenation of fields from individual fieldsets. So methods
> as_table, as_ul and as_li and "for field in form" would work as now.
> But if you want to really use fieldsets, you have to iterate fieldsets
> in your templates.
around to it yet. I have an external app, django-form-utils[1], that
already implements form fieldsets in very much the way you propose
(although with a few differences). The app has seen some use and
people seem happy with it; I would be happy to port this
implementation to a core patch, with a few changes.
One thing form-utils does differently from your proposal is that it
defines each fieldset as a two-tuple (like admin fieldsets), with a
name as the first element and the dictionary of additional info as the
second element. One advantage of having a fieldset "name" is that it
also allows accessing fieldsets individually in the template, for more
fine-grained layout control (thanks Rob Hudson for adding this to form-
utils).
Just to give a full summary of the fieldsets API I would propose
(which is not exactly the same as what's currently in form-utils; I'd
like to correct some mistakes :> ):
class PersonForm(forms.Form):
name = forms.CharField()
age = forms.IntegerField()
nickname = forms.CharField()
class Meta:
fieldsets = [('main', {'fields': ['name', 'age']}),
('extra', {'fields': ['nickname'],
'legend': 'Extra info',
'classes': ['aside']})]
(If "fieldsets" is provided for a ModelForm, it takes priority over
any "fields" or "exclude" definitions: the fields included in the
fieldsets are the ones included in the form, whether you iterate over
fieldsets or just fields.)
Iterating over the form directly yields BoundFields just as it does
now:
{% for field in form %}
Iterating over form.fieldsets yields Fieldset objects. Fieldset
objects can be iterated over to yield BoundFields:
{% for fieldset in form.fieldsets %}
{% for field in fieldset %}
Fieldsets can also be accessed individually by name on form.fieldsets:
{% for field in form.fieldsets.main %}
And fields can be accessed directly by name on the "fields" attribute
of a Fieldset (to avoid clashes between form field names and other
Fieldset attribute names):
{% form.fieldsets.main.fields.age %}
Fieldset objects also have the attributes "name", "legend", and
"classes" (which is still an iterable, not collapsed to a string, so
template authors can check {% if "blah" in fieldset.classes %} if
needed.
I am proposing the "classes" option, again parallel to current admin
fieldsets, in lieu of any additional options like "attrs" or
"template". There's a slippery slope where almost any info _could_
theoretically be provided for the template author by the Python form
author; I think it makes sense to keep it to a simple list of class
names, as the most basic, generic way to distinguish "types" of
fieldsets. I think pretty much any use case can be handled via this
mechanism, since you can write custom template tags if needed to
provide reusable display logic. (I'd be open to the "attrs" option,
but am quite opposed to "template": form definitions seem like
entirely the wrong place to embed template names.)
I would of course assume that a patch for fieldsets in forms would
also involve porting the admin's fieldsets to use this feature.
FWIW, I think it would be much preferable to separate discussion of
orthogonal features into separate threads, even if they are all form-
related. Makes it much easier to follow the discussion.
Thanks!
Carl
[1] http://bitbucket.org/carljm/django-form-utils
Carl Meyer
wrote:
> forms, (bound) fields and widgets
> 6. Support for (X)HTML5 and legacy HTML
>
> I strongly suspect that these two may be covered by something I
> discussed with Jacob at DjangoCon. Broadly, the proposal is to
> deprecate the {{ field }} and {{ form }} approach to template
> rendering in favor of a template tag {% render field %} and {% render
> form %}; these template tags would allow you to customize the way that
> individual fields, fieldsets and forms are rendered, including
> controlling doctype compliance.
>
> If you want to override the way fields or forms are rendered, you will
> be able to subclass the base render template tag, override the
> rendering behavior, and re-register the tag (or register it under a
> different name).
on me about that markup he can't control), and I am happy to help make
it happen. Looking forward to seeing the results of your tinkering. If
those tags could optionally take a template name as argument, I think
that would cover the vast majority of customization use-cases without
any need for subclassing or re-registering the template tag.
Personally I'd love to also see the default widgets rendered by
templates, so a normal template override of a predictably-named
builtin template is all that's needed to customize the rendering of a
given widget. But there might be performance issues there if the
caching template loader isn't used, and that isn't enabled by default.
Carl
petr.marhoun
> One thing form-utils does differently from your proposal is that it
> defines each fieldset as a two-tuple (like admin fieldsets), with a
> name as the first element and the dictionary of additional info as the
> second element. One advantage of having a fieldset "name" is that it
> also allows accessing fieldsets individually in the template, for more
> fine-grained layout control (thanks Rob Hudson for adding this to form-
> utils).
>
> Just to give a full summary of the fieldsets API I would propose
> (which is not exactly the same as what's currently in form-utils; I'd
> like to correct some mistakes :> ):
>
> class PersonForm(forms.Form):
> name = forms.CharField()
> age = forms.IntegerField()
> nickname = forms.CharField()
>
> class Meta:
> fieldsets = [('main', {'fields': ['name', 'age']}),
> ('extra', {'fields': ['nickname'],
> 'legend': 'Extra info',
> 'classes': ['aside']})]
- but I like this proposal more.
I have some notes to forms-utils:
- The code says: "if legend is None: legend = name". I dislike it - I
want to have possibility not to set legend. I think that if legend is
not set it should be "None" in fieldset.
- It seems that it is not possible to say if form has fieldsets
(fieldsets method even construct fieldsets if meta attribute is not
set) - I think it is useful to know it in templates.
> (If "fieldsets" is provided for a ModelForm, it takes priority over
> any "fields" or "exclude" definitions: the fields included in the
> fieldsets are the ones included in the form, whether you iterate over
> fieldsets or just fields.)
"attrs", not "classes", so it is more consistent to use "attrs". And I
have proposed "attrs" in another proposal because I think that there
is general problem - how to customize parts of form rendering without
writing of whole templates? So I would start with items "fields" and
"legend", other items could be added later.
> I would of course assume that a patch for fieldsets in forms would
> also involve porting the admin's fieldsets to use this feature.
names. But in admin "fields" are list of items - and each item can be
field name or list of field names. So I think that generally it is not
possible to port admin's fieldsets to these proposals' fieldsets -
proposals' fieldsets are subset of admin's fieldsets. (And syntax from
admin seems to be too complex for Django core.)
I will try to implement the opposite approach I have proposed in
previous mail - extract fieldsets meta attribute of form to fieldset
attribute of model admin.
Carl Meyer
On Jun 3, 5:48 pm, "petr.marhoun" <petr.marh...@gmail.com> wrote:
> I have some notes to forms-utils:
> - The code says: "if legend is None: legend = name". I dislike it - I
> want to have possibility not to set legend. I think that if legend is
> not set it should be "None" in fieldset.
correct in the Django-core version :-) Note I didn't mention anything
about auto-setting legend in the proposal here.
> - It seems that it is not possible to say if form has fieldsets
> (fieldsets method even construct fieldsets if meta attribute is not
> set) - I think it is useful to know it in templates.
you can say {{ if form.fieldsets }}.
> Here I would prefer to be minimalistic. For example, widgets have
> "attrs", not "classes", so it is more consistent to use "attrs". And I
> have proposed "attrs" in another proposal because I think that there
> is general problem - how to customize parts of form rendering without
> writing of whole templates? So I would start with items "fields" and
> "legend", other items could be added later.
"classes" vs "attrs."
> It is not so simple. Our proposals are that "fields" are list of field
> names. But in admin "fields" are list of items - and each item can be
> field name or list of field names. So I think that generally it is not
> possible to port admin's fieldsets to these proposals' fieldsets -
> proposals' fieldsets are subset of admin's fieldsets. (And syntax from
> admin seems to be too complex for Django core.)
admin functionality on top of the core functionality as outlined
above, without duplicating code or bringing extra admin complexity
into core. It probably will require the admin code to have pretty
intimate knowledge of the core code, but that's nothing new :-)
Carl
Carl Meyer
> (fieldsets method even construct fieldsets if meta attribute is not
> set) - I think it is useful to know it in templates.
utils, if you define no fieldsets it builds a default one for you
automatically with all the fields in it, so len(form.fieldsets) is
always at least 1. The advantage is that this allows templates that
expect fieldsets to always work, but I'm not sure it's actually a good
idea. Open to thoughts.
Carl
Russell Keith-Magee
Admin contains all sorts of functionality; but one of the pieces of
functionality is a fieldset implementation. What I'm saying is that
rather than building *another* fieldset implementation, we should
either factor the admin implemetnation into the core forms library, or
we should replace the admin implementation with a new implementation
that is added to the core forms library.
I don't doubt that admin will require additional customization -- but
making sure that it is possible to add that customization is a core
part of your design task. If you can't provide a base implementation
that admin can subclass and extend to meet it's needs, then you
probably don't have a viable proposal for an API.
To my mind, the ultimate goal is that admin should be little more than
a relatively minor customization of the builtin form, formset and
generic view capabilities of Django core.
>> 2. Inlines
>>
>> I'm not sure I see the benefit of this. Why does inline processing
>> need to be crammed into the base form?
>
> Because I want to edit object including related objects. So the
> object's form is valid if all inlines are valid. And saving the
> objects means to save all inlines.
>
> Yes, it is possible to iterate all inlines and decide if they are
> valid and then save them once by once - but I thing it is very common
> activity and it should be in core forms. And what if I want to edit my
> objects with general views (for example class model views which could
> be in 1.3)? How should I tell the general view I want to edit also
> related objects?
Ok - so again; abstract out some of the utilities from admin to make
the validation and error combining process easier. I don't see any
reason why we need to try and cram inline objects into the base form
object; we just need better tools for asking the "if this object, or
any of it's related objects are invalid" type questions.
>> 3. Better API for formsets
>>
>> Formsets are complex beasts, to be sure. However, I'm extremely wary
>> of proposals that advocate widespread changes to existing API.
>> Backwards compatibility will be a *huge* issue here - however broken
>> the current implementation may be, we can't change the conditions
>> under which forms validate at present.
>>
>> That said, I'm sure there are validation edge cases that can be
>> legitmately called bugs, and that aren't tested at the moment. If
>> cleaning up the formset API allows us to fix those edge cases, all the
>> better.
>>
>> So - a tentative +0 to this idea, but most of the effort will be in
>> proving that you're fixing bugs, not breaking features.
>
> Current API for using of validated formsets is:
> - cleaned_data - I think that this property is broken by design
> because valid formsets can have invalid forms without cleaned_data (if
> they should be deleted).
> - deleted_forms - it is useful as it is.
> - ordered_forms - it is useful as it is but it would be redundant if
> there would be more general property working also with unordered
> formsets.
>
> And there is no API for the my main requirement: give me all filled
> and valid forms and nothing more, possible ordered (if can_order is
> True). I propose new property which give me these forms.
Like I said, a tentative +0, which is completely dependent on the
implementation demonstrating that it is backwards compatible. It's
very easy to *say* your approach will be backwards compatible -- take
it from personal experience that it's often much harder in reality.
As soon as you make the parameter optional, it ceases to be
ubiquitous, which significantly reduces it's usefulness. The only way
I can see around this is to introduce a "RequestForm", but I'm not
sure if I'm wild about the idea of complicating the decision about
which form base class you should use.
Yours,
Russ Magee %-)
petr.marhoun
>
> >> I suspect the answer on this one will be no for the simple reason of
> >> backwards compatibility. A request attribute is only really useful if
> >> it is ubiquitous, and there's no way to make it ubiquitous without
> >> breaking backwards compatibility. Truth be told, having the request
> >> around would have made CSRF a lot easier, but it wasn't so we had to
> >> fall back on {% csrf_token %}.
>
> >> This is also something that can be implemented in user-space by
> >> subclassing a form and enforcing the request argument. So, call this a
> >> -0 from me.
>
> > I think it should not be backward-incompatible - init of forms and
> > formsets would have last argument requiest with default value None. So
> > current code will continue to work.
>
> > But I fully agree that it could be solved by subclass which would save
> > the request. But if it will be quite common for init methods to accept
> > request arguments, new generic views could set this argument.
> > Otherwise everybody who would like to use request in forms and generic
> > views has to subclass forms and generic views, they could not be
> > enhanced independently.
>
> As soon as you make the parameter optional, it ceases to be
> ubiquitous, which significantly reduces it's usefulness. The only way
> I can see around this is to introduce a "RequestForm", but I'm not
> sure if I'm wild about the idea of complicating the decision about
> which form base class you should use.
in django.forms would be rejected.
1. Implement very simple RequestForm:
class RequestForm(object):
def __init__(self, *args, **kwargs):
self.request = kwargs.pop('request')
super(RequestForm, self).__init__(*args, **kwargs)
2. Check RequestForm in my general views:
if isinstance(form_class, RequestForm):
form = form_class(request.POST, request.FILES, request=request)
else:
form = form_class(request.POST, request.FILES)
3. Subclass RequestForm if request is needed:
class MyForm(RequestForm, forms.ModelForm):
def do_something(self):
if self.request.user.is_authenticated:
...
RequestForm is not a form (maybe better name is needed) so it is no
problem to use it for common forms and model forms.
Is any chance to accept something similar to Django? Otherwise authors
of reusable general views would not be able to check if they can set
request.
>
> Yours,
> Russ Magee %-)