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

Remove "test" ExecuteCommandOption #10094

Closed
limarkxx opened this issue Aug 9, 2023 · 6 comments · Fixed by #10108
Closed

Remove "test" ExecuteCommandOption #10094

limarkxx opened this issue Aug 9, 2023 · 6 comments · Fixed by #10108

Comments

@limarkxx
Copy link

limarkxx commented Aug 9, 2023

At https://github.com/vimeo/psalm/blob/master/src/Psalm/Internal/LanguageServer/LanguageServer.php#L422
there is $serverCapabilities->executeCommandProvider = new ExecuteCommandOptions(['test']);, what appears to be a test-call of the workspace/executeCommand LSP feature.
This doesn't affect the first LSP in VSCode, but if you have multiple instances the subsequent LSP clients fail during initialization with

[Error - 9:23:14 PM] Server initialization failed.
Error: command 'test' already exists
	at a.registerCommand (c:\Program Files\Microsoft VS Code\resources\app\out\vs\workbench\api\node\extensionHostProcess.js:102:76383)
	at Object.registerCommand (c:\Program Files\Microsoft VS Code\resources\app\out\vs\workbench\api\node\extensionHostProcess.js:120:20180)
@psalm-github-bot
Copy link

Hey @limarkxx, can you reproduce the issue on https://psalm.dev ?

@limarkxx
Copy link
Author

limarkxx commented Aug 9, 2023

Hey @limarkxx, can you reproduce the issue on https://psalm.dev ?

It's not possible to instantiate multiple LSP instances there.

@tm1000
Copy link
Contributor

tm1000 commented Aug 9, 2023

@limarkxx I think we can just remove that. Perhaps it was left by me?

If that line is removed does it fail in another place?

@limarkxx
Copy link
Author

limarkxx commented Aug 9, 2023

@weirdan
Copy link
Collaborator

weirdan commented Aug 9, 2023

I don't think Fix all ever worked in LSP context.

@tm1000
Copy link
Contributor

tm1000 commented Aug 9, 2023

It did not that was me trying to figure out how to get it to work though it shouldnt even show up. Regardless it should be removed along with the test command

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

Successfully merging a pull request may close this issue.

3 participants