-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[DependencyInjection] include file and line number in deprecation #24191
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
[DependencyInjection] include file and line number in deprecation #24191
Conversation
If we agree on this change, I will update the YAML file loaders from the Routing, Serializer, Translation and Validator components accordingly. |
59092e5
to
196727a
Compare
That is very specific and needs cooperation from the outside. I'm not convinced at all this is a good design... |
For what it's worth: we already follow a similar approach in the |
For configuration errors; file > stacktrace.
Stacktrace is huge but doesnt help me at all. I end up searching the project for |
So, this looks like a workaround to me. |
In general i like the goal of; deprecation X > click WDT > click file > fix |
My intent was to improve the situation in Symfony 3.3 where such a flag could not be added as this would be a new feature. However, I agree that this would be a cleaner solution and thus I opened #24253. |
9126a73
to
176589e
Compare
This PR was merged into the 3.4 branch. Discussion ---------- [Yaml] support parsing files | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #24191 (comment) | License | MIT | Doc PR | TODO This PR adds a new flag `PARSE_FILE` which can be passed to the YAML parser. When given, the input will be interpreted as a filename whose contents will then be parsed. We already supported passing filenames in the past. Back then the features was not deterministic as it just relied on the string being an existing file or not. Now that we are able to control the behaviour of the parser by passing flags this can be done in a much cleaner way and allows to properly deal with errors (i.e. non existent or unreadable files). This change will also allow to improve error/deprecation messages to include the filename being parsed and thus showing more descriptive error messages when people are using the YAML parser with a bunch of files (e.g. as part of the DependencyInjection or Routing component). Commits ------- 9becb8a [Yaml] support parsing files
This PR was squashed before being merged into the 3.3 branch (closes #24244). Discussion ---------- TwigBundle exception/deprecation tweaks | Q | A | ------------- | --- | Branch? | 3.3 | Bug fix? | yes | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes/no | Fixed tickets | #... <!-- #-prefixed issue number(s), if any --> | License | MIT | Doc PR | symfony/symfony-docs#... <!--highly recommended for new features--> - 1st commit) if you view a exception in the profiler, there is no logger available. Making the tab useless, disabled state is now triggered at zero log messages. There's a specialized panel here. - 2nd commit) when an exception occurs this highlights deprecations in the log table outside the profiler with a warning status. This follows the same signal colors in the profiler. - 3rd commit) hide the default inactive tabs from CSS to avoid scrollbar flickering. - 4th commit) favors document.DOMContentLoaded over window.load, we dont want to wait for images to be loaded Further out-of-scope improvements could be; - From #24191; i think the logs table should show a direct `View file` link for every error/deprecation/red or yellow line in here. Traversing with `Show context` is tedious. - links to file.php for your trigger_error() calls - links to config.yml for trigger_error() calls by SF - From #24151; having the same tooling on both sides is nice - Events/Translations logs is noise, we have specialized panels for those. To further reduce the overall page size container logs can be moved away too, linked from Configuration and/or Logs. Also see #23247 Commits ------- 1c595fc [TwigBundle][WebProfilerBundle] Switch to DOMContentLoaded event ea4b096 [WebProfilerBundle] Hide inactive tabs from CSS 0c10f97 [TwigBundle] Make deprecations scream in logs 03cd9e5 [TwigBundle] Hide logs if unavailable, i.e. webprofiler
This PR was squashed before being merged into the 3.3 branch (closes #24244). Discussion ---------- TwigBundle exception/deprecation tweaks | Q | A | ------------- | --- | Branch? | 3.3 | Bug fix? | yes | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes/no | Fixed tickets | #... <!-- #-prefixed issue number(s), if any --> | License | MIT | Doc PR | symfony/symfony-docs#... <!--highly recommended for new features--> - 1st commit) if you view a exception in the profiler, there is no logger available. Making the tab useless, disabled state is now triggered at zero log messages. There's a specialized panel here. - 2nd commit) when an exception occurs this highlights deprecations in the log table outside the profiler with a warning status. This follows the same signal colors in the profiler. - 3rd commit) hide the default inactive tabs from CSS to avoid scrollbar flickering. - 4th commit) favors document.DOMContentLoaded over window.load, we dont want to wait for images to be loaded Further out-of-scope improvements could be; - From symfony/symfony#24191; i think the logs table should show a direct `View file` link for every error/deprecation/red or yellow line in here. Traversing with `Show context` is tedious. - links to file.php for your trigger_error() calls - links to config.yml for trigger_error() calls by SF - From #24151; having the same tooling on both sides is nice - Events/Translations logs is noise, we have specialized panels for those. To further reduce the overall page size container logs can be moved away too, linked from Configuration and/or Logs. Also see #23247 Commits ------- 1c595fcf48 [TwigBundle][WebProfilerBundle] Switch to DOMContentLoaded event ea4b0966ab [WebProfilerBundle] Hide inactive tabs from CSS 0c10f97f98 [TwigBundle] Make deprecations scream in logs 03cd9e553b [TwigBundle] Hide logs if unavailable, i.e. webprofiler
This PR was squashed before being merged into the 3.3 branch (closes #24244). Discussion ---------- TwigBundle exception/deprecation tweaks | Q | A | ------------- | --- | Branch? | 3.3 | Bug fix? | yes | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes/no | Fixed tickets | #... <!-- #-prefixed issue number(s), if any --> | License | MIT | Doc PR | symfony/symfony-docs#... <!--highly recommended for new features--> - 1st commit) if you view a exception in the profiler, there is no logger available. Making the tab useless, disabled state is now triggered at zero log messages. There's a specialized panel here. - 2nd commit) when an exception occurs this highlights deprecations in the log table outside the profiler with a warning status. This follows the same signal colors in the profiler. - 3rd commit) hide the default inactive tabs from CSS to avoid scrollbar flickering. - 4th commit) favors document.DOMContentLoaded over window.load, we dont want to wait for images to be loaded Further out-of-scope improvements could be; - From symfony/symfony#24191; i think the logs table should show a direct `View file` link for every error/deprecation/red or yellow line in here. Traversing with `Show context` is tedious. - links to file.php for your trigger_error() calls - links to config.yml for trigger_error() calls by SF - From #24151; having the same tooling on both sides is nice - Events/Translations logs is noise, we have specialized panels for those. To further reduce the overall page size container logs can be moved away too, linked from Configuration and/or Logs. Also see #23247 Commits ------- 1c595fcf48 [TwigBundle][WebProfilerBundle] Switch to DOMContentLoaded event ea4b0966ab [WebProfilerBundle] Hide inactive tabs from CSS 0c10f97f98 [TwigBundle] Make deprecations scream in logs 03cd9e553b [TwigBundle] Hide logs if unavailable, i.e. webprofiler
0e8936d
to
87bdcc4
Compare
87bdcc4
to
cf03552
Compare
Thank you @xabbuh. |
…ecation (xabbuh) This PR was merged into the 3.3 branch. Discussion ---------- [DependencyInjection] include file and line number in deprecation | Q | A | ------------- | --- | Branch? | 3.3 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | In #23108 we removed line numbers from deprecation messages created by the YAML parser because those numbers were quite useless without the file being parsed. I suggest to revert this change and add the file being parsed to the deprecation message. Commits ------- cf03552 include file and line number in deprecation
In #23108 we removed line numbers from deprecation messages created by the YAML parser because those numbers were quite useless without the file being parsed. I suggest to revert this change and add the file being parsed to the deprecation message.