Skip to content

accepting callbacks for symfony/options-resolver -> setAllowedValues() #28129

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.

Sign up for GitHub

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

Jump to bottom
Closed
hdimo opened this issue Aug 3, 2018 · 7 comments
Closed

accepting callbacks for symfony/options-resolver -> setAllowedValues() #28129

hdimo opened this issue Aug 3, 2018 · 7 comments
Labels
Feature OptionsResolver

Comments

@hdimo
Copy link

hdimo commented Aug 3, 2018

Description
component : symfony/options-resolver
class: OptionsResolver
method: setAllowedValues()

add capability for setAllowedValues to accept callbacks , instead of rewriting the same Closure for same allowed value , for sharing and prevent retyping

Example

class MyAllowedValue {
    public function date_type($value){
        //example :  accept only date format to be in dd-mm-yyyy
        return preg_match('[0-9]{2}-[0-9]{2}-[0-9]{4}', $value);
    }
}
$optionResolver->setAllowedValues('date_in', [(new MyAllowedValue()), 'date_type']);

//2nd or inside class
class MySimple {
    public function configureOptions(array $option)
    {
        $resolver = new OptionsResolver();
        $resolver->setDefault('option_name', 'default');
        $resolver->setAllowedValues('option_name', [$this, 'myAllowedValues']);
    }
    public function myAllowedValues($value){
        // do some work
        return true;// | false
    }
}
@yceruto
Copy link
Member

yceruto commented Aug 4, 2018

Hi @jkhaled Yes, I also think that allowing callable instead of \Closure opens more alternatives, but since PHP 7.1 we can use \Closure::fromCallable() that converts a callable into a closure, which is enough to solve the problem... what do you think?

@jvasseur
Copy link
Contributor

jvasseur commented Aug 4, 2018

Allowing callable would make the argument ambiguous, since it could be interpreted as an array of values that you want to allow.

@hdimo
Copy link
Author

hdimo commented Aug 4, 2018

@jvasseur I have just created a PR for the implementation, tested
you can check

@sstok
Copy link
Contributor

sstok commented Aug 4, 2018 •
edited
Loading

@jkhaled Can you post a link to the pull-request here? Thanks.

@xabbuh
Copy link
Member

xabbuh commented Aug 4, 2018

I am against making such a change. As said above supporting all types of callables makes the option value ambiguous. That's by the way the reason we dropped this kind of support for some options provided by core form types.

@HeahDude
Copy link
Contributor

HeahDude commented Aug 4, 2018

This is a BC break anyway, since a string or an array of two strings currently setting one or two allowed values could then become callable, and there is no way to prevent the break AFAIK.

@javiereguiluz
Copy link
Member

Let's close this for the reasons given by the maintainers. Also, an alternative solution was given by @yceruto. Thanks.

@yceruto yceruto mentioned this issue Mar 7, 2019
Merged
@yceruto yceruto mentioned this issue May 17, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Feature OptionsResolver
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@javiereguiluz @jvasseur @sstok @xabbuh @yceruto @hdimo @HeahDude