Generate server methods based on `lsprotocol` type definitions #489

alcarney · 2024-08-14T19:20:28Z

#487 introduced some breaking changes, so this PR is taking the opportunity to make a few more that were going to be part of #418.

Similar to the BaseLanguageClient, this PR updates the code generation script to now also produce a BaseLanguageServer with all the server side methods generated based on the type definitions provided by lsprotocol

Not only does this improve consistency and remove the need for the duplicated methods on the LanguageServerProtocol class (Closes #306), we also get all the server side methods we were missing!

Aside from the obvious naming/argument changes, this also introduces the following changes (which we will need to document in a migration guide)

server.progress now sends a $/progress notification, rather than giving access to pygls' Progress helper. The helper is now accessed via server.work_done_progress
The underlying protocol class is now accessed via server.protocol rather than server.lsp.

Finally, the base Server class has been tidied up a little

Removed some LSP specific quriks (e.g. sync_kind)
The report_server_error code has been pushed down into the base
The feature, thread and command registration code has been pushed down into the base

TODO

Merge Update to latest lsprotocol v2024.0.0a2 #487
Fix tests and example servers
Identify and document all breaking changes

Code review checklist (for code reviewer to complete)

Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR)
Title summarizes what is changing
Commit messages are meaningful (see this for details)
Tests have been included and/or updated, as appropriate
Docstrings have been included and/or updated, as appropriate
Standalone docs have been updated accordingly

Automated linters

You can run the lints that are run on CI locally with:

poetry install --all-extras --with dev
poetry run poe lint

alcarney · 2024-08-20T20:19:37Z

Added an initial draft of the migration guide, there are a few more changes we can make (e.g. removing deprecated methods) so I expect it will take a few more revisions to get right.

So this should be ready for a review now.

That said, the lint job has started failing and I'm not entirely sure why...

Run source $VENV
/home/runner/work/_temp/0f22c65c-8345-4c8d-8709-e3f4d68c7c6b.sh: /home/runner/work/pygls/pygls/.venv/bin/poe: /home/runner/work/pygls/pygls/.venv/bin/python: bad interpreter: No such file or directory
Error: Process completed with exit code 126.

tombh

Such great work again ❤️ So many improvements, detailed docs and fixing old issues 💪

Do the new docs get automatically published on merge to main? I can't remember now. It might be good to have an update in our main README about how we're looking for beta testers for the breaking changes and a link to the migration docs?

tombh · 2024-08-21T14:11:11Z

scripts/check_client_is_uptodate.py

-        "2. Commit"
+        (
+            "🔴 Pygls client or server not up to date\n"
+            "1. Re-generate with: `poetry run poe generate_client`\n"


It's generate_client_server now right?

tombh · 2024-08-21T14:13:17Z

pyproject.toml

@@ -58,7 +58,7 @@ ruff = "ruff check ."
 mypy = "mypy -p pygls"
 check_generated_client = "python scripts/check_client_is_uptodate.py"
 check_commit_style = "npx commitlint --from origin/main --to HEAD --verbose --config commitlintrc.yaml"
-generate_client = "python scripts/generate_client.py --output pygls/lsp/client.py"
+generate_client = "python scripts/generate_client_server.py pygls/lsp"


Should we rename the command to generate_client_server?

I've renamed it and the script to generate_code - seems to complement the check_generated_code_is_uptodate.py script?

tombh · 2024-08-21T14:14:07Z

scripts/check_client_is_uptodate.py

Should we rename this to check_generated_code_is_uptodate.py?

tombh · 2024-08-21T14:47:27Z

I have a PR for the failing CI issue: #491

Similarly to the `BaseLanguageClient` this commit now introduces a `BaseLanguageServer` with methods automatically generated from type definitions provided by `lsprotocol`

This commit refactors the `LanguageServer` class to be based on the new `BaseLanguageServer`. The server has also been moved into the `pygls.lsp` module in an attempt to be more consistent with the client side code. It also takes the opportunity to tidy a few things up - Pushing generic server features such as error reporting and feature registration down into the base `Server` class - Removing LSP specific quirks from the base class e.g. `TextDocumentSyncKind` - Renames `server.lsp` to `server.protocol` to align better with the client code

alcarney · 2024-08-22T19:19:29Z

Do the new docs get automatically published on merge to main?

Yes, https://pygls.readthedocs.io/en/latest/ points to main

It might be good to have an update in our main README about how we're looking for beta testers for the breaking changes and a link to the migration docs?

I've added a note to the README, though there are a few more changes we should make before shouting too loudly about it :)

tombh

Nice! LGTM 🚀

alcarney force-pushed the generated-server branch 6 times, most recently from 368fbb3 to 5ad2d52 Compare August 20, 2024 20:10

alcarney marked this pull request as ready for review August 20, 2024 20:19

alcarney requested a review from tombh August 20, 2024 20:19

tombh mentioned this pull request Aug 21, 2024

ci: use Python version in cache key #491

Merged

6 tasks

tombh requested changes Aug 21, 2024

View reviewed changes

alcarney added 7 commits August 22, 2024 20:15

feat: generate server methods from lsp types

d87b95f

Similarly to the `BaseLanguageClient` this commit now introduces a `BaseLanguageServer` with methods automatically generated from type definitions provided by `lsprotocol`

chore: remove redundant method definitions

652a34a

chore: align to new server implementation

8f2365f

chore: align tests with new server implementation

358a5be

docs: add draft v2 migration guide

ded4a0a

chore: rename scripts to generate_code

c5155c7

alcarney force-pushed the generated-server branch from 5ad2d52 to 68eb120 Compare August 22, 2024 19:15

alcarney added 2 commits August 22, 2024 20:21

refactor: rename Server to JsonRPCServer

75f781d

chore: add call for testing to README

f043d9a

alcarney force-pushed the generated-server branch from 68eb120 to f043d9a Compare August 22, 2024 19:21

alcarney requested a review from tombh August 22, 2024 19:27

tombh approved these changes Aug 22, 2024

View reviewed changes

alcarney merged commit d221533 into openlawlibrary:main Aug 22, 2024
16 checks passed

alcarney deleted the generated-server branch August 22, 2024 22:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generate server methods based on `lsprotocol` type definitions #489

Generate server methods based on `lsprotocol` type definitions #489

alcarney commented Aug 14, 2024 •

edited

Loading

alcarney commented Aug 20, 2024

tombh left a comment

tombh Aug 21, 2024

tombh Aug 21, 2024

alcarney Aug 22, 2024

tombh Aug 21, 2024

alcarney Aug 22, 2024

tombh commented Aug 21, 2024

alcarney commented Aug 22, 2024

tombh left a comment

Generate server methods based on lsprotocol type definitions #489

Generate server methods based on lsprotocol type definitions #489

Conversation

alcarney commented Aug 14, 2024 • edited Loading

Code review checklist (for code reviewer to complete)

Automated linters

alcarney commented Aug 20, 2024

tombh left a comment

Choose a reason for hiding this comment

tombh Aug 21, 2024

Choose a reason for hiding this comment

tombh Aug 21, 2024

Choose a reason for hiding this comment

alcarney Aug 22, 2024

Choose a reason for hiding this comment

tombh Aug 21, 2024

Choose a reason for hiding this comment

alcarney Aug 22, 2024

Choose a reason for hiding this comment

tombh commented Aug 21, 2024

alcarney commented Aug 22, 2024

tombh left a comment

Choose a reason for hiding this comment

Generate server methods based on `lsprotocol` type definitions #489

Generate server methods based on `lsprotocol` type definitions #489

alcarney commented Aug 14, 2024 •

edited

Loading