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

[Routing] Make important parameters required when matching#29770

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

@vudaltsov
Copy link
Contributor

@vudaltsovvudaltsov commentedJan 4, 2019•
edited
Loading

QA
Branch?master
Bug fix?yes
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets#29763
LicenseMIT
Doc PRn/a
  1. This PR improves "important" route parameters implementation. Instead of considering!slug to be a variable name (which is not correct from my POV and leads to a lot of'!' === $varName[0] andsubstr($varName, 1) snippets), I took advantage of the$token array and used offset[5] for keeping boolean importance flag. This approach improved and simplified code.
  2. This PR makes important parameters required when matching according toMatching for! prefixed placeholders should make them required #29763

@nicolas-grekasnicolas-grekas added this to thenext milestoneJan 4, 2019
@vudaltsov
Copy link
ContributorAuthor

As discussed with@nicolas-grekas ,#29763 will be implemented in this PR as well :)

@vudaltsovvudaltsov changed the title[Routing] Refactor important route parameters[Routing] Make important parameters required when matchingJan 4, 2019
@vudaltsovvudaltsovforce-pushed therouting-important-parameter-refactoring branch from7ae21c9 to098ab7bCompareJanuary 4, 2019 18:38
for ($i =\count($tokens) -1;$i >=0; --$i) {
$token =$tokens[$i];
if ('variable' ===$token[0] &&$route->hasDefault($token[3])) {
if ('variable' ===$token[0] &&!$token[5] &&$route->hasDefault($token[3])) {
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Here is the change that prevents matching

@vudaltsov
Copy link
ContributorAuthor

@nicolas-grekas , ready!

@vudaltsov
Copy link
ContributorAuthor

CI fails are not relevant (I see some Intl component errors), Routing is green locally :)

@vudaltsovvudaltsovforce-pushed therouting-important-parameter-refactoring branch 3 times, most recently fromf18a6e1 toca35103CompareJanuary 15, 2019 12:36
@vudaltsov
Copy link
ContributorAuthor

Finally green and ready!

@vudaltsovvudaltsovforce-pushed therouting-important-parameter-refactoring branch 2 times, most recently from4bd3fc8 to8d4be10CompareJanuary 24, 2019 14:50
@vudaltsovvudaltsovforce-pushed therouting-important-parameter-refactoring branch from8d4be10 to2c3ab22CompareJanuary 25, 2019 07:56
@vudaltsov
Copy link
ContributorAuthor

@nicolas-grekas , I managed to keep consistency with old tests!

@nicolas-grekas
Copy link
Member

Thank you@vudaltsov.

@nicolas-grekasnicolas-grekas merged commit2c3ab22 intosymfony:masterJan 25, 2019
nicolas-grekas added a commit that referenced this pull requestJan 25, 2019
… (vudaltsov)This PR was merged into the 4.3-dev branch.Discussion----------[Routing] Make important parameters required when matching| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets |#29763| License       | MIT| Doc PR        | n/a1. This PR improves "important" route parameters implementation. Instead of considering `!slug` to be a variable name (which is not correct from my POV and leads to a lot of `'!' === $varName[0]` and `substr($varName, 1)` snippets), I took advantage of the `$token` array and used offset `[5]` for keeping boolean importance flag. This approach improved and simplified code.1. This PR makes important parameters required when matching according to#29763Commits-------2c3ab22 Made important parameters required when matching
@vudaltsovvudaltsov deleted the routing-important-parameter-refactoring branchJanuary 25, 2019 11:28
@nicolas-grekasnicolas-grekas modified the milestones:next,4.3Apr 30, 2019
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@javiereguiluzjaviereguiluzjaviereguiluz left review comments

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

4.3

Development

Successfully merging this pull request may close these issues.

4 participants

@vudaltsov@nicolas-grekas@javiereguiluz@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp