Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Router] allow to use \A and \z as regex start and end#37711

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

@zlodes
Copy link
Contributor

@zlodeszlodes commentedJul 30, 2020
edited
Loading

QA
Branch?master
Bug fix?yes
New feature?yes
Deprecations?no
Tickets
LicenseMIT
Doc PRsymfony/symfony-docs#...

Some people are using\A and\z as alternative for^ and$ for route requirements.

E.g.: UUID pattern in ramsey/uuid (many routes broke after updating the lib):
ramsey/uuid@569e93a#diff-23c8536f7d61e7d839fd1c93ce0dd354L30-R31

References:
https://www.rexegg.com/regex-anchors.html#A
https://www.rexegg.com/regex-anchors.html#z

P.S.: It is my first PR into Symfony. Maybe I should fix something, just let me know.

@zlodeszlodes changed the titleRouter: allow to use \A and \z as regex start and end[Router] allow to use \A and \z as regex start and endJul 30, 2020
@zlodeszlodesforce-pushed theroute-requirement-with-alternative-regex-start-and-end-syntax branch 2 times, most recently from9892b31 to9362fa8CompareJuly 30, 2020 15:23
@zlodes
Copy link
ContributorAuthor

Travis failed with

fatal: couldn't find remote ref 4

privatefunctionsanitizeRequirement(string$key,string$regex)
{
// Replace \A, \z with classic ^ and $
$regex =str_replace(['\A','\z'], ['^','$'],$regex);

Choose a reason for hiding this comment

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

There is no need to replace all occurrences, only the ones on the edges should be dealt with.
For\A it's trivial, but for\z, we should deal with escaped backslashes, eg\\z should not be replaced.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

@nicolas-grekas Done. I also removed the type-casting forsubstr result because after checking for empty stringsubstr($regex, 1) will always return string.

@nicolas-grekasnicolas-grekas added this to thenext milestoneJul 30, 2020
@nicolas-grekas
Copy link
Member

Note that this could be considered as a new feature. It should target master.

zlodes reacted with thumbs up emoji

@zlodeszlodes changed the base branch from5.0 tomasterJuly 30, 2020 16:51
@zlodeszlodesforce-pushed theroute-requirement-with-alternative-regex-start-and-end-syntax branch from9362fa8 to9bf388dCompareJuly 30, 2020 16:54
@zlodeszlodes marked this pull request as draftJuly 30, 2020 16:59
@zlodeszlodes marked this pull request as ready for reviewJuly 30, 2020 17:10
@fabpotfabpotforce-pushed theroute-requirement-with-alternative-regex-start-and-end-syntax branch from8430e81 tof752eeeCompareJuly 31, 2020 06:30
@fabpot
Copy link
Member

Thank you@zlodes.

@fabpotfabpot merged commitbd59105 intosymfony:masterJul 31, 2020
@rvitaliy
Copy link
Contributor

@nicolas-grekas@zlodes
we probably should add\Z too.
see pcre documentation:https://www.pcre.org/original/doc/html/pcrepattern.html#SEC5

\A matches at the start of the subject
\Z matches at the end of the subject also matches before a newline at the end of the subject
\z matches only at the end of the subject

zlodes reacted with heart emoji

@zlodeszlodes deleted the route-requirement-with-alternative-regex-start-and-end-syntax branchJuly 31, 2020 10:39
@nicolas-grekasnicolas-grekas modified the milestones:next,5.2Oct 5, 2020
@fabpotfabpot mentioned this pull requestOct 5, 2020
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas left review comments

@fabpotfabpotfabpot approved these changes

+1 more reviewer

@TobionTobionTobion approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

5.2

Development

Successfully merging this pull request may close these issues.

6 participants

@zlodes@nicolas-grekas@fabpot@rvitaliy@Tobion@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp