Skip to content
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

Improve exception renderers #422

Merged
merged 21 commits into from
Feb 9, 2025
Merged

Improve exception renderers #422

merged 21 commits into from
Feb 9, 2025

Conversation

mvorisek
Copy link
Member

@mvorisek mvorisek commented Feb 9, 2025

improve #421

@mvorisek mvorisek changed the title Improve exception render Improve exception renderers Feb 9, 2025
@mvorisek mvorisek merged commit fe0073c into develop Feb 9, 2025
31 checks passed
@mvorisek mvorisek deleted the ex_render_improve branch February 9, 2025 12:58

private function optimizeText(string $value): string
{
$res = preg_replace("~\e\\[\\d{1,2}m\e\\[0m~", '', $value);
Copy link
Member

@DarkSide666 DarkSide666 Feb 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be nice to add small comment here about what these regexes does
first removes empty sequences, but second ...

@@ -268,14 +268,12 @@ protected function makeRelativePath(string $path): string

protected function tryRelativizePathsInString(string $str): string
{
$str = preg_replace_callback('~(?<!\w)(?:[/\\\]|[a-z]:)\w?+[^:"\',;]*?\.php(?!\w)~i', function ($matches) {
return preg_replace_callback('~(?<!\w)(?:[/\\\]|[a-z]:)\w?+[^:"\',;]*?\.php(?!\w)~i', function ($matches) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be nice to add comment about what this regex does

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK, this is even untested (exact behaviour is unasserted), thus help welcomed. It is a little problematic as the repo can be part of another repo and the tests should still pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants