New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove spaces at start and end of text field values. #20
Conversation
Hi, do you agree with this pull request ? Thomas |
@@ -46,6 +46,8 @@ def toFieldValue(self, value): | |||
"""See interfaces.IDataConverter""" | |||
if value == u'': | |||
return self.field.missing_value | |||
if value is not None: | |||
value = value.strip() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So ''
is converted to self.field.missing_value
but ' '
is converted to ''
. Does that make sense?
(I don't know, I must admit I never understood missing values).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right ! i will intervert 49-50 and 47-48
Other than that question, this looks good to me. It is, however, backwards-incompatible. Is that important? I don't know! Is anyone out there relying on extra spaces in text fields being preserved? I don't know! Therefore I'm not going to be the one who merges this. |
Hi, i considered mgedmin diff Anyone could review/merge the pull request ? Thanks |
Hi, can we merge this ? |
The biggest question is "Is anyone out there relying on extra spaces in text fields being preserved?" |
Hi @agroszer i added a class variable to do that. Is it ok now ? |
In general I am for accepting the patch. I do not have time for the merge and release, but Adam, feel free to do so. |
Thanks ! 2014-06-09 15:09 GMT+02:00 Adam Groszer notifications@github.com:
Thomas Desvenain Téléphone : 09 51 37 35 18 |
that merge broke tests in Plone, in which tests' we rely on spaces in fields. for a discussion on this see: plone/plone.app.portlets@a8330bd#commitcomment-6667480 |
No description provided.