Skip to content

[Routing] Add matched and default parameters to redirect responses #18012

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

Conversation

artursvonda
Copy link
Contributor

Q A
Branch 2.3
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR

}

// baz2
if ($pathinfo === '/test/baz.html') {
return array('_route' => 'baz2');
$ret = array('_route' => 'baz2');
return $ret;
Copy link
Contributor

Choose a reason for hiding this comment

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

Are those changes intended ? You should revert them.

Copy link
Member

Choose a reason for hiding this comment

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

This is a fixture file and the code is generated, this is intended.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, I've missed that.

@fabpot
Copy link
Member

fabpot commented Mar 6, 2016

👍

@fabpot
Copy link
Member

fabpot commented Mar 6, 2016

Original PR is #11380

@xabbuh
Copy link
Member

xabbuh commented Mar 7, 2016

Is this now considered a bug fix or a new feature (I seem to miss the conclusion in #11380)?

@fabpot
Copy link
Member

fabpot commented Mar 31, 2016

Indeed, I messed up in the other PR. @Tobion was saying that this should be considered as a new feature. Anyway, I can still merge this one in master instead of 2.3. Sorry for the confusion.

@Tobion Does it look good?

@fabpot
Copy link
Member

fabpot commented Jun 12, 2016

@Tobion Can you give us your opinion on this PR?

@Tobion
Copy link
Contributor

Tobion commented Jun 12, 2016

I finished it in #19037. Rebased against master as it didn't apply cleanly and fixed adding the route params for the slash redirect as well. This was only done in the dumped version but not in the RedirectableUrlMatcher directly.

@Tobion Tobion closed this Jun 12, 2016
nicolas-grekas added a commit that referenced this pull request Jul 11, 2017
…ct responses (artursvonda, Tobion)

This PR was merged into the 3.4 branch.

Discussion
----------

[Routing] Add matched and default parameters to redirect responses

| Q | A |
| --- | --- |
| Branch? | master |
| Bug fix? | no |
| New feature? | yes |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #18012, #19037 |
| License | MIT |
| Doc PR |  |

Finished #18012 and #19037

Commits
-------

4c1fdd4 [Routing] also add matched params for redirect due to trailing slash
dc3f7a9 [Routing] Add matched and default parameters to redirect responses
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.

7 participants