Skip to content
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

[TwigBridge][TwigBundle] Add current locale to AppVariable #49913

Merged
merged 1 commit into from Apr 11, 2023
Merged

[TwigBridge][TwigBundle] Add current locale to AppVariable #49913

merged 1 commit into from Apr 11, 2023

Conversation

SVillette
Copy link
Contributor

@SVillette SVillette commented Apr 3, 2023

Q A
Branch? 6.3
Bug fix? no
New feature? yes
Deprecations? no
Tickets Fix #49870
License MIT
Doc PR symfony/symfony-docs#18190

As stated in #49870, they were no way to get the current locale without passing it through a variable when rendering a template within LocaleSwitcher::runWithLocale().

#[AsController]
final class HomeController
{
    #[Route('/', name: 'app_home')]
    public function __invoke(LocaleSwitcher $localeSwitcher, Environment $twig): Response
    {
        $localeSwitcher->setLocale('en');

        return $localeSwitcher->runWithLocale('fr', function () use ($twig) {
            return new Response($twig->render('index.html.twig'));
        });
    }
}
{{ app.locale }} // fr

A doc PR will be submitted if this change is accepted.

@SVillette SVillette requested a review from yceruto as a code owner April 3, 2023 14:55
@carsonbot carsonbot added this to the 6.3 milestone Apr 3, 2023
@carsonbot carsonbot changed the title [TwigBridge] [TwigBundle] Add current locale to AppVariable [TwigBridge][TwigBundle] Add current locale to AppVariable Apr 3, 2023
@SVillette
Copy link
Contributor Author

About the failing tests, I may need some help / directions here.

  • The Integration / Integration (8.1) seems to be unrelated to this PR (Cache related).
  • The Unit Tests / Unit Tests (8.1) is failing because AppVariable::setLocaleSwitcher() does not have a return type and is not listed in .github/expected-missing-return-types.diff. Updating this file should resolve the issue but I can't do it as they are bunch of other changes here.
  • The Unit Tests / Unit Tests (8.1, low-deps) fails because LocaleSwitcher class is missing. Do I have to skip the test if the class does not exist ?
  • Finally, continuous-integration/appveyor/pr fail seems unrelated to this PR (VarDumper issue).

src/Symfony/Bridge/Twig/AppVariable.php Outdated Show resolved Hide resolved
@stof
Copy link
Member

stof commented Apr 7, 2023

For low-deps, you need to increase the lower bound of the requirement

@SVillette
Copy link
Contributor Author

Thank you for the input, CI is still failling but not because of this PR.

@fabpot
Copy link
Member

fabpot commented Apr 11, 2023

Thank you @SVillette.

@fabpot fabpot merged commit 6b92f5d into symfony:6.3 Apr 11, 2023
8 of 9 checks passed
@SVillette SVillette deleted the twig-bridge-app-locale branch April 11, 2023 18:16
OskarStark added a commit to symfony/symfony-docs that referenced this pull request Apr 14, 2023
This PR was merged into the 6.3 branch.

Discussion
----------

Add Twig `app.locale` documentation

The PR add documentation of symfony/symfony#49913

Commits
-------

ac51842 docs: add `app.locale` from Twig `AppVariable`
@curry684
Copy link
Contributor

curry684 commented May 3, 2023

When installing the Twig Bundle 6.3.0-BETA1 this breaks the application if you didn't directly add the bridge as well:

!!    [Symfony\Component\ErrorHandler\Error\UndefinedMethodError]
!!    Attempted to call an undefined method named "setLocaleSwitcher" of class "S
!!    ymfony\Bridge\Twig\AppVariable".

Bundle 6.3 is thus incompatible with Bridge 6.2 yet this is not enforced through dependencies.

fabpot added a commit that referenced this pull request May 7, 2023
…version (SVillette)

This PR was merged into the 6.3 branch.

Discussion
----------

[TwigBundle] fixed wrong `symfony/twig-bridge` dependency version

| Q             | A
| ------------- | ---
| Branch?       | 6.3
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | Fix #50245
| License       | MIT
| Doc PR        | -

`TwigBundle` had wrong dependency version  after #49913. This PR fixes this issue.

Commits
-------

c05f484 fix(twig-bundle): fixed wrong `symfony/twig-bridge` dependency
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[TwigBridge] Add current locale to AppVariable service
5 participants