Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[OptionsResolver] Allow setting same normalizer to multiple option#18254

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

@unexge
Copy link

QA
Branch?master
Bug fix?no
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets-
LicenseMIT
Doc PR-

@HeahDude
Copy link
Contributor

I've done the same in#18134 to easily normalize prototype options, but haven't pushed it yet (there is more to come).

#18134 is just a proof of concept for now, so I guess it's good to have this one as a stand-alone too ;)

@backbone87
Copy link
Contributor

whats the problem of callingsetNormalizer multiple times? callingsetNormalizer('a', $f) makes it perfectly clear that$f normalizes all values of optiona, but callingsetNormalizer([ 'a', 'b' ], $f) isnt as clear: does$f operate on each single optiona andb or does it normalize the combination of both[ 'a', 'b' ]?

@HeahDude
Copy link
Contributor

@backbone87 actuallysetDefined andsetRequired works that way.

@backbone87
Copy link
Contributor

@HeahDude These methods have no additional arguments, so its less confusing. But the whole interface shifted more towards a "one call per option" in the recent versions, but the "array set" possibilities didnt got removed (BC). I dont think its a good idea to soften or reverse this path now.

@HeahDude
Copy link
Contributor

@backbone87 I agree. however for handling prototypes like in#18134 it can make sense, my version to come is a new methodsetNormalizerForAll

@unexge
Copy link
Author

@backbone87 nothing wrong with callingsetNormalizer multiple times, this is just syntactic sugar 😄.

i didn't know the whole interface shifted to "one call per option".

closing this PR in favor of interface reliability.

@unexgeunexge closed thisMar 22, 2016
@backbone87
Copy link
Contributor

i didn't know the whole interface shifted to "one call per option".

at least that is my interpretation of recent changes, you dont need to close this PR and wait for feedback from some symfony members or@webmozart in this particular case

@webmozart
Copy link
Contributor

I agree with the decision to close this ticket. I don't think this kind of syntactic sugar is needed. :)

@unexgeunexge deleted the allow-setting-normalizer-to-multiple-option branchMarch 30, 2016 17:03
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@unexge@HeahDude@backbone87@webmozart@javiereguiluz@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp