Fix up runtime discovery with progress notification and improved user feedback #9247
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR improves the user experience for runtime discovery by adding progress notifications and better feedback messages when users explicitly trigger interpreter discovery. The key enhancement is showing users what's happening during the potentially long discovery process and providing clear feedback about the results.
Key changes:
- Added progress notification during runtime discovery with cancellable UI
- Enhanced user feedback with messages for both successful discoveries and cases where no new interpreters are found
- Restructured the discovery flow to properly await completion before showing results
| await new Promise<void>((resolve) => { | ||
| this._languageRuntimeService.onDidChangeRuntimeStartupPhase(phase => { | ||
| if (phase === RuntimeStartupPhase.Complete) { |
There was a problem hiding this comment.
The event listener created by onDidChangeRuntimeStartupPhase is never disposed of, causing a memory leak. The listener should be stored and disposed when the promise resolves or the method completes.
Copilot uses AI. Check for mistakes.
| await this._progressService.withProgress({ | ||
| location: ProgressLocation.Notification, | ||
| title: nls.localize('positron.runtimeStartupService.discoveringRuntimes', 'Discovering interpreters...'), | ||
| cancellable: false | ||
| }, async (progress) => { |
There was a problem hiding this comment.
[nitpick] The progress service is configured as non-cancellable, but users might want to cancel a long-running discovery process. Consider making this cancellable and handling the cancellation appropriately.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Given that discoverAllRuntimes() calls back into the extensions, making it cancellable will take a lot of piping through. Let's not do this now.
Addresses #8404
With this PR, we now show a progress notification when users explicitly call Interpreter: Discover All Interpreters. It looks like this while discovery is going on (which can take a while, but if a user really explicitly said to do this, probably the right option):
If nothing new is found, the progress notification resolves and then you see this:
If something new is found, the progress notification resolves and then you see this:
Release Notes
New Features
Bug Fixes
QA Notes
The command Interpreter: Discover All Interpreters will not run unless the first round of discovery is done, which runs quietly in the background. This may take longer than you expect. Once you can run it: