Skip to content

[Routing] Allow using UTF-8 parameter names #45054

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

Merged
merged 1 commit into from
Jan 19, 2022

Conversation

nicolas-grekas
Copy link
Member

Q A
Branch? 6.1
Bug fix? no
New feature? yes
Deprecations? no
Tickets Fix #41909
License MIT
Doc PR -

Copy link
Contributor

@fancyweb fancyweb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we add a changelog entry?

@nicolas-grekas
Copy link
Member Author

Updated, thanks for the review.

@GromNaN
Copy link
Member

GromNaN commented Jan 18, 2022

A test could be added to RouteTest and RouteCompilerTest to cover this feature.

@nicolas-grekas
Copy link
Member Author

This is already covered by the current test to me.

@stof
Copy link
Member

stof commented Jan 18, 2022

The change in extractInlineDefaultsAndRequirements is not covered by tests, as you don't use an UTF-8 name on a parameter using inline config (try reverting that part of the diff and see that tests won't fail).
and changes in places computing $hasTrailingVar might need a test using a route using a trailing optional parameter using an UTF-8 name to be covered in a way preventing regression.

@nicolas-grekas
Copy link
Member Author

Tests added @stof

@fabpot
Copy link
Member

fabpot commented Jan 19, 2022

Thank you @nicolas-grekas.

@fabpot fabpot merged commit ae3b078 into symfony:6.1 Jan 19, 2022
@nicolas-grekas nicolas-grekas deleted the route-utf8 branch January 19, 2022 09:46
@fabpot fabpot mentioned this pull request Apr 15, 2022
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.

Route not found with UTF-8 parameter name
6 participants