openlawlibrary / pygls

A pythonic generic language server
https://pygls.readthedocs.io/en/latest/
Apache License 2.0
568 stars 103 forks source link

Update example vscode extension #357

Closed alcarney closed 1 year ago

alcarney commented 1 year ago

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

Let me know your thoughts! :smile:

Code review checklist (for code reviewer to complete)

tombh commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

Ah right, yeah that sounds great.

alcarney commented 1 year ago

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

The following config options have been introduced

Finally, the following technology upgrades have been performed

Supporting changes

tombh commented 1 year ago

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

tombh commented 1 year ago

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. (/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. (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 commented 1 year ago

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 https://github.com/openlawlibrary/pygls/pull/357#issuecomment-1678219483 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! :sweat_smile:

tombh commented 1 year ago

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 commented 1 year ago

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 :smile:

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! :sweat_smile: I'll probably drop it when opening the PR for real though

tombh commented 1 year ago

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 commented 1 year ago

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

:crossed_fingers: this is ready to merge? :slightly_smiling_face:

tombh commented 1 year ago

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!

z80dev commented 1 year ago

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 commented 1 year ago

Unfortunately I'm having the same issue as @tombh

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

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

I'm running Linux

To ask some of the "obvious" questions

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?

tombh commented 1 year ago

@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 commented 1 year ago

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 :thinking:

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 commented 1 year ago

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 commented 1 year ago

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

I think we're talking about different workspaces :sweat_smile:. 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 :)