804 + Improve 'no installers available' messaging #823
Conversation
MikhailArkhipov
changed the title
Codecov Report
@@ Coverage Diff @@
## master #823 +/- ##
==========================================
+ Coverage 63.64% 63.64% +<.01%
==========================================
Files 258 260 +2
Lines 11807 11837 +30
Branches 2088 2093 +5
==========================================
+ Hits 7514 7534 +20
- Misses 4285 4295 +10
Partials 8 8
Continue to review full report at Codecov.
|
|
@MikhailArkhipov I.e. change this code const pythonInstaller = new PythonInstaller(serviceContainer);
pythonInstaller.checkPythonInstallation(PythonSettings.getInstance())
.catch(ex => console.error('Python Extension: pythonInstaller.checkPythonInstallation', ex));
// This must be completed before we can continue.
await interpreterManager.autoSetInterpreter();to this: // This must be completed before we can continue.
await interpreterManager.autoSetInterpreter();
const pythonInstaller = new PythonInstaller(serviceContainer);
pythonInstaller.checkPythonInstallation(PythonSettings.getInstance())
.catch(ex => console.error('Python Extension: pythonInstaller.checkPythonInstallation', ex)); |
| }); | ||
| const displayName = await this.versionProvider.getVersion(interpreter, path.basename(interpreter)); | ||
| const virtualEnv = await this.virtualEnvMgr.detect(interpreter); | ||
| const virtualEnvSuffix = virtualEnv ? virtualEnv.name : this.getVirtualEnvironmentRootDirectory(interpreter); |
There was a problem hiding this comment.
The previous approach of Promise.all() would be faster.
This new approach will be pretty much a sequential approach.
First code will wait to get version from getVersion, then after the value is available it will move onto detect.
Where as with Promise.all both methods will be executed and we'll wait for both to complete.
This then affects the overall time taken to discover interpreters.
There was a problem hiding this comment.
As far as I understand they are not actually run in parallel but sure. I mean, there is no thread switch in a middle of async code at it happens in C# or C++ threading, so each call executes to the end before another call begins. Hence no thread synchronization primitives AFAIK. Then can execute out or order though.
| }); | ||
| let displayName = await this.versionProvider.getVersion(interpreter, path.basename(interpreter)); | ||
| const virtualEnv = await this.virtualEnvMgr.detect(interpreter); | ||
| displayName += virtualEnv ? ` (${virtualEnv.name})` : ''; |
There was a problem hiding this comment.
The previous approach of Promise.all() would be faster.
This new approach will be pretty much a sequential approach.
First code will wait to get version from getVersion, then after the value is available it will move onto detect.
Where as with Promise.all both methods will be executed and we'll wait for both to complete.
| @@ -16,7 +16,7 @@ export class JediFactory implements Disposable { | |||
| this.disposables = []; | |||
| } | |||
| public getJediProxyHandler<T extends ICommandResult>(resource: Uri): JediProxyHandler<T> { | |||
| const workspaceFolder = workspace.getWorkspaceFolder(resource); | |||
| const workspaceFolder = resource ? workspace.getWorkspaceFolder(resource) : undefined; | |||
There was a problem hiding this comment.
We should change the signature of the parameter to resource?:Uri to be explicit about the fact that resource can be undefined.
|
I have a feeling that |
Fixes #804 as well as improves 'No installers available' case.
how to install pip on the current platform