-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Form] max option of Length & Range Constraint #11930
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
Conversation
OK for Length, but I think guessing the pattern for Range is wrong though |
@stof The maximum length for a range guessed on the same principle https://github.com/vadim2404/symfony/blob/guess/src/Symfony/Component/Form/Extension/Validator/ValidatorTypeGuesser.php#L203 |
but a Range is not about the string length at all. |
Of course, but if we use range validator from "12" to "153" we know, that length of number must be from "2" to "3". I think that in pattern for range validator must be used "\d" instead of ".". |
@@ -234,6 +238,10 @@ public function guessPatternForConstraint(Constraint $constraint) | |||
|
|||
case 'Symfony\Component\Validator\Constraints\Range': | |||
if (is_numeric($constraint->min)) { | |||
if (is_numeric($constraint->max)) { | |||
return new ValueGuess(sprintf('.{%s,%s}', strlen((string) $constraint->min), strlen((string) $constraint->max)), Guess::LOW_CONFIDENCE); |
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.
Could you replace the dot by a "\d" here and down below?
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.
IMO, The pattern guessing should be removed entirely for Range. It is broken for float 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.
guessing min
and max
attributes (supported by the number
type in HTML5) is better for the Range constraint
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.
👍 for using min
max
attributes, I never understood why pattern was used. Was it more supported ?
Remove fix for range validator or replace "." to "\d"? What should be? |
Thanks for working on this @vadim2404! I agree with @stof that it would be better to generate "min"/"max" attributes instead. Unfortunately, that's not possible before #7868 is implemented, so I'm going to close this PR. |
see #15014 |
Extended option "pattern" because "max_length" has been marked as deprecated since 2.5