Skip to content
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

Better notification when min and max string length are the same #266

Merged
merged 1 commit into from
Jun 12, 2018
Merged

Better notification when min and max string length are the same #266

merged 1 commit into from
Jun 12, 2018

Conversation

philippefaes
Copy link
Contributor

Sometimes a string of a fixed length is expect in a form. If you use the Length validator, and choose the same value for max and min, this patch makes the error message more sensible: "Field must be exactly 'n' characters long."

@whb07
Copy link
Contributor

whb07 commented Jun 12, 2018

Hey Phillip I just ran a series of tests setting the Length validator to have a matching min/max values and I wasn't able to reproduce what you're talking about. If you are able to properly document such an event and come back with what you found I'd be more than happy to look at your work further!

If you're trying to get some sample data for the tests checkout Hypothesis for property based testing. I also used Faker on the side for random text.

@whb07 whb07 closed this Jun 12, 2018
@ftm
Copy link
Contributor

ftm commented Jun 12, 2018

@whb07 I'm not entirely sure why you've closed this, @philippefaes was submitting a working patch that I was just working on merging

@ftm ftm added the enhancement New feature, or existing feature improvement label Jun 12, 2018
@davidism davidism reopened this Jun 12, 2018
@whb07
Copy link
Contributor

whb07 commented Jun 12, 2018

whoops! I haven't been able to recreate the error my bad! I ran a series of tests locally and I've yet to see anything that triggers it. Mind showing me a case where it breaks?

@ftm
Copy link
Contributor

ftm commented Jun 12, 2018

@whb07 Nothing breaks, this is just making the error message more helpful. Previously it might say something like Field must be between 5 and 5 characters long whereas now it will say Field must be exactly 5 characters long

@ftm ftm merged commit 7898da5 into wtforms:master Jun 12, 2018
ftm added a commit that referenced this pull request Jun 12, 2018
@whb07
Copy link
Contributor

whb07 commented Jun 12, 2018

oh yah! My bad I didn't review the message properly and thought they somehow ran into an issue with min/max not validating properly. nice patch 👍

azmeuk added a commit to azmeuk/wtforms that referenced this pull request Apr 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature, or existing feature improvement
Development

Successfully merging this pull request may close these issues.

None yet

4 participants