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

Fix "Python: Create Environment" not selecting the corresponding interpreter #3305

Merged
merged 3 commits into from
May 30, 2024

Conversation

seeM
Copy link
Contributor

@seeM seeM commented May 29, 2024

Intent

Address #3298.

Approach

The fix is to listen to IInterpreterService.onDidChangeInterpreter and call selectLanguageRuntime.

Unfortunately, that makes it possible to do quick consecutive calls to selectLanguageRuntime which errors before this PR. The second change is to coalesce selectLanguageRuntime – indirectly, by extracting shutdownRuntimeSession and coalescing calls to it. That'll also be needed when we bring functionality from positron-notebook-controller into positron core (#2671).

QA Notes

See the repro in the issue, also worth playing around with the "Python: Select Interpreter" command, and the other ways of shutting down a runtime in the UI.

@seeM seeM requested review from jmcphers and petetronic May 29, 2024 16:57
@petetronic
Copy link
Collaborator

Confirmed venvs are now created and prompt to install ipykernel, and complete successfully when Install is selected. Tested this branch on both Mac and Windows.

Copy link
Collaborator

@jmcphers jmcphers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! LGTM pending some hygiene as noted.

@seeM seeM merged commit b8f3f03 into main May 30, 2024
21 checks passed
@seeM seeM deleted the fix-create-env-not-selecting-interpreter branch May 30, 2024 15:56
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