-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
| ->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
| ->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
| ->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_pathsfromvendor/symfony/web-profiler-bundle/DependencyInjection/Configuration.phpbecausethere is no neither
app.phpnorapp_dev.phpin symfony 4We also need update this Documentation