fix(lsp): context is singleton in vscode#4722
Conversation
04201bc to
27a2fae
Compare
27a2fae to
b720d5f
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the behavior of the LSP context by ensuring it is treated as a singleton in VSCode. The changes update context creation, context reloading in document events, and ensure consistency when a new context is initialized.
| if self.lsp_context is None: | ||
| context = self.context_class(paths=paths) | ||
| else: | ||
| self.lsp_context.context.load() | ||
| context = self.lsp_context.context | ||
| self.lsp_context = LSPContext(context) | ||
| return self.lsp_context |
There was a problem hiding this comment.
The current logic re-wraps the context in a new LSPContext even when one exists. To fully honor the singleton pattern, consider reusing the existing LSPContext instance when no state updates are required.
| @self.server.feature(types.TEXT_DOCUMENT_DID_CHANGE) | ||
| def did_change(ls: LanguageServer, params: types.DidChangeTextDocumentParams) -> None: | ||
| if self.lsp_context is None: | ||
| current_path = Path.cwd() |
There was a problem hiding this comment.
Using Path.cwd() might be fragile if the server's working directory does not reflect the intended project folder. Consider deriving the path from the document URI or a configuration parameter to ensure the correct context is used.
| current_path = Path.cwd() | |
| current_path = self._get_workspace_folder_or_default() |
| context = self.lsp_context | ||
| context.context.load() # Reload or refresh context | ||
| self.lsp_context = LSPContext(context.context) | ||
| self.lsp_context.context.load() |
There was a problem hiding this comment.
[nitpick] The context reinitialization logic is similar to what is done in _create_lsp_context. Consider refactoring this duplication into a helper method to ensure consistent behavior and easier maintenance.
No description provided.