Re: [Django] #5335: Add an append method to ErrorDict
Django
-------------------------------------+-------------------------------------
Reporter: Thomas | Owner: jkocherhans
Güttler <hv@…> | Status: new
Type: New | Component: Forms
feature | Severity: Normal
Milestone: 1.4 | Keywords: append errodict
Version: SVN | Has patch: 1
Resolution: | Needs tests: 0
Triage Stage: Accepted | Easy pickings: 0
Needs documentation: 0 |
Patch needs improvement: 1 |
UI/UX: 0 |
-------------------------------------+-------------------------------------
Changes (by julien):
* ui_ux: => 0
* easy: => 0
Comment:
Thanks a lot for updating the patch. I showed it to Alex and he wondered
if you could write a test that verifies that doing errors['foo'] without
actually altering it doesn't cause any problem?
--
Ticket URL: <https://code.djangoproject.com/ticket/5335#comment:23>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: Thomas | Owner: jkocherhans
Güttler <hv@…> | Status: new
Type: New | Component: Forms
feature | Severity: Normal
Milestone: 1.4 | Keywords: append errodict
Version: SVN | Has patch: 1
Resolution: | Needs tests: 0
Triage Stage: Accepted | Easy pickings: 0
Needs documentation: 0 |
Patch needs improvement: 1 |
UI/UX: 0 |
-------------------------------------+-------------------------------------
* cc: bronger (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/5335#comment:24>
Django
-------------------------------------+-------------------------------------
Reporter: Thomas | Owner: jkocherhans
Güttler <hv@…> | Status: new
Type: New | Component: Forms
feature | Severity: Normal
Milestone: 1.4 | Keywords: append errodict
Version: SVN | Has patch: 1
Resolution: | Needs tests: 0
Triage Stage: Accepted | Easy pickings: 0
Needs documentation: 0 |
Patch needs improvement: 1 |
UI/UX: 0 |
-------------------------------------+-------------------------------------
Comment (by kmtracey):
Current patch (form_error_append_r16741.diff) will auto-create an
`ErrorList()` object for a field's error list even if the including form
has over-ridden the form's error class
(https://docs.djangoproject.com/en/1.3/ref/forms/api/#customizing-the-
error-list-format). If we are going to add this (and it would be nice) it
needs to be done in a way that will use the form's custom error class if
it has been defined. This case should also be tested, in a test similar
to:
https://code.djangoproject.com/browser/django/tags/releases/1.3.1/tests/regressiontests/forms/tests/error_messages.py#L199
--
Ticket URL: <https://code.djangoproject.com/ticket/5335#comment:25>
Django
-------------------------------------+-------------------------------------
<hv@…> | jkocherhans
Type: New feature | Status: new
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: append errodict | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by anonymous):
Check [http://www.google.com]
--
Ticket URL: <https://code.djangoproject.com/ticket/5335#comment:27>
Django
-------------------------------------+-------------------------------------
Reporter: Thomas Güttler | Owner:
<hv@…> | jkocherhans
Type: New feature | Status: new
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: append errodict | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by loic84):
The problem is that `ErrorDict` should be `ErrorList` agnostic. The latest
patch hardcodes the `ErrorList` type which can be customized at the form
level with the `error_class` argument of `Form.__init__()` .
The `Form.add_errors()` method discussed
[https://code.djangoproject.com/ticket/20867#comment:4 here] is IMO a much
better approach.
Django
-------------------------------------+-------------------------------------
Reporter: Thomas Güttler | Owner:
<hv@…> | jkocherhans
Type: New feature | Status: new
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: append errodict | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by loic84):
Actually there is merit in making `ErrorDict` inherit from `defaultdict`.
PR https://github.com/django/django/pull/1481
--
Ticket URL: <https://code.djangoproject.com/ticket/5335#comment:28>
Django
-------------------------------------+-------------------------------------
Reporter: Thomas Güttler | Owner:
<hv@…> | jkocherhans
Type: New feature | Status: new
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: append errodict | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by alasdair):
Looping through defaultdicts in templates can cause problems. See #16335.
I believe this change will stop the following template snippet from
working.
{{{
<ul>
{% for field, errors in form.errors.items %}
<li>{{ field }}: {{errors}}
{% endford %}
</ul>
}}}
I realise it's a slightly artificial example, and that usually you would
use `{{ form.errors }}`.
--
Ticket URL: <https://code.djangoproject.com/ticket/5335#comment:29>
Django
-------------------------------------+-------------------------------------
Reporter: Thomas Güttler | Owner:
<hv@…> | jkocherhans
Type: New feature | Status: new
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: append errodict | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by simon29):
Check out the [https://groups.google.com/forum/#!topic/django-
developers/rTbfg3JtLkA related discussion] and also ticket #20867
--
Ticket URL: <https://code.djangoproject.com/ticket/5335#comment:30>
Django
-------------------------------------+-------------------------------------
Reporter: Thomas Güttler | Owner:
<hv@…> | jkocherhans
Type: New feature | Status: new
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: append errodict | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by loic84):
@alasdair: that looks like a nasty incompatibility with no obvious
workaround.
The only reason I fixed this despite #20867 was so internally it wouldn't
be needed to check if keys existed, it's clearly not worth breaking
existing templates over this.
I think we should close this ticket as wontfix, especially now that
another ticket addresses the main issue.
--
Ticket URL: <https://code.djangoproject.com/ticket/5335#comment:31>
Django
-------------------------------------+-------------------------------------
Reporter: Thomas Güttler | Owner:
<hv@…> | jkocherhans
Type: New feature | Status: new
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: append errodict | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: alasdair (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/5335#comment:32>
Django
-------------------------------------+-------------------------------------
Reporter: Thomas Güttler | Owner:
<hv@…> | jkocherhans
Component: Forms | Version: master
Severity: Normal | Resolution: duplicate
Keywords: append errodict | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => duplicate
Comment:
I agree that this is not the best way forwards. The ticket #20867 is now
covering the `Form.add_errors()` approach. I'm going to close this ticket
as a "duplicate" cos I think that's closest to what's happening.
--
Ticket URL: <https://code.djangoproject.com/ticket/5335#comment:33>