[wip] refactor python language server #10908
Conversation
|
E2E Tests 🚀 |
austin3dickey
force-pushed
the
aus/pls
branch
from
e253979 to
82a7c52
Compare
There was a problem hiding this comment.
Pull request overview
This PR refactors the Python language server implementation by replacing the jedi-language-server dependency with a custom lightweight LSP server implementation. The new server maintains Positron-specific features (namespace-aware completions, DataFrame/Series column completions, magic commands, help topics, syntax diagnostics) while delegating static analysis features to third-party extensions like Pylance.
Key changes:
- Removed jedi-language-server, jedi, and parso dependencies and vendored code
- Created new
positron_lsp.pyimplementing a minimal LSP server using pygls directly - Removed complex Jedi patches and integration code
- Updated dependency versions (pygls 2.0.0, lsprotocol 2025.0.0)
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
scripts/patches/*.patch |
Removed patch files for jedi, parso, and jedi-language-server |
python_files/run-jedi-language-server.py |
Added type ignore comment for import |
python_files/positron_requirements/requirements.txt |
Updated dependencies, removed jedi-language-server, jedi, parso |
python_files/positron_requirements/requirements.in |
Removed jedi-language-server, upgraded pygls to 2.0.0 |
python_files/posit/positron_language_server.py |
Updated import from positron_jedilsp to positron_lsp |
python_files/posit/positron/tests/test_positron_lsp.py |
New test file for the refactored LSP server |
python_files/posit/positron/tests/test_positron_jedilsp.py |
Removed old test file |
python_files/posit/positron/positron_lsp.py |
New LSP server implementation |
python_files/posit/positron/positron_jedilsp.py |
Removed old LSP server implementation |
python_files/posit/positron/lsp.py |
Updated import from positron_jedilsp to positron_lsp |
python_files/posit/positron/jedi.py |
Removed Jedi patches and integration code |
extensions/positron-python/python_files/posit/positron/positron_lsp.py
Outdated
Show resolved
Hide resolved
extensions/positron-python/python_files/posit/positron/positron_lsp.py
Outdated
Show resolved
Hide resolved
extensions/positron-python/python_files/posit/positron/positron_lsp.py
Outdated
Show resolved
Hide resolved
seeM
left a comment
seeM
left a comment
There was a problem hiding this comment.
I took a look at the code and it looks like a great start!
extensions/positron-python/python_files/run-jedi-language-server.py
Outdated
Show resolved
Hide resolved
| self._messages_to_handle: list[Any] = [] | ||
|
|
||
| @lru_cache # noqa: B019 | ||
| def get_message_type(self, method: str) -> type | None: |
There was a problem hiding this comment.
Is this pygls' recommended way to add methods? I wonder if there's something neater now that we're not depending on JediLanguageServerProtocol
| init_opts = PositronInitializationOptions() | ||
|
|
||
| # Store the working directory (using params.root_path since workspace may not be initialized yet) | ||
| server._working_directory = init_opts.working_directory or params.root_path # noqa: SLF001 |
There was a problem hiding this comment.
I also wonder if there's another pygls recommended way
| # Yield to parent implementation which handles workspace setup | ||
| return (yield from super().lsp_initialize(params)) | ||
|
|
||
| def _data_received(self, data: bytes) -> None: # type: ignore[override] |
There was a problem hiding this comment.
I don't think we need this anymore, and may want to try to make our server async
| # Cache for magic completions | ||
| self._magic_completions: dict[str, tuple] = {} | ||
|
|
||
| def start_tcp(self, host: str) -> None: |
There was a problem hiding this comment.
It's possible we don't need all of this in the latest pygls. A lot of this was because pygls didn't let you use your own loop, which we needed to do
| elif trimmed_line.startswith((_LINE_MAGIC_PREFIX, _CELL_MAGIC_PREFIX)): | ||
| # Magic command completion only | ||
| pass # Will add magics below | ||
| else: |
There was a problem hiding this comment.
I haven't checked, are we providing the right completions (if any) inside string literals? Do we provide path completions?
extensions/positron-python/python_files/posit/positron/positron_lsp.py
Outdated
Show resolved
Hide resolved
extensions/positron-python/python_files/posit/positron/positron_lsp.py
Outdated
Show resolved
Hide resolved
extensions/positron-python/python_files/posit/positron/positron_lsp.py
Outdated
Show resolved
Hide resolved
austin3dickey
force-pushed
the
aus/pls
branch
from
3caa0d0 to
6f74801
Compare
Release Notes
New Features
Bug Fixes
QA Notes
@:console @:help @:notebooks @:positron-notebooks @:outline @:problems @:references @:web @:win