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

fix: clickable editorUrl #1663

Closed
wants to merge 5 commits into from
Closed

Conversation

Chris53897
Copy link

phpstan/phpstan#7796

Tests are now green.
But i am not sure how to test in correct in real App.

@ondrejmirtes
Copy link
Member

This is wrong syntax for symfony/console: https://symfony.com/blog/new-in-symfony-4-3-console-hyperlinks

What's the foundation behind your change?

@Chris53897
Copy link
Author

Chris53897 commented Aug 30, 2022

Thanks for the link. Maybe the missing " will already fix the linked issue? Or removing the first "

@ondrejmirtes
Copy link
Member

That's up to you to test :)

@Chris53897
Copy link
Author

I will try. If i run the UnitTest for the particular method it works. But with make tests it fails. So i am a bit confused what the difference is.

@Chris53897
Copy link
Author

If your terminal does not support hyperlinks, they will be rendered as normal and non-clickable text and you won't see their URLs. That's why it's recommended to check out the growing [list of terminal emulators that support hyperlinks](https://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda) before using this feature in your own apps and commands.

This is the reason of the failing in terminals that do not support the links.

This pseudo code would be correct in my eyes if there is a possibility to detect if the actual terminal supports links.

if($console->supportsLinks())
{
 $message .= "\n✏️  <href=" . OutputFormatter::escape($url) . ">" . $this->relativePathHelper->getRelativePath($editorFile) . "</>";
}
else {
 $message .= "\n✏️  <href=" . OutputFormatter::escape($url) . ">" . OutputFormatter::escape($url) . "</>";
}

But i do not know if it is possible to detect this.

We have a tradeoff here. With the latest version of this PR, ...

  1. Teminal with Link-Support: The link-Text is now longer (same as Target). Could lead to display-problems
  2. Teminal without Link-Support: Only Link-Text but now it can be clicked. Could lead to display-problems

@ondrejmirtes
Copy link
Member

There's now editorUrlTitle config option which you can use to fix the clickability: https://phpstan.org/user-guide/output-format#opening-file-in-an-editor

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