Skip to content

Localized Passthrough Locale for #[Route()] #53571

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

Closed
wants to merge 0 commits into from

Conversation

cyraid
Copy link
Contributor

@cyraid cyraid commented Jan 18, 2024

Q A
Branch? 7.1
Bug fix? no
New feature? yes
Deprecations? no
License MIT

As per reference of #53539, I was looking for a way to solve:
https://stackoverflow.com/questions/41721408/symfony-default-locale-without-specifying-in-url/77819142

Currently the first path (either specifying a path, or a path array) the first path index is 0 in AttributeClassLoader.php. Taking advantage of this, my answer in the Stack Overflow and in #53539 works, but is not advised as this is not documented. My PR adds support for:

    #[Route(path: ['{_locale}' => '/{_locale}', 'nl' => '/nl'], name: 'action')]

Which adds the path as 'action' and 'action.nl', means the {_locale} can be used to catch all other locales. As @xabbuh and I talked about. Using this:

    #[Route(path: ['/{_locale}', 'nl' => '/nl'], name: 'action')]

Is still allowed as the first array index of 0 was always allowed anyway (as the Route would turn the $path into $paths[]), and should remain to not break backwards compatibility with anyone using it, but if 1 => etc (integer array keys) used, it will raise an exception (also tested in the Tests).

If you guys could look this over that would be great. If someone needs any clarification for anyone wanting to document this feature near https://symfony.com/doc/current/routing.html#localized-routes-i18n that also would be great!

@carsonbot carsonbot added this to the 7.1 milestone Jan 18, 2024
@cyraid cyraid changed the title Locale catch route Localized Passthrough Locale for #[Route()] Jan 18, 2024
@cyraid
Copy link
Contributor Author

cyraid commented Jan 18, 2024

I'll wait for merge of PR #53580 AttributeClassLoaderTestCase, and then update this PR.

@cyraid cyraid marked this pull request as draft January 18, 2024 13:30
@cyraid
Copy link
Contributor Author

cyraid commented Jan 19, 2024

@xabbuh since the tests for this require the patch from 7.0, and I need to base it off 7.1 for the feature. Would we be able to merge the 7.0 fix to 7.1 and then use the PR if everyone is alright with the idea?

@xabbuh
Copy link
Member

xabbuh commented Jan 19, 2024

@cyraid I have merged up the 7.0 branch

@cyraid cyraid closed this Jan 20, 2024
@cyraid cyraid force-pushed the locale-catch-route branch from 5192fe3 to 6c44684 Compare January 20, 2024 07:34
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.

3 participants