Remove extra didOpen message sent from our end #7173
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Kudos, SonarCloud Quality Gate passed!
|
AdamYoblick
approved these changes
Sep 27, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #7115
Pylance used to require a new open notification after a workspace folder change (not sure if it still requires now). The code in
PythonLanguageClient.cswas a requirement of pylance to force a reopen on all documents whenever the current workspace folder changed (opening a solution would provide the base folder for the workspace).This makes sense in VSCode, because VS Code allows you to open multiple folders at a time. So adding or removing WorkspaceFolders makes sense there. But in VS, it does not support opening multiple workspace folders at the same time. When you close the current workspace (i.e. close all open folders) and open a new one in VS, we just start a new instance of Pylance.
The LSP spec says that the
workspace/didChangeWorkspaceFoldersshould be sent for "workspace folder configuration changes" which is not very specific. But thedidChangeWorkspaceFoldermessage only contains added/removed arrays of WorkspaceFolder objects, and those objects just contain the folder uri and folder name (to show in UI), so I can't think of another scenario where the message would need to be sent other than opening/closing a solution/workspace.LSP Client already sends at least one didOpen anytime documents are opened, so it should be ok to remove the extra didOpen from PTVS. It can be sent in whichever of these two places is hit first:
https://devdiv.visualstudio.com/DevDiv/_git/VSLanguageServerClient?path=/src/product/RemoteLanguage/Impl/Infrastructure/LspProjectionSupportService.cs&version=GBdevelop&line=101&lineEnd=101&lineStartColumn=1&lineEndColumn=120&lineStyle=plain&_a=contents
https://devdiv.visualstudio.com/DevDiv/_git/VSLanguageServerClient?path=/src/product/RemoteLanguage/Impl/Infrastructure/TextSynchronization/LanguageServerBufferManager.cs&version=GBdevelop&line=95&lineEnd=95&lineStartColumn=1&lineEndColumn=90&lineStyle=plain&_a=contents
Also tested the change in both open folder and project view scenarios, pylance still works well after the change.