Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[ErrorHandler] Handle PHP 8.3highlight_file function output changes#51586
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Merged
nicolas-grekas merged 1 commit intosymfony:5.4fromPhilETaylor:php8errorrenderhighlightfileSep 11, 2023
Merged
[ErrorHandler] Handle PHP 8.3highlight_file function output changes#51586
nicolas-grekas merged 1 commit intosymfony:5.4fromPhilETaylor:php8errorrenderhighlightfileSep 11, 2023
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
d73b6c0 toad10b00CompareMember
derrabus commentedSep 6, 2023
Thank you. Does this affect 5.4 or 6.3 as well? If that's the case, please retarget your PR. |
ContributorAuthor
PhilETaylor commentedSep 6, 2023
looks like it needs to be 5.4.
|
ad10b00 to5f260a3Compare5f260a3 tob4190dbCompareb4190db to809cf74CompareContributorAuthor
PhilETaylor commentedSep 6, 2023
its late and my eyes sleepy, but I think Ive downgraded it ok, and removed short function syntax in my original to be compatible with PHP 7 also. Should be ok to test now. |
javiereguiluz approved these changesSep 11, 2023
Member
javiereguiluz left a comment
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.
It's very important to update Symfony for this BC-break in PHP 8.3. Thanks Phil for taking care of this!
stof approved these changesSep 11, 2023
lyrixx approved these changesSep 11, 2023
Member
nicolas-grekas commentedSep 11, 2023
Thank you@PhilETaylor. |
This was referencedJun 9, 2024
fabpot added a commit that referenced this pull requestJun 15, 2024
… PHP 8.3 (tscni)This PR was merged into the 5.4 branch.Discussion----------[ErrorHandler] Fix rendered exception code highlighting on PHP 8.3| Q | A| ------------- | ---| Branch? | 5.4| Bug fix? | yes| New feature? | no| Deprecations? | no| Issues |Fix#57354| License | MIT#51586 made some mistakes when fixing the use of `highlight_file()` for PHP 8.3:- It assumed that the inner `<span>` were changed to `<code>`- It did not properly adjust the pattern for `\n` when splitting the highlighting across multiple lines- It replaced all spaces with ` `, including those inside tags, breaking the highlighting entirelyThe first two are easy to fix.The latter one not so much without CSS adjustments. But just changing `white-space: nowrap` to `white-space: pre` would remove the need for that. I'm a bit worried about side effects though and I'm not sure if `CodeExtension` uses separate styling somewhere, but I can't find anything problematic at least.A test would be a bit cumbersome to add for this, so unless very much preferred I'd rather not spend the time on it.But at least in manual tests this resolved all the highlighting issues.Commits-------2b46e2a [ErrorHandler] Fix rendered exception code highlighting on PHP 8.3
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading.Please reload this page.
PHP 8.3 changes the output from the
highlight_filefunction as described here:https://php.watch/versions/8.3/highlight_file-highlight_string-html-changesI accidentally ran into this head first when using PHP 8.3.0-RC1 in development with Symfony 6.4-dev when my error had no code rendered
Also affected the
{{ '/path/to/file.html.twig'|file_excerpt(140,5) }}twig codefile_exceptrenderer - easier to replicate with that code just throw it into your twig file with a valid/path/to/file.html.twig- before this PR there would be no output, after this PR the file except would be shown.Same fix applied to both files.
After this fix the code rendered returned.