Opened 8 years ago

Closed 2 months ago

Last modified 8 weeks ago

#4960 closed New feature (fixed)

Add "strip" keyword argument to CharField constructor

Reported by: tzellman@… Owned by: danielward
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@… 8 years ago.
Patch containing example addition of "strip" keyword to CharField
after.patch (1.3 KB) - added by danielward 3 months ago.
Follow-up tests
potential-solution.patch (1.1 KB) - added by danielward 3 months ago.
Potential solution by adding in 'strip' option to CharField

Download all attachments as: .zip

Change History (20)

Changed 8 years ago by tzellman@…

Patch containing example addition of "strip" keyword to CharField

comment:1 Changed 8 years ago by tzellman@…

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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 8 years ago by Simon G. <dev@…>

  • Triage Stage changed from Unreviewed to Design decision needed

comment:3 Changed 8 years ago by adrian

  • Resolution set to wontfix
  • Status changed from new to closed

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

comment:4 Changed 8 years ago by guettli

  • Cc hv@… added

comment:5 Changed 3 years ago by intgr

  • Cc marti@… added
  • Easy pickings unset
  • Severity set to Normal
  • Type set to Uncategorized
  • UI/UX unset

comment:6 Changed 5 months ago by timgraham

  • Patch needs improvement set
  • Triage Stage changed from Design decision needed to Accepted
  • Type changed from Uncategorized to New feature

comment:7 Changed 4 months ago by timgraham

  • Resolution wontfix deleted
  • Status changed from closed to new

comment:8 Changed 3 months ago by timgraham

  • Patch needs improvement unset

comment:9 Changed 3 months ago by timgraham

  • Patch needs improvement set

comment:10 Changed 3 months ago by danielward

  • Owner changed from nobody to danielward
  • Patch needs improvement unset
  • Status changed from new to assigned

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

Changed 3 months ago by danielward

Follow-up tests

Changed 3 months ago by danielward

Potential solution by adding in 'strip' option to CharField

comment:11 Changed 3 months ago by timgraham

  • 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 2 months ago by timgraham

  • Patch needs improvement unset

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

  • Resolution set to fixed
  • Status changed from assigned to closed

In 11cac1b:

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

comment:14 Changed 2 months ago by guettli

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

comment:15 Changed 8 weeks ago by Tim Graham <timograham@…>

In e7c6a2cf:

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

comment:16 Changed 8 weeks ago by lamby

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

comment:17 Changed 8 weeks ago by timgraham

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