🌐 AI搜索 & 代理 主页
Skip to content

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