Use the master request from the stack #127
Conversation
| @@ -17,7 +17,7 @@ | |||
| "minimum-stability": "dev", | |||
| "require": { | |||
| "php": ">=5.3.2", | |||
| "symfony/framework-bundle": ">=2.2.2", | |||
| "symfony/framework-bundle": ">=2.4", | |||
There was a problem hiding this comment.
i guess we can live with that. there are released versions available for the 2.3 LTS...
|
@dbu don't know if subrequests will carry the locale. I remember there were some problems with subrequests (!master request) |
|
I think we should fix the failing test first, do a tag and work on the request_stack then |
|
Sounds good. I'll try to find some time later this week. |
awesome! how much do we need to worry about ESI requests in the locale switcher? |
|
This fixes the switcher tests and updates the validator tests that were failing. Filipino seems to have been left out of ISO-639-2 in Intl. Report this as a Symfony bug do you think? I can't send a pull request as a tag, but could close and re-request against a branch if you'd like to do that. You'll need to create the branch first though. |
| @@ -81,7 +82,7 @@ public function validate($locale, Constraint $constraint) | |||
| if ($this->intlExtension) { | |||
| $primary = SymfonyLocale::getPrimaryLanguage($locale); | |||
| $region = SymfonyLocale::getRegion($locale); | |||
| $locales = SymfonyLocale::getLocales(); | |||
| $locales = Intl::getLocaleBundle()->getLocales(); | |||
There was a problem hiding this comment.
is Intl always present or only from 2.4 on? i remember there was a up and down on this.
maybe we could explicitly require the intl component if we need this here, if you do the fix i propose above to support symfony 2.3 again.
There was a problem hiding this comment.
Just double checked; looks like Intl has indeed been around since 2.3. Will update per the above.
|
yeah, if the Intl is missing things, best discuss that in an issue at symfony/symfony. did it work for SymfonyLocale::getLocales? should we continue to use that until there is a fixed Intl, to avoid regressions? |
|
oh, and for the process of fixing things here: you would need to create a separate pull request with just the test fixes so we get green again, then @lunetics can merge this and create the tag. then you would rebase this PR on master and change the branch-alias in composer.json so that we start working on a new version of this bundle. |
|
Filipino is dicey in the older Locale component. Seems to work on some platforms; I assume because of the available version of ICU. |
|
Actually, Filipino seems to have also been removed from the old Locale component as of Symfony 2.3 |
… figure out why it was removed.
|
GitHub has defeated me. Closing this and re-requesting against #125. |
I think this solves #84, though not sure if it will cause problems with ESI. Can anyone confirm?
Always using the master request from the request stack means that we're only concerned about the locale of the controlling page.