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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for relative paths in editorUrl #1414

Merged
merged 5 commits into from Jun 18, 2022

Conversation

Wirone
Copy link
Contributor

@Wirone Wirone commented Jun 11, 2022

This allows running PHPStan within Docker environment and output errors with links to files on the host (editorUrl must be configured using actual host's path to the project + %rel_file%).

Fixes phpstan/phpstan#7043


I decided to use %rel_file% because there was RelativePathHelper already in the formatter, so it was almost non-invasive. Other options would require much more work and many other files modified.

I wanted to add test for it, but honestly I don't know how to do it 馃 I created test similar to TableErrorFormatterTest::testEditorUrlWithTrait() but flow of outputting errors does not allow to check if href added to message is correct - it's missing in the output retrieved with $this->getOutputContent(). Anyway, I was able to test it with Docker runtime created locally and it works:

# phpstan.neon
includes:
	- phpstan.neon.dist

parameters:
	editorUrl: 'phpstorm://open?file=/Volumes/Projects/~Github/phpstan/phpstan-src/%%rel_file%%&line=%%line%%'

image

Screenshot shows PHPStan analysis within Docker container, at the bottom you can see tooltip with href that is shown when mouse cursor is over the activated link - it's proper host's path, not container's one (which is /etc/phpstan/...).

FYI: ~/Dev is a symlink to /Volumes/Projects/, if CLI prompt looks suspicious 馃槈

@Wirone Wirone force-pushed the issue-7043-rel-path-in-editor-url branch from c7f7912 to 723ba37 Compare June 11, 2022 23:22
Wirone added a commit to Wirone/phpstan that referenced this pull request Jun 11, 2022
Copy link
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

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

  1. Needs a test
  2. It's not going to work with the default relativePathHelper configured for TableErrorFormatter (FuzzyRelativePathHelper). It has more complex logic which doesn't always result in relative path starting at CWD. You need to inject simpleRelativePathHelper there as a 2nd one and use that.
  3. I don't like rel_file, all names are camelCaps now.

@Wirone
Copy link
Contributor Author

Wirone commented Jun 12, 2022

@ondrejmirtes how would you like to test it? Because when I debug it:

image

I see that there's no <href> in the output, so I don't know how to test if relative path was used for generating link 馃 Is it trimmed somewhere in between?

EDIT: Decorated output stream did the trick 馃檪

This allows running PHPStan within Docker environment and output errors with links to files on the host (`editorUrl` must be configured using actual host's path to the project + `%rel_file%`).

Fixes phpstan/phpstan#7043
@Wirone Wirone force-pushed the issue-7043-rel-path-in-editor-url branch from 723ba37 to 7d030de Compare June 15, 2022 22:13
@Wirone Wirone requested a review from ondrejmirtes June 15, 2022 22:14
@ondrejmirtes ondrejmirtes merged commit e61a782 into phpstan:1.7.x Jun 18, 2022
@ondrejmirtes
Copy link
Member

Thank you very much! I finally got it and it's nice!

@Wirone Wirone deleted the issue-7043-rel-path-in-editor-url branch June 18, 2022 09:20
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