Skip to content

[Console] Fix Helper::removeDecoration hyperlink bug #47779

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

Merged
merged 1 commit into from
Oct 4, 2022

Conversation

greew
Copy link
Contributor

@greew greew commented Oct 3, 2022

Q A
Branch? 4.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #47769 , fix #45521
License MIT
Doc PR none

Helper::removeDecoration() does correctly remove colors and such from the text, but terminal hyperlinks are not removed here. This causes Helper::width() and Helper::length() to report wrong values, as the number of visual characters it not the same as the number of characters in the text.

This PR

See https://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda for information about hyperlinks in terminals

@greew
Copy link
Contributor Author

greew commented Oct 4, 2022

fabbot.io tells me that there are two code style fixes somewhere else than in the code, I've added.

Should I fix these as well or just leave them?

@fabpot
Copy link
Member

fabpot commented Oct 4, 2022

Thank you @greew.

@fabpot fabpot merged commit 216b339 into symfony:4.4 Oct 4, 2022
@greew
Copy link
Contributor Author

greew commented Oct 4, 2022

@fabpot The bug is also present in 5.x and 6.x - do you merge the bug up to these branches yourself or should I create PRs for those as well?

@greew greew deleted the ticket_47769 branch October 4, 2022 07:18
@greew
Copy link
Contributor Author

greew commented Oct 4, 2022

@fabpot The bug is also present in 5.x and 6.x - do you merge the bug up to these branches yourself or should I create PRs for those as well?

Nevermind - I found the answer myself :)

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

Successfully merging this pull request may close these issues.

3 participants