Opened 9 years ago

Closed 17 months ago

Last modified 17 months ago

#4960 closed New feature (fixed)

Add "strip" keyword argument to CharField constructor

Reported by: tzellman@… Owned by: Daniel Ward
Component: Forms Version: master
Severity: Normal Keywords: CharField strip
Cc: hv@…, marti@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I am suggesting that a strip keyword (or something similar) gets added to the CharField constructor in newforms/fields.py.

This would allow for certain fields to get stripped before being cleaned.
Example: Assume you have a CharField with min_length set to 3. Typing in 3 spaces will be valid, which in some cases is fine. But other times, you want to strip the whitespace. This way, a field filled with spaces will be considered to be empty, if the "strip" argument is True.

This is only a suggestion, but it would be nice to have. I know that I could easily sub-class and create my own "StrippedCharField", and that might be the right way to go, but I thought this might be a common issue for other people as well.

Thanks!

Attachments (3)

CharField_strip_rev5753.diff.txt (888 bytes) - added by tzellman@… 9 years ago.
Patch containing example addition of "strip" keyword to CharField
after.patch (1.3 KB) - added by Daniel Ward 18 months ago.
Follow-up tests
potential-solution.patch (1.1 KB) - added by Daniel Ward 18 months ago.
Potential solution by adding in 'strip' option to CharField

Download all attachments as: .zip

Change History (20)

Changed 9 years ago by tzellman@…

Patch containing example addition of "strip" keyword to CharField

comment:1 Changed 9 years ago by tzellman@…

Attached is another alternative, as I mentioned:

class StrippedCharField(forms.CharField):
    def __init__(self, max_length=None, min_length=None, strip=True, *args, **kwargs):
        super(StrippedCharField, self).__init__(max_length, min_length, *args, **kwargs)
        self.strip = strip

    def clean(self, value):
        if self.strip:
            value = value.strip()
        return super(StrippedCharField, self).clean(value)

comment:2 Changed 9 years ago by Simon G. <dev@…>

Triage Stage: UnreviewedDesign decision needed

comment:3 Changed 9 years ago by Adrian Holovaty

Resolution: wontfix
Status: newclosed

Marking as wontfix -- it's too specialized for inclusion in the main library. A bunch of us in #django-sprint agreed.

comment:4 Changed 9 years ago by Thomas Güttler

Cc: hv@… added

comment:5 Changed 4 years ago by Marti

Cc: marti@… added
Easy pickings: unset
Severity: Normal
Type: Uncategorized
UI/UX: unset

comment:6 Changed 20 months ago by Tim Graham

Patch needs improvement: set
Triage Stage: Design decision neededAccepted
Type: UncategorizedNew feature

comment:7 Changed 20 months ago by Tim Graham

Resolution: wontfix
Status: closednew

comment:8 Changed 18 months ago by Tim Graham

Patch needs improvement: unset

comment:9 Changed 18 months ago by Tim Graham

Patch needs improvement: set

comment:10 Changed 18 months ago by Daniel Ward

Owner: changed from nobody to Daniel Ward
Patch needs improvement: unset
Status: newassigned

Uploading patches with potential solution and follow-up tests. Public documentation patches can be produced if solution accepted.

Changed 18 months ago by Daniel Ward

Attachment: after.patch added

Follow-up tests

Changed 18 months ago by Daniel Ward

Attachment: potential-solution.patch added

Potential solution by adding in 'strip' option to CharField

comment:11 Changed 18 months ago by Tim Graham

Patch needs improvement: set

I guess you missed the more extensive pull request that was already proposed. It still needs some improvements.

comment:12 Changed 17 months ago by Tim Graham

Patch needs improvement: unset

comment:13 Changed 17 months ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In 11cac1b:

Fixed #4960 -- Added "strip" option to CharField

comment:14 Changed 17 months ago by Thomas Güttler

Thank you very much. I will use the "strip" option in most of my fields.

comment:15 Changed 17 months ago by Tim Graham <timograham@…>

In e7c6a2cf:

Refs #4960 -- Fixed selenium test failures for CharField strip changes.

comment:16 Changed 17 months ago by Chris Lamb

Should the default really be "True"? Somewhat backward incompatibile / unexpected change.

comment:17 Changed 17 months ago by Tim Graham

That was the conclusion of the django-developers thread linked in comment 6. Feel free to raise the issue there if you disagree.

Note: See TracTickets for help on using tickets.
Back to Top