Use PHPStan's `editorUrl` and add example on how to use Symfony's Console Hyperlinks #18

ruudk · 2021-12-12T11:13:16Z

Thanks for this simple but amazing error formatter.

I noticed that clicking the file path in my terminal (iTerm2) would open my default editor (VS Code) instead of my configured editorUrl PHPStorm.

In this PR I added the editorUrl so that it's available inside the formatter. The file and line are replaced and then the editorUrl variable is available in the formatter.

It uses the same replace format as PHPStan does, see: https://github.com/phpstan/phpstan-src/blob/0f2fbfff2d861e17345981f2c02ab7696a2298e3/src/Command/ErrorFormatter/TableErrorFormatter.php#L86

Together with Symfony's Console Hyperlinks you can now easily make the paths clickable for almost all terminals.

parameters:
	editorUrl: 'phpstorm://open?file=%%file%%&line=%%line%%'
	compact:
		format: "<href={editorUrl}>{path}:{line}</>\n ↳ {error}" # default

ruudk · 2021-12-18T08:38:28Z

@jiripudil What do you think?

jiripudil · 2021-12-20T08:14:06Z

src/CompactErrorFormatter.php

 	)
 	{
 		$this->relativePathHelper = $relativePathHelper;
 		$this->format = $format;
+		$this->editorUrl = $editorUrl ?? '';


What happens if this falls back to empty string? I presume symfony prints the escape sequence with an empty href – are terminals ok with it?

I would assume, that null variant cannot happen, therefore it should be removed. Because there should be a reasonable default. In case it is missing, maybe write a notice into stderr output?

https://github.com/phpstan/phpstan-src/blob/fed28bc6a9d413c373dda0a8fc598c7ff8035977/conf/config.neon#L280

I think this is fine as it is. If you want to use the editorUrl then you already know that you need to set it (as per readme). Also, if you are using a custom error formatter, you kind of know what you are doing and also test it.

Please add the error message to phpstan output & reasonable default when editorUrl is null.

jiripudil · 2021-12-20T08:14:39Z

Hi, lgtm! @jkuchar what do you think?

jkuchar · 2021-12-20T08:42:09Z

src/CompactErrorFormatter.php

@@ -48,7 +53,8 @@ public function formatErrors(
 					$this->format,
 					[
 						'{absolutePath}' => $absolutePath,
-						'{path}' => $this->relativePathHelper->getRelativePath($fileSpecificError->getFile()),
+                        '{editorUrl}' => str_replace(['%file%', '%line%'], [$absolutePath, $fileSpecificError->getLine() ?? ''], $this->editorUrl),


Please extract into closure/function as parametr expansion can be separated:

$formatEditorUrl = fn(string $path, string $line) => str_replace(['%file%', '%line%'], [$path, $line], $this->editorUrl); '{editorUrl}' => $formatEditorUrl($absolutePath, (string) ($fileSpecificError->getLine() ?? ''));

I decided to not use this package anymore, and use my own formatter because I wanted to do more modifications. Therefore I have no interest in updating the PR anymore. Should I close it? Or do we merge this as is?

I also don't agree with that the proposed change is a better one tbh.

What are those modifications? Feel free to open another PR for them.

Color highlighting phpstan/phpstan-src#841 (comment)
Very opinionated and not in the scope of this library I would say.

I have no problem with keeping this the way it is. I idea here is to keep the formatting logic separated. No big deal.

jkuchar

@jiripudil Could you please fix the null param & merge this?

jkuchar · 2021-12-20T08:45:24Z

src/CompactErrorFormatter.php

 	)
 	{
 		$this->relativePathHelper = $relativePathHelper;
 		$this->format = $format;
+		$this->editorUrl = $editorUrl ?? '';


Please add the error message to phpstan output & reasonable default when editorUrl is null.

jkuchar · 2021-12-20T08:46:28Z

src/CompactErrorFormatter.php

@@ -48,7 +53,8 @@ public function formatErrors(
 					$this->format,
 					[
 						'{absolutePath}' => $absolutePath,
-						'{path}' => $this->relativePathHelper->getRelativePath($fileSpecificError->getFile()),
+                        '{editorUrl}' => str_replace(['%file%', '%line%'], [$absolutePath, $fileSpecificError->getLine() ?? ''], $this->editorUrl),


What are those modifications? Feel free to open another PR for them.

ruudk added 2 commits December 12, 2021 12:04

Use editorUrl parameter

c97f943

Update readme

9348050

jiripudil reviewed Dec 20, 2021

View reviewed changes

jiripudil requested a review from jkuchar December 20, 2021 08:14

jkuchar reviewed Dec 20, 2021

View reviewed changes

jkuchar requested changes Dec 20, 2021

View reviewed changes

ruudk closed this Jan 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Use PHPStan's `editorUrl` and add example on how to use Symfony's Console Hyperlinks #18

Use PHPStan's `editorUrl` and add example on how to use Symfony's Console Hyperlinks #18

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Use PHPStan's editorUrl and add example on how to use Symfony's Console Hyperlinks #18

Use PHPStan's editorUrl and add example on how to use Symfony's Console Hyperlinks #18

Uh oh!

Conversation

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Use PHPStan's `editorUrl` and add example on how to use Symfony's Console Hyperlinks #18

Use PHPStan's `editorUrl` and add example on how to use Symfony's Console Hyperlinks #18