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

Simplify vscode-playground setup and fix Python discovery #374

Merged
merged 4 commits into from
Nov 15, 2023

Conversation

alcarney
Copy link
Collaborator

@alcarney alcarney commented Sep 10, 2023

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

image

This attempts to fix the issues seen in #357
@tombh @z80dev, would you mind trying this out and seeing if it fixes your issues?

  • The multi-root setup was perhaps more complicated than necessary, so this moves examples/workspace/ into the examples/servers/ directory.
  • The extension now launches in single-root mode in the examples/servers folder by default
  • The extension tries to be smarter about discovering the right interpreter to use
  • If all else fails there is now a pygls.server.pythonPath option to override the interpreter selection made by the extension (basically how the json-extension used to work)
  • An example settings.json file is now included so should be easier for the user to start tweaking values

There was also a bug in how the json_server.py handled the workspace/diagnostic request when no documents were open - that should be fixed now

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 Sep 14, 2023

It works!

Loading up VSCode from within a source env/bin/activate shell just automatically finds the right Python interpreter now 🥳

2023-09-14 18:02:22.536 [info] Extension activated.
2023-09-14 18:02:22.536 [info] python env modified, restarting server...
2023-09-14 18:02:22.536 [info] cwd: '/home/tombh/Workspace/pygls/examples/servers'
2023-09-14 18:02:22.536 [info] server: 'json_server.py'
2023-09-14 18:02:22.536 [info] Looking for environment in which to execute: 'file:///home/tombh/Workspace/pygls/examples/servers/json_server.py'
2023-09-14 18:02:22.536 [info] Found environment: /home/tombh/Workspace/pygls/env/bin/python: /home/tombh/Workspace/pygls/env/bin/python
2023-09-14 18:02:22.757 [info] client options: {
  "documentSelector": [
    {
      "scheme": "file",
      "language": "json"
    }
  ],
  "outputChannel": {
    "name": "pygls",
    "logLevel": 3
  },
  "connectionOptions": {
    "maxRestartCount": 0
  }
}
2023-09-14 18:02:28.097 [info] Validating json...

And when I make an error in the test.json it seems I get 2 sets of diagnostic reports, I assume one of them must be the example server. The other must come by default with VSCode?
image

But I spent a long time trying to get the Code Actions and Inlay Hints to work, because I didn't realise that I had to edit the settigns.json file 😬 Coming from the Neovim world I thought I had to find a keyboard shortcut to show the Code Actions popup, and that I had to enable Inlay Hints in some global settings. So I think it'd be good to very briefly explain how to run each of the example servers and what to expect when each one does run.

This is so awesome!!
image
image

@alcarney
Copy link
Collaborator Author

alcarney commented Sep 15, 2023

It works!

Glad to hear it!

Loading up VSCode from within a source env/bin/activate shell just automatically finds the right Python interpreter now

Would you mind quickly trying it with the venv deactivated also? In an ideal world the extension shouldn't require you to open VSCode in any special way. (Though you might have to use the Python: Select Interpreter command as a first time setup step)

And when I make an error in the test.json it seems I get 2 sets of diagnostic reports, I assume one of them must be the example server. The other must come by default with VSCode?

Yes :)

So I think it'd be good to very briefly explain how to run each of the example servers and what to expect when each one does run.

Ok, I'll look at expanding on the README a bit more - do you mind if I pinch your screenshots?

@tombh
Copy link
Collaborator

tombh commented Sep 21, 2023

I had a bit of a play without using the venv's activate and couldn't get it to work. I think the basic fact is that VSCode's Python extension can only find the env/bin/python interpreter when VSCode is opened in the root of the project (therefore the parent of .git/ and env).

This is what I see when opening VSCode in examples/vscode-playground:
image

And this is what I see when opening VSCode from the project root:
image

@alcarney
Copy link
Collaborator Author

I think the basic fact is that VSCode's Python extension can only find the env/bin/python interpreter when VSCode is opened in the root of the project (therefore the parent of .git/ and env).

Oh! It's amazing what rough edges you gloss over when you're used to something. 😅

That's how it works for me as well. You should be able to use the Enter interpreter path option to get a file open dialog and manually set the path to the python exe for the playground to use 🤞

Once setup VsCode should remember the choice for future sessions.

Another step to go in README...

@alcarney
Copy link
Collaborator Author

Alternatively....

Perhaps it's better to move the launch.json file for the playground into the root of the repo... and try and set things up so that people don't have to open VSCode in a subfolder at all 🤔

@tombh
Copy link
Collaborator

tombh commented Sep 23, 2023

Oh! It's amazing what rough edges you gloss over when you're used to something. 😅

OMG I know, so true 😅

You should be able to use the Enter interpreter path option to get a file open dialog and manually set the path to the python exe for the playground to use 🤞

It doesn't seem to in my case. Again, I might be doing something wrong, but even once I've selected the ./env/bin/python from a VSCode instance opened in the examples/vscode-playground folder, I still get the No module named 'pygls' error 🫤

Perhaps it's better to move the launch.json file for the playground into the root of the repo

Hmm, yes, I suppose that might be what's needed.

@alcarney
Copy link
Collaborator Author

Again, I might be doing something wrong, but even once I've selected the ./env/bin/python from a VSCode instance opened in the examples/vscode-playground folder, I still get the No module named 'pygls' error 🫤

So strange! :/

@karthiknadig I don't suppose you'd able to take a quick look at this function and see if I'm missing anything obvious. It should allow people to use Python: Select Interpreter to configure the Python envrionment to use with pygls, but it doesn't seem to work for anyone except me! 😅

Otherwise, I'll look into moving things around.

@karthiknadig
Copy link
Contributor

karthiknadig commented Sep 26, 2023

When calling getActiveEnvironmentPath is it called with the workspace.uri or some uri from the current workspace. If it is called with undefined it can fail to get selected python.

@alcarney alcarney force-pushed the fix-extension branch 2 times, most recently from 6282f17 to 6c6e4bf Compare November 8, 2023 18:26
@alcarney alcarney requested a review from tombh November 8, 2023 18:29
@alcarney
Copy link
Collaborator Author

alcarney commented Nov 8, 2023

@tombh I wonder if we should merge this?

I did have a go at moving the launch.json file to the root of the repo, but VSCode didn't seem to want to launch a debug instance in the same folder as the main instance :/

At the very least the simplified folder structure should be an improvement and we have a few workarounds if discovery fails

  • Launch VSCode with the venv active - like how you've been doing
  • Use the pygls.server.pythonPath option to override the automatic discovery mechanism

Ideally it should Just WorkTM, but I'm not sure what else to try at this point

@tombh
Copy link
Collaborator

tombh commented Nov 11, 2023

Sure, let's merge it. I think they're relatively minor issues really. The main thing is that there are workarounds. And having it merged might give it more exposure and more feedback and ideas.

Along with the simpler single-root setup, this introduces a new
`pygls.server.pythonPath` option that if set, will be used instead of
the interpreter discovered via the Python extension.

The interpreter discovery code has also been updated to be explicit
about what it's trying to use the interpreter with - in an attempt to
work in more scenarios
@tombh
Copy link
Collaborator

tombh commented Nov 14, 2023

Shall I merge this then? 🚀

@alcarney
Copy link
Collaborator Author

@tombh go for it :)

@tombh tombh merged commit 4df6bb6 into openlawlibrary:main Nov 15, 2023
17 checks passed
@tombh
Copy link
Collaborator

tombh commented Nov 15, 2023

Done. And a little point release as well to celebrate?

@alcarney
Copy link
Collaborator Author

alcarney commented Nov 15, 2023

Aren't we waiting to hear back about the impact of #411?

@alcarney alcarney deleted the fix-extension branch November 15, 2023 21:02
@karthiknadig
Copy link
Contributor

@alcarney I will provide an Update on #411 by end of the day for me (which is Pacific time)

tombh added a commit that referenced this pull request Nov 16, 2023
* Remove dependency on `typeguard` by @karthiknadig in #411
* Simplify vscode-playground setup and fix Python discovery by @alcarney in #374
@tombh tombh mentioned this pull request Nov 16, 2023
tombh added a commit that referenced this pull request Nov 16, 2023
* Remove dependency on `typeguard` by @karthiknadig in #411
* Simplify vscode-playground setup and fix Python discovery by @alcarney in #374
tombh added a commit that referenced this pull request Nov 18, 2023
* Remove dependency on `typeguard` by @karthiknadig in #411
* Simplify vscode-playground setup and fix Python discovery by @alcarney in #374
tombh added a commit that referenced this pull request Nov 18, 2023
* Remove dependency on `typeguard` by @karthiknadig in #411
* Simplify vscode-playground setup and fix Python discovery by @alcarney in #374
tombh added a commit that referenced this pull request Nov 18, 2023
* Remove dependency on `typeguard` by @karthiknadig in #411
* Simplify vscode-playground setup and fix Python discovery by @alcarney in #374
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