[Validator] Support "maxSize" given in KiB #11027
Conversation
| { | ||
| if (is_array($options) && array_key_exists('maxSizeFormat', $options)) { | ||
| throw new ConstraintDefinitionException(sprintf( | ||
| 'The option "maxSizeFormat" is not supported by the constraint %s', |
There was a problem hiding this comment.
I don't think this makes sense to disallow. Since it's a public property it could be manually set after the constructor anyway. So I would make it so that one could specifiy the size in binary format but still set the formatting to decimal for the validation message. Only set it to binary when it's null.
There was a problem hiding this comment.
Then I would also rename the property to $binaryFormat = null|false|true when null then based on maxSize.
There was a problem hiding this comment.
In a previous PR, @fabpot says that he prefered to avoid adding another parameter (#10917 (comment)).
And I agree with him, the purpose of this new parameter will just be to let the user choise the format of the error message 😕
But you're right this trick is trying to avoid to expose a new paramater, but in the reality, there is a new parameter.
Should I leave maxSize has the user defined it. And expose 2 getter in Constraint\File: getMaxSizeBytes and getMaxSizeFormat ? But this is not what @webmozart expected in his ticket #10962
There was a problem hiding this comment.
I don't see what is bad at having an option for binary/decimal validation format. For example I can image to have that configurable in a CMS which can be useful when you expect your users won't understand the binary suffixes.
|
👍 |
1 similar comment
|
👍 |
|
Thank you @jeremy-derusse. |
This PR was merged into the 2.3-dev branch. Discussion ---------- [Validator] smaller CS fixes | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT First commit fixes some CS of #11027 Second one removes `null` initialization from Validator as we don't do it. Commits ------- 2025778 [Validator] no need to initialize properties with null 967576a [Validator] smaller fixes for binary format
…sse) This PR was merged into the master branch. Discussion ---------- [Validator] Support "maxSize" given in KiB | Q | A | ------------- | --- | Doc fix? | no | New docs? | symfony/symfony#11027 | Applies to | 2.6 | Fixed tickets | symfony/symfony#10962 To display the constraint validation message with an expected suffix, this PR add a new option in File constraint. Commits ------- 1e8fa48 Replace bullet list by a table 77a0687 Add versionAdded on binary suffix b414f5c Use comma as thousands separator 7dcd7c3 Move secion "binaryFormat" to the right place 71fdd60 Fix line wrap 3f3f4e0 Provide information about SI and Binary prefixes deac0c3 Add option binaryFormat in constraint file e3acdc5 Support MaxSize in KiB and MiB
To display the constraint validation message with an expected suffix, this PR add a new option in File constraint.