-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Update excluded_ajax_paths for sf4 #26177
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
@@ -38,7 +38,7 @@ public function getConfigTreeBuilder() | |||
->children() | |||
->booleanNode('toolbar')->defaultFalse()->end() | |||
->booleanNode('intercept_redirects')->defaultFalse()->end() | |||
->scalarNode('excluded_ajax_paths')->defaultValue('^/(app(_[\\w]+)?\\.php/)?_wdt')->end() | |||
->scalarNode('excluded_ajax_paths')->defaultValue('index.php?_wdt')->end() |
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.
that's not the right update. It will now match index.php_wdt
and index.ph_wdt
(and anywhere in the URL, not only at the start), while it should be matching /_wdt
and /index.php/_wdt
and we should actually change it in 3.4, supporting both |
hi @stof thanks for u're review, you want something like this ? |
@ro0NL thanks, i tried with |
5445c10
to
423abe9
Compare
@@ -38,7 +38,7 @@ public function getConfigTreeBuilder() | |||
->children() | |||
->booleanNode('toolbar')->defaultFalse()->end() | |||
->booleanNode('intercept_redirects')->defaultFalse()->end() | |||
->scalarNode('excluded_ajax_paths')->defaultValue('^/(app(_[\\w]+)?\\.php/)?_wdt')->end() | |||
->scalarNode('excluded_ajax_paths')->defaultValue('^/((?:index|app(_[\w]+)?)\.php/)?_wdt')->end() |
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.
perhaps to fully clarify the regex remove ?:
as well.. (it's not really needed) since you removed escaped \
also (before \\
, but also not really needed).
0ce89f6
to
15b44bd
Compare
15b44bd
to
869b81d
Compare
@@ -38,7 +38,7 @@ public function getConfigTreeBuilder() | |||
->children() | |||
->booleanNode('toolbar')->defaultFalse()->end() | |||
->booleanNode('intercept_redirects')->defaultFalse()->end() | |||
->scalarNode('excluded_ajax_paths')->defaultValue('^/(app(_[\\w]+)?\\.php/)?_wdt')->end() | |||
->scalarNode('excluded_ajax_paths')->defaultValue('^/((index|app(_[\w]+)?).php/)?_wdt')->end() |
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.
The dot .
still needs escaping ;) (\.
that is)
For 3.4 i guess |
Thank you @jenaye. |
This PR was submitted for the master branch but it was squashed and merged into the 3.4 branch instead (closes #26177). Discussion ---------- Update excluded_ajax_paths for sf4 | Q | A | ------------- | --- | Branch? | 4.0 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #25941 | License | MIT | Doc PR | https://github.com/symfony/symfony-docs/pull/9267/files This PR update `excluded_ajax_paths` from `vendor/symfony/web-profiler-bundle/DependencyInjection/Configuration.php` because there is no neither `app.php` nor `app_dev.php` in symfony 4 We also need update this [Documentation](https://symfony.com/doc/current/reference/configuration/web_profiler.html) Commits ------- ce01097 Update excluded_ajax_paths for sf4
This PR was merged into the master branch. Discussion ---------- Update Documentation excluded_ajax_paths sf4 Linked with this PR : symfony/symfony#26177 Commits ------- 31f75d3 update doc sf4 for excluded_ajax_paths
This PR update
excluded_ajax_paths
fromvendor/symfony/web-profiler-bundle/DependencyInjection/Configuration.php
becausethere is no neither
app.php
norapp_dev.php
in symfony 4We also need update this Documentation