[form] lazy trans post_max_size_message.
#19595
Conversation
aitboudad
commented
Aug 10, 2016
| Q | A |
|---|---|
| Branch? | 2.7 |
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #19256 |
| License | MIT |
| Doc PR | ~ |
| } | ||
|
|
||
| /** | ||
| * This method is public because it needs to be callable from a closure in PHP 5.3. |
There was a problem hiding this comment.
you can do without it by using a reference in the use of the closure, wouldn't that be better?
There was a problem hiding this comment.
even better :), thanks
d25a0c8 to
1238c9e
Compare
| @@ -81,7 +81,7 @@ public function handleRequest(FormInterface $form, $request = null) | |||
| $form->submit(null, false); | |||
|
|
|||
| $form->addError(new FormError( | |||
| $form->getConfig()->getOption('post_max_size_message'), | |||
| (string) $form->getConfig()->getOption('post_max_size_message'), | |||
There was a problem hiding this comment.
could this be a bc break somehow?
There was a problem hiding this comment.
not sure, but it can be treated as a string like the old ones
There was a problem hiding this comment.
does it work if you remove the cast?
There was a problem hiding this comment.
just checked and I found that it can lead a bc break (if someone try to serialize this option)
|
@aitboudad any other idea to fix the linked issue that has no bc break inside? |
| @@ -55,4 +57,9 @@ public function getExtendedType() | |||
| { | |||
| return 'form'; | |||
| } | |||
|
|
|||
| public function __toString() | |||
There was a problem hiding this comment.
-1 for making the TypeExtension itself castable to string as the error message. It looks really weird to me.
There was a problem hiding this comment.
I don't like either :), but I was looking for a solution to avoid bc break
|
Replace |
|
@aitboudad let's try :) |
6f52062 to
7797a16
Compare
|
done |
| @@ -207,8 +213,10 @@ public function configureOptions(OptionsResolver $resolver) | |||
| 'action' => '', | |||
| 'attr' => $defaultAttr, | |||
| 'post_max_size_message' => 'The uploaded file was too large. Please try to upload a smaller file.', | |||
| 'upload_max_size_message' => $uploadMaxSizeMessage, | |||
There was a problem hiding this comment.
You could add // internal at the end of the line, and/or maybe add a comment somewhere explaining why you define it as a callable.
There was a problem hiding this comment.
just added a small description, can you look up ?
|
Wow, great! Thanks for that fix! |
7797a16 to
0b597f5
Compare
| @@ -146,6 +146,14 @@ public function configureOptions(OptionsResolver $resolver) | |||
| }; | |||
| }; | |||
|
|
|||
| // Derive "upload_max_size_message" closure from "post_max_size_message" option, | |||
| // to allow override or translate the message only when it's needed and not during the resolve `options`. | |||
There was a problem hiding this comment.
I think there are typo: overriding and translating.
What about something like:
// Wrap "post_max_size_message" in a closure to translate it lazily
$uploadMaxSizeMessage ...0b597f5 to
c03164e
Compare
|
This one is very tricky :) 👍 Status: reviewed |
|
Thank you @aitboudad. |
This PR was merged into the 2.7 branch. Discussion ---------- [form] lazy trans `post_max_size_message`. | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #19256 | License | MIT | Doc PR | ~ Commits ------- c03164e [form] lazy trans `post_max_size_message`.