-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Routing] Expose request in route conditions, if needed and possible #22636
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
I don't know if it's possible to change it, but don't you think that the error message in this case should be a yellow warning with this message?
|
It's possible... sure ;-) but perhaps out-of-scope for now. The part after Also |
{ | ||
return class_exists('Symfony\Component\HttpFoundation\Request') | ||
? Request::create($this->context->getScheme().'://'.$this->context->getHost().$this->context->getBaseUrl().$pathinfo, $this->context->getMethod(), $this->context->getParameters()) | ||
: null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be on one line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will do 👍 i also realized the request is probably not fully functional now; it needs some server params so it can properly resolve baseurl + basepath and such. I wanted to look at that this weekend :)
@fabpot this should do. |
@@ -248,4 +248,16 @@ protected function getExpressionLanguage() | |||
|
|||
return $this->expressionLanguage; | |||
} | |||
|
|||
protected function createRequest($pathinfo) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be private IMHO
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hum, @internal
in fact
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Thank you @ro0NL. |
…d possible (ro0NL) This PR was squashed before being merged into the 2.7 branch (closes #22636). Discussion ---------- [Routing] Expose request in route conditions, if needed and possible | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #16968, #22635 | License | MIT | Doc PR | - given ``` /** * @route("/", name="homepage", condition="request.isXmlHttpRequest()") */ ``` ``` $ app/console route:match / ``` before  after  Commits ------- 016e976 [Routing] Expose request in route conditions, if needed and possible
Thank you @ro0NL. |
…d and possible (ro0NL) This PR was squashed before being merged into the 2.7 branch (closes #22636). Discussion ---------- [Routing] Expose request in route conditions, if needed and possible | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #16968, #22635 | License | MIT | Doc PR | - given ``` /** * @route("/", name="homepage", condition="request.isXmlHttpRequest()") */ ``` ``` $ app/console route:match / ``` before  after  Commits ------- 94371d0 [Routing] Expose request in route conditions, if needed and possible
given
before
after