-
Notifications
You must be signed in to change notification settings - Fork 653
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
Map LSP paths #8026
Map LSP paths #8026
Conversation
4c16ee9
to
d1e58b4
Compare
4.x branch is closed now as we prepare for the 5.0 release. Please target the master branch instead. |
On your own PR :D nice! |
Closing as this targets 4.x which is closed. Please reopen on master going forward |
@weirdan are you planning on targeting the master branch with your original merge request? |
c1457b4
to
8a98d97
Compare
8a98d97
to
e4607d8
Compare
@m-graham are you using this branch? |
Hi @weirdan, I work from within a dev container and realized after sending my previous message that I could still use the VSCode plugin by specifying that it uses a TCP connection. I'm not actively using this branch. |
@tm1000 as our LSP expert, do you think it would be better to use the following approach instead?
This works for me with my local patched Psalm, and has an added benefit of not requiring any configuration. |
So technically I think we could support multiple workspace folders from the extension but it would just launch separate Psalm instances for each workspace. Thus I think what you propose still works
So I think this works. The way I'm doing it now for all of my docker containers is I actually have psalm running outside of the container so it runs locally on the system. I'm not saying that works for for everyone. But that way I dont run into docker networking issues like this. I like that you don't have to configure anything but I believe someone out there will want/need to be able to configure that. Is it possible to allow that? I think bare minimum it needs to be configurable, not that they need to change it from day 1 |
I believe that's already the case, see psalm/psalm-vscode-plugin#184
Possibly. I'm wouldn't go with the config file approach (as in this PR) though, it's too much hassle to set up for end users. The way I see it, it would be limited to LSP use case, so either a CLI switch (e.g. This won't immediately allow running Psalm in Docker containers with |
Oh. and I merged it too! I forgot
The CLI switch is fine as usually most of those I then store in the Configuration class in the LSP itself which means they are easily changeable on the fly from vscode (or other LSPs)
Yes this is true it will require additional settings as I have seen in a few other plugins/extensions. I will work on that in the upcoming weeks along with sending configuration now that we support that in the LSP itself |
Superseded by #10033 |
Allows mapping of paths passed over LSP protocol. This is useful where you need
to run Psalm language server inside a container where project paths differ from
those on host machine.
Partly addresses #6919 (for LSP only)