-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[RFC][DX][OptionsResolver] Allow setting info message per option #35400
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
Here's a crazy proposal 🙈 to solve this same problem but using the Symfony Validator component: // Before:
$resolver = new OptionsResolver();
$resolver->setAllowedValues('date', static function ($value): bool {
return $value >= new \DateTime('now');
});
$resolver->setInfo('date', 'This option must be a date in the future.');
// After:
use Symfony\Component\Validator\Constraints\GreaterThan;
$resolver = new OptionsResolver();
$resolver->setValidator('date', new GreaterThan([
'value' => new DateTime('today'),
'message' => 'This option must be a date in the future.'
])); |
Hi @javiereguiluz, thanks for your input :) your proposal would require the integration of the Validator component, isn't it? Note that my proposal isn't only about improving the validation error message, but also about debugging information on |
Thanks for your comments ... I was thinking about an optional dependency with the Validator component (the usual "You can't use XXXX because YYYY. Try running composer require ..." message we use in other parts of Symfony). But yes, my comment is completely different from your original proposal. We can ignore it! Thanks. |
9f968b7
to
2748e6f
Compare
b76529c
to
cd75ad6
Compare
cd75ad6
to
0477a06
Compare
Comments addressed, thanks! |
Thank you @yceruto. |
…per option (yceruto) This PR was merged into the 5.1-dev branch. Discussion ---------- [RFC][DX][OptionsResolver] Allow setting info message per option | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | - | License | MIT | Doc PR | TODO This is a DX proposal that will help in debugging/errors to better understand the meaning of one defined option. This is how you'd use it: ```php $resolver = new OptionsResolver(); $resolver->setDefined('date'); $resolver->setAllowedTypes('date', \DateTime::class); $resolver->setInfo('date', 'A future date'); // <-- NEW // ... ``` This information may be useful for those options where their name cannot be intuitive enough, or their purpose is too complex. Here is an example (based on the example above): ```php // ... $resolver->setAllowedValues('date', static function ($value): bool { return $value >= new \DateTime('now'); }); ``` So, if you introduce a date value that does not match the criteria, you will get this error message: **Before:** ``` The option "date" with value DateTime is invalid. ``` Note that the allowed value is not printable in this case, hence the error message cannot be clear at all. **After:** ``` The option "date" with value DateTime is invalid. Info: A future date. ``` Although a more accurate error message can be triggered within the `\Closure` if desired. Also you'll see this info message on `debug:form` command (see tests), then you have in advance the informative description of any option. What do you think? Commits ------- 0477a06 Allow setting one info message per option
This is a DX proposal that will help in debugging/errors to better understand the meaning of one defined option.
This is how you'd use it:
This information may be useful for those options where their name cannot be intuitive enough, or their purpose is too complex. Here is an example (based on the example above):
So, if you introduce a date value that does not match the criteria, you will get this error message:
Before:
Note that the allowed value is not printable in this case, hence the error message cannot be clear at all.
After:
Although a more accurate error message can be triggered within the
\Closure
if desired.Also you'll see this info message on
debug:form
command (see tests), then you have in advance the informative description of any option.What do you think?