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

Update example vscode extension #357

Merged
merged 7 commits into from
Aug 29, 2023
Merged

Conversation

alcarney
Copy link
Collaborator

@alcarney alcarney commented Aug 6, 2023

Description (e.g. "Related to ...", etc.)

The example json-vscode-extension needs an update - at the very least to enable support for the new LSP v3.17 methods.

However, now that the vscode-python-tools-extension-template exists I'd argue that we don't need to worry too much about showing people the "right" way to integrate a pygls powered language server into VSCode and we can take it in a slightly different direction.

Basically I'm going to argue for converting it into a "playground"!

This PR is the start of taking things in that direction (which helped me implement #356) but I thought I'd check in before going all the way with this. Ultimately I'd like to

  • Drop the fountain-vscode-extension and focus on json-vscode-extension - maintaining one extension is enough!
  • Refactor the json-server into a single file, and put it in examples/servers/
  • Rather than having to provide a python interpreter manually, use @vscode/python-extension to use the Python interpreter configured in the official Python extension (this bit is already implemented)
  • Try and expose as many options as possible (server path, document selectors etc.) so that people can just play with the example servers, rather than having to figure out how to recompile the extension etc.
  • See all the TODOs in extension.ts for an idea of the potential options

Let me know your thoughts! 😄

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

@tombh
Copy link
Collaborator

tombh commented Aug 7, 2023

This sounds awesome! Some well-needed refactors and some exciting new directions.

So does that mean that in order to bootstrap VSCode-based language server we'll point to vscode-python-tools-extension-template first and our json-vscode-extension as extra inspiration? Either way i think it's good.

@alcarney
Copy link
Collaborator Author

alcarney commented Aug 7, 2023

So does that mean that in order to bootstrap VSCode-based language server we'll point to vscode-python-tools-extension-template first and our json-vscode-extension as extra inspiration?

I'm hoping that our extension will be simple enough that is can remain a single *.ts file and serve as a useful "here's just enough to get you going" reference.

Then point people to vscode-python-tools-extension-template as an example of how to "productionise" and distribute your extension since it covers steps like bundling the server.

@tombh
Copy link
Collaborator

tombh commented Aug 8, 2023

Ah right, yeah that sounds great.

@alcarney alcarney force-pushed the update-vscode branch 3 times, most recently from 85d8327 to b7b218c Compare August 14, 2023 19:44
This refactors the example server that was part of the
`json-vscode-extension` into a single file that now lives in the
`examples/servers` directory.

Additionaly, relevant test cases that were part of the unit test suite
for the json-server have been converted into end-to-end tests in the
`tests/lsp/` directory.

To support writing these and any new test cases going forward, there
is a test language client in `tests/client.py` with support for
"catching" messages like diagnostics from the server, as well as
waiting for certain notifications coming from the server.
This refactor changes the purpose of the example VSCode extension from
integrating a single language server into VSCode to providing a
playground in which to experiment with the `pygls` framework in
general.

Now the extension will

- Start the server when the user opens the first text file/notebook
- Restart the server if
  - they change Python environment
  - change a setting that affects the client/server
  - trigger it manually via the command palette
  - save the text file containing the current server's source code

Additionally, the following technology upgrades have been performed

- Upgrading `vscode-languageclient-node` to `8.1.0` - adding support
  for the new `v3.17` LSP methods
- Using `@vscode/python-extension` to grab the user's currently
  configured Python environment - rather than forcing them to
  configure it manually.
- Using a `LogOutputChannel` for all client side logging
@alcarney alcarney force-pushed the update-vscode branch 3 times, most recently from af9c62d to a5019e1 Compare August 14, 2023 23:04
@alcarney
Copy link
Collaborator Author

pylgs-playground

VSCode Extension Changes

This PR changes the purpose of the example VSCode extension from integrating a single language server into VSCode to providing a playground in which to experiment with the pygls framework in general.

There's still plenty that could be added, (see all TODOs left in src/extension.ts) but hopefully this is a good enough starting point

The extension will now

  • Start the server when the user opens the first text file/notebook
  • Restart the server if the user
    • changes Python environment
    • changes a setting that affects the client/server
    • triggers it manually via the command palette
    • saves the text file containing the current server's source code

The following config options have been introduced

  • pygls.server.cwd: controls the working directory from which servers are launched
  • pygls.server.launchScript: the python script containing the server to launch (relative to cwd)
  • pygls.trace.server: this adds a GUI option for something backed into VSCode's language client, if set to vebose the client will log every message sent/received to/from the server.
  • pygls.client.documentSelector: controls which documents the language client will send to the server

Finally, the following technology upgrades have been performed

  • Upgrading vscode-languageclient-node to 8.1.0 - adding support for the new v3.17 LSP methods
  • Using @vscode/python-extension to grab the user's currently configured Python environment - rather than forcing them to configure it manually.
  • Using a LogOutputChannel for all client side logging

Supporting changes

  • The fountain-vscode-extension has been deleted.
  • The json-vscode-extension has been refactored (see above) and renamed to vscode-playground
  • The ci pipeline for the example vscode extension has been updated.
  • The example json-server itself has been refactored into a single file and moved into examples/servers
  • The old json-server unit tests have been replaced with equivalent end-to-end tests in tests/lsp where possible
  • 8b448d8 introduces a common pattern for writing end-to-end tests - hopefully this can be extended in the future to automatically parametrize over supported transports.

@alcarney alcarney marked this pull request as ready for review August 14, 2023 23:27
@alcarney alcarney requested a review from tombh August 14, 2023 23:27
@tombh
Copy link
Collaborator

tombh commented Aug 17, 2023

This looks great! I'm still digesting it, I'll reply more soon once I've got the gist of what's here.

@tombh
Copy link
Collaborator

tombh commented Aug 20, 2023

I tried installing the playground myself, but got an error at the "Launch Client" step:

Error
rejected promise not handled within 1 second: Error: Pending response rejected since connection got disposed
stack trace: Error: Pending response rejected since connection got disposed
	at Object.dispose (/home/tombh/Workspace/pygls/examples/vscode-playground/node_modules/vscode-jsonrpc/lib/common/connection.js:1167:27)
	at Object.dispose (/home/tombh/Workspace/pygls/examples/vscode-playground/node_modules/vscode-languageclient/lib/common/client.js:1514:35)
	at LanguageClient.handleConnectionClosed (/home/tombh/Workspace/pygls/examples/vscode-playground/node_modules/vscode-languageclient/lib/common/client.js:1105:34)
	at LanguageClient.handleConnectionClosed (/home/tombh/Workspace/pygls/examples/vscode-playground/node_modules/vscode-languageclient/lib/node/main.js:180:22)
	at closeHandler (/home/tombh/Workspace/pygls/examples/vscode-playground/node_modules/vscode-languageclient/lib/common/client.js:1092:18)
	at CallbackList.invoke (/home/tombh/Workspace/pygls/examples/vscode-playground/node_modules/vscode-jsonrpc/lib/common/events.js:55:39)
	at Emitter.fire (/home/tombh/Workspace/pygls/examples/vscode-playground/node_modules/vscode-jsonrpc/lib/common/events.js:117:36)
	at closeHandler (/home/tombh/Workspace/pygls/examples/vscode-playground/node_modules/vscode-jsonrpc/lib/common/connection.js:314:26)
	at CallbackList.invoke (/home/tombh/Workspace/pygls/examples/vscode-playground/node_modules/vscode-jsonrpc/lib/common/events.js:55:39)
	at Emitter.fire (/home/tombh/Workspace/pygls/examples/vscode-playground/node_modules/vscode-jsonrpc/lib/common/events.js:117:36)
	at StreamMessageWriter.fireClose (/home/tombh/Workspace/pygls/examples/vscode-playground/node_modules/vscode-jsonrpc/lib/common/messageWriter.js:42:27)
	at Socket.<anonymous> (/home/tombh/Workspace/pygls/examples/vscode-playground/node_modules/vscode-jsonrpc/lib/common/messageWriter.js:74:42)
	at Socket.emit (node:events:513:28)
	at Pipe.<anonymous> (node:net:757:14)
	at Pipe.callbackTrampoline (node:internal/async_hooks:130:17)
rejected promise not handled within 1 second: Error: Client is not running and can't be stopped. It's current state is: starting
stack trace: Error: Client is not running and can't be stopped. It's current state is: starting
	at LanguageClient.shutdown (/home/tombh/Workspace/pygls/examples/vscode-playground/node_modules/vscode-languageclient/lib/common/client.js:914:19)
	at LanguageClient.stop (/home/tombh/Workspace/pygls/examples/vscode-playground/node_modules/vscode-languageclient/lib/common/client.js:885:21)
	at LanguageClient.stop (/home/tombh/Workspace/pygls/examples/vscode-playground/node_modules/vscode-languageclient/lib/node/main.js:150:22)
	at LanguageClient.doInitialize (/home/tombh/Workspace/pygls/examples/vscode-playground/node_modules/vscode-languageclient/lib/common/client.js:867:27)
	at async LanguageClient.start (/home/tombh/Workspace/pygls/examples/vscode-playground/node_modules/vscode-languageclient/lib/common/client.js:722:13)
	at async startLangServer (/home/tombh/Workspace/pygls/examples/vscode-playground/out/extension.js:127:9)
	at async r.value (/home/tombh/Workspace/pygls/examples/vscode-playground/out/extension.js:71:13)

I'm not a VSCode user so I could be missing something really obvious.

BTW, I also had to add the Python extension, which meant a restart of VSCode, I don't know if that's a step worth mentioning in the README?

I don't think the screenshot from the comment above is in this PR? I think it, or something similar, would be good to add. And maybe a brief description of what can be done in the playground? I assume that the idea is that a sever (say the inlay_hints.py one) can be live edited to see how it effects the sums.txt file?

Also, seeing as VSCode works in the browser, is this something that we could put online!?

@alcarney
Copy link
Collaborator Author

alcarney commented Aug 21, 2023

but got an error at the "Launch Client" step:

Hmm, not much to go on... I assume the error log you shared is from the Debug Console in the VSCode instance you are running "Launch Client" from? Have you managed to find the pygls Output Panel in the child VSCode instance "Launch Client" opens? There should hopefully be some additional info there

BTW, I also had to add the Python extension, which meant a restart of VSCode, I don't know if that's a step worth mentioning in the README?

I don't think the screenshot from #357 (comment) is in this PR? I think it, or something similar, would be good to add. And maybe a brief description of what can be done in the playground?

Good points, I'll update the README :)

I assume that the idea is that a sever (say the inlay_hints.py one) can be live edited to see how it effects the sums.txt file?

Yes

Also, seeing as VSCode works in the browser, is this something that we could put online!?

We'd have to publish it in the VSCode Marketplace... but yes at least enabling VSCode + pygls in the browser has been a dream of mine since #218

While the now deleted fountain-extension (and some experimental vesions of esbonio) kind of worked using pyodide, things have moved on and the VSCode team have opted for a the WASM-WASI runtime which should make it possible to have pygls actually read the contents of files in a project opened on the web!

Not that I think we need to get rid of pyodide! But I do have a proof of concept branch that enables WASI support for pygls (with some limitations). The trick is now figuring out how to wire it up to VSCode - but that's a different PR! 😅

@tombh
Copy link
Collaborator

tombh commented Aug 22, 2023

I got a bit further with that error. So the "Output" tab in the child VSCode instance has the debug from the vscode-playground extension, and no matter what I've tried, it's always using my system's global Python interpreter, which I see from this line: Using environment: /bin/python: /bin/python.

I've tried activating the venv (source env/bin/activate) and launching VSCode from within the new venv shell. And I've tried explicitly setting the Python interpreter from the VSCode Python: Select Interpreter config. VSCode is all still quite new to me, so I'm sure I'm missing something obvious.

Oh wow, ok, so WASM-WASI is the path forward for in-browser Python. And you've already got a proof of concept, that's amazing. I see you've got some fancy Nix stuff going on 🤓 Does that mean you're just hacking on that locally (as in outside of any browser environment), the main difference that you use cpython-wasi rather than the normal interpreter?

@alcarney
Copy link
Collaborator Author

I've tried explicitly setting the Python interpreter from the VSCode Python: Select Interpreter config

Strange... that should be the right command....

Time passes

Ah, after trying it myself I think I know what your issue might be... when you run the Select Interpreter command do you see a window like this pop up?

image

It looks like you need to choose that first option, corresponding with the examples/workspace/ folder for it to take effect.

so WASM-WASI is the path forward for in-browser Python

in VSCode at least, pyodide would still make sense in other web contexts (like Jupyterlite maybe?)

Does that mean you're just hacking on that locally (as in outside of any browser environment), the main difference that you use cpython-wasi rather than the normal interpreter?

Pretty much... though perhaps I misspoke when I said WASM-WASI was a runtime, it's actually a specification on what APIs a process running under WASI can expect to be available. wasmtime is one implementation that you can use to run cpython-wasm locally, VSCode happens to be another implementation that can work in the browser.

If you're interested this blog post covers all this better than I can 😄

I see you've got some fancy Nix stuff going on

Haha, I've been playing with it over the last year or so, very nice when I can convince it to work! 😅
I'll probably drop it when opening the PR for real though

@tombh
Copy link
Collaborator

tombh commented Aug 25, 2023

That popup is almost what I see, just the path to the venv version is different
image

But no matter what I do F5 runs a command like this:
image

I'm thinking we should just merge this, I'm sure it's just a minor issue and besides I think it'd be better to get more eyes on it from it being on main.

Ahh WASI is like POSIX. That's a nice blog post thanks. And so VSCode being based on Electron, which itself is a browser, lends itself to being a WASM runtime, and even WASI compliant. Very much looking forward to seeing where this goes!

@alcarney
Copy link
Collaborator Author

But no matter what I do F5 runs a command like this

That's strange... though just to confirm the expected behavior, once that second VSCode window is open, the language server should start once you open a file in the examples/workspace folder - no F5 required.

I've updated the README to include the screenshots in this thread and some notes about installing the Python extension and choosing the environment.

I also had to pin the version of poetry in CI, since 3.7 support was dropped recently

🤞 this is ready to merge? 🙂

@tombh
Copy link
Collaborator

tombh commented Aug 29, 2023

That's strange... though just to confirm the expected behavior, once that second VSCode window is open, the language server should start once you open a file in the examples/workspace folder - no F5 required.

Oh I suspected that might be the case

I've updated the README to include the screenshots in this thread and some notes about installing the Python extension and choosing the environment.

Great

I also had to pin the version of poetry in CI, since 3.7 support was dropped recently

Great, thanks

🤞 this is ready to merge? 🙂

Amdani!

@tombh tombh merged commit 73a08a7 into openlawlibrary:main Aug 29, 2023
@alcarney alcarney deleted the update-vscode branch August 29, 2023 21:41
@z80dev
Copy link

z80dev commented Sep 8, 2023

Unfortunately I'm having the same issue as @tombh . I'm on mac

@alcarney any chance you're not on a mac?

and @tombh any chance you are?

@alcarney
Copy link
Collaborator Author

alcarney commented Sep 8, 2023

Unfortunately I'm having the same issue as @tombh

That's frustrating, the whole point of this change was to have things Just WorkTM 😅

@alcarney any chance you're not on a mac?

I'm running Linux

To ask some of the "obvious" questions

  • Have you opened your primary VSCode window (the one you launch the extension from) inside the pygls/examples/vscode-playground folder?
  • Have you run npm run compile? (I've just noticed that the preLaunchTask in the launch.json file is commented out! 😬)
  • Which version of VSCode are you running? the API we're using requires >= 1.78.0
  • Do you have the latest version of Python extension installed? We don't require the absolute latest, but the API provided the extension is relatively new (~ a few months old)

Assuming the above doesn't fix anything and that you're launching the extension via our launch.json file, there's a chance the multi-root setup is confusing it.

Would you mind trying the following and seeing if the issue persists?

  • Copy json_server.py from the servers/ folder to the workspace/ folder
  • Comment out this line from the launch.json file, and restart the debug session
  • In the newly opened VSCode window, use the Python: Select Interpreter command to select the environment containing your pygls install (there should be no pop-up like this as you should now only have a single folder open)
  • You will have to set the pygls.server.cwd option to the path of the workspace/ folder e.g. /Users/me/Projects/pygls/examples/workspace/
  • Open a JSON file and see if it works! 🤞

@tombh
Copy link
Collaborator

tombh commented Sep 8, 2023

@z80dev I'm on Linux too
@alcarney I tried deleting that line in launch.json and at least it uses the right python executable now! But I get a different error:

2023-09-08 16:25:57.268 [info] [Error - 16:25:57] Workspace diagnostic pull failed.
2023-09-08 16:25:57.268 [info]   Message: IndexError: list index out of range
  Code: -32602 
{'traceback': ['  File "/home/tombh/Workspace/pygls/pygls/protocol.py", line 377, in _handle_request\n    self._execute_request(msg_id, handler, params)\n', '  File "/home/tombh/Workspace/pygls/pygls/protocol.py", line 299, in _execute_request\n    self._send_response(msg_id, handler(params))\n                                ^^^^^^^^^^^^^^^\n', '  File "/home/tombh/Workspace/pygls/examples/servers/json_server.py", line 113, in workspace_diagnostic\n    first = list(json_server.workspace._docs.keys())[0]\n            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^\n']}

@alcarney
Copy link
Collaborator Author

alcarney commented Sep 9, 2023

I tried deleting that line in launch.json and at least it uses the right python executable now!

Awesome! So it sounds like, the extension is not handling the multi-root setup correctly... I wonder why it managed to work for me 🤔

But I get a different error:

That looks like a bug in the workspace_diagnostic method, not handling the case where the server's workspace is empty

@tombh
Copy link
Collaborator

tombh commented Sep 10, 2023

Ohh interesting, yeah multi-root isn't so common, so makes sense.

So there's no need to support empty workspace in the example workspace right? Just good to know what's happening right?

@alcarney
Copy link
Collaborator Author

So there's no need to support empty workspace in the example workspace right?

I think we're talking about different workspaces 😅. The error is the server failing to handle the case where the user has not yet opened any documents in the editor yet - thus pygls' internal Workspace object is empty.

Definitely one to fix, see my incoming PR :)

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 this pull request may close these issues.

3 participants