Skip to content

Commit

Permalink
Auto select Python Interpreter prior to validation of interpreters (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
DonJayamanne authored Nov 21, 2018
1 parent d987489 commit 7e30acb
Show file tree
Hide file tree
Showing 13 changed files with 43 additions and 31 deletions.
6 changes: 5 additions & 1 deletion .github/test_plan.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,11 @@
- Make sure that you do not have `python.pythonPath` specified in your `settings.json` when testing automatic detection
- Do note that the `Select Interpreter` drop-down window scrolls

- [ ] Detected a single virtual environment at the top-level of the workspace folder (if you created this _after_ opening VS Code, then run `Reload Window` to pick up the new environment)
- [ ] Detected a single virtual environment at the top-level of the workspace folder on Mac when when `python` command points to default Mac Python installation or `python` command fails in the terminal.
- [ ] Appropriate suffix label specified in status bar (e.g. `(venv)`)
- [ ] Detected a single virtual environment at the top-level of the workspace folder on Windows when `python` fails in the terminal.
- [ ] Appropriate suffix label specified in status bar (e.g. `(venv)`)
- [ ] Detected a single virtual environment at the top-level of the workspace folder
- [ ] Appropriate suffix label specified in status bar (e.g. `(venv)`)
- [ ] [`Create Terminal`](https://code.visualstudio.com/docs/python/environments#_activating-an-environment-in-the-terminal) works
- [ ] Steals focus
Expand Down
1 change: 1 addition & 0 deletions news/2 Fixes/3326.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Auto select `Python Interpreter` prior to validation of interpreters and changes to messages displayed.
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@
"activationEvents": [
"onLanguage:python",
"onDebugResolve:python",
"onDebugResolve:python",
"onCommand:python.execInTerminal",
"onCommand:python.sortImports",
"onCommand:python.runtests",
Expand Down
31 changes: 16 additions & 15 deletions package.nls.json
Original file line number Diff line number Diff line change
Expand Up @@ -84,22 +84,23 @@
"DataScience.startingJupyter": "Starting Jupyter Server",
"Interpreters.RefreshingInterpreters": "Refreshing Python Interpreters",
"Interpreters.LoadingInterpreters": "Loading Python Interpreters",
"DataScience.restartKernelMessage" : "Do you want to restart the Jupter kernel? All variables will be lost.",
"DataScience.restartKernelMessageYes" : "Restart",
"DataScience.restartKernelMessageNo" : "Cancel",
"DataScience.restartKernelMessage": "Do you want to restart the Jupter kernel? All variables will be lost.",
"DataScience.restartKernelMessageYes": "Restart",
"DataScience.restartKernelMessageNo": "Cancel",
"DataScienceSurveyBanner.bannerMessage": "Can you please take 2 minutes to tell us how the Python Data Science features are working for you?",
"DataScienceSurveyBanner.bannerLabelYes": "Yes, take survey now",
"DataScienceSurveyBanner.bannerLabelNo": "No, thanks",
"DataScience.restartingKernelStatus" : "Restarting Jupyter Kernel",
"DataScience.executingCode" : "Executing Cell",
"DataScience.collapseAll" : "Collapse all cell inputs",
"DataScience.expandAll" : "Expand all cell inputs",
"DataScience.export" : "Export as Jupyter Notebook",
"DataScience.restartServer" : "Restart iPython Kernel",
"DataScience.undo" : "Undo",
"DataScience.redo" : "Redo",
"DataScience.clearAll" : "Remove All Cells",
"DataScience.pythonVersionHeader" : "Python Version:",
"DataScience.pythonVersionHeaderNoPyKernel" : "Python Version may not match, no ipykernel found:",
"DataScience.pythonRestartHeader" : "Restarted Kernel:"
"DataScience.restartingKernelStatus": "Restarting Jupyter Kernel",
"DataScience.executingCode": "Executing Cell",
"DataScience.collapseAll": "Collapse all cell inputs",
"DataScience.expandAll": "Expand all cell inputs",
"DataScience.export": "Export as Jupyter Notebook",
"DataScience.restartServer": "Restart iPython Kernel",
"DataScience.undo": "Undo",
"DataScience.redo": "Redo",
"DataScience.clearAll": "Remove All Cells",
"DataScience.pythonVersionHeader": "Python Version:",
"DataScience.pythonVersionHeaderNoPyKernel": "Python Version may not match, no ipykernel found:",
"DataScience.pythonRestartHeader": "Restarted Kernel:",
"Linter.InstalledButNotEnabled": "Linter {0} is installed but not enabled."
}
3 changes: 2 additions & 1 deletion src/client/activation/languageServer/languageServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
import { PythonSettings } from '../../common/configSettings';
// tslint:disable-next-line:ordered-imports
import { isTestExecution, STANDARD_OUTPUT_CHANNEL } from '../../common/constants';
import { Logger } from '../../common/logger';
import { IFileSystem, IPlatformService } from '../../common/platform/types';
import {
BANNER_NAME_LS_SURVEY, DeprecatedFeatureInfo, IConfigurationService,
Expand Down Expand Up @@ -225,7 +226,7 @@ export class LanguageServerExtensionActivator implements IExtensionActivator {
const interpreterDataService = new InterpreterDataService(this.context, this.services);
interpreterData = await interpreterDataService.getInterpreterData();
} catch (ex) {
this.appShell.showWarningMessage('Unable to determine path to the Python interpreter. IntelliSense will be limited.');
Logger.error('Unable to determine path to the Python interpreter. IntelliSense will be limited.', ex);
}

this.interpreterHash = interpreterData ? interpreterData.hash : '';
Expand Down
4 changes: 4 additions & 0 deletions src/client/common/utils/localize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ export namespace Interpreters {
export const refreshing = localize('Interpreters.RefreshingInterpreters', 'Refreshing Python Interpreters');
}

export namespace Linters {
export const installedButNotEnabled = localize('Linter.InstalledButNotEnabled', 'Linter {0} is installed but not enabled.');
}

export namespace DataScienceSurveyBanner {
export const bannerMessage = localize('DataScienceSurveyBanner.bannerMessage', 'Can you please take 2 minutes to tell us how the Python Data Science features are working for you?');
export const bannerLabelYes = localize('DataScienceSurveyBanner.bannerLabelYes', 'Yes, take survey now');
Expand Down
6 changes: 3 additions & 3 deletions src/client/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,15 +77,15 @@ export async function activate(context: ExtensionContext): Promise<IExtensionApi
registerServices(context, serviceManager, serviceContainer);
initializeServices(context, serviceManager, serviceContainer);

const interpreterManager = serviceContainer.get<IInterpreterService>(IInterpreterService);
await interpreterManager.autoSetInterpreter();

// When testing, do not perform health checks, as modal dialogs can be displayed.
if (!isTestExecution()) {
const appDiagnostics = serviceContainer.get<IApplicationDiagnostics>(IApplicationDiagnostics);
await appDiagnostics.performPreStartupHealthCheck();
}

const interpreterManager = serviceContainer.get<IInterpreterService>(IInterpreterService);
await interpreterManager.autoSetInterpreter();

serviceManager.get<ITerminalAutoActivation>(ITerminalAutoActivation).register();
const configuration = serviceManager.get<IConfigurationService>(IConfigurationService);
const pythonSettings = configuration.getSettings();
Expand Down
2 changes: 1 addition & 1 deletion src/client/interpreter/contracts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export const IInterpreterLocatorService = Symbol('IInterpreterLocatorService');

export interface IInterpreterLocatorService extends Disposable {
readonly onLocating: Event<Promise<PythonInterpreter[]>>;
getInterpreters(resource?: Uri): Promise<PythonInterpreter[]>;
getInterpreters(resource?: Uri, ignoreCache?: boolean): Promise<PythonInterpreter[]>;
}

export type CondaInfo = {
Expand Down
4 changes: 2 additions & 2 deletions src/client/interpreter/interpreterService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,14 @@ export class InterpreterService implements Disposable, IInterpreterService {
}
// Check pipenv first.
const pipenvService = this.serviceContainer.get<IInterpreterLocatorService>(IInterpreterLocatorService, PIPENV_SERVICE);
let interpreters = await pipenvService.getInterpreters(activeWorkspace.folderUri);
let interpreters = await pipenvService.getInterpreters(activeWorkspace.folderUri, true);
if (interpreters.length > 0) {
await this.pythonPathUpdaterService.updatePythonPath(interpreters[0].path, activeWorkspace.configTarget, 'load', activeWorkspace.folderUri);
return;
}
// Now check virtual environments under the workspace root
const virtualEnvInterpreterProvider = this.serviceContainer.get<IInterpreterLocatorService>(IInterpreterLocatorService, WORKSPACE_VIRTUAL_ENV_SERVICE);
interpreters = await virtualEnvInterpreterProvider.getInterpreters(activeWorkspace.folderUri);
interpreters = await virtualEnvInterpreterProvider.getInterpreters(activeWorkspace.folderUri, true);
const workspacePathUpper = activeWorkspace.folderUri.fsPath.toUpperCase();

const interpretersInWorkspace = interpreters.filter(interpreter => Uri.file(interpreter.path).fsPath.toUpperCase().startsWith(workspacePathUpper));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ export abstract class CacheableLocatorService implements IInterpreterLocatorServ
return this.locating.event;
}
public abstract dispose();
public async getInterpreters(resource?: Uri): Promise<PythonInterpreter[]> {
public async getInterpreters(resource?: Uri, ignoreCache?: boolean): Promise<PythonInterpreter[]> {
const cacheKey = this.getCacheKey(resource);
let deferred = this.promisesPerResource.get(cacheKey);

if (!deferred) {
if (!deferred || ignoreCache) {
deferred = createDeferred<PythonInterpreter[]>();
this.promisesPerResource.set(cacheKey, deferred);

Expand Down
4 changes: 3 additions & 1 deletion src/client/linters/linterAvailability.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@
import { inject, injectable } from 'inversify';
import { Uri } from 'vscode';
import { IApplicationShell, IWorkspaceService } from '../common/application/types';
import '../common/extensions';
import { traceError } from '../common/logger';
import { IConfigurationService, IInstaller, Product } from '../common/types';
import { Linters } from '../common/utils/localize';
import { IAvailableLinterActivator, ILinterInfo } from './types';

@injectable()
Expand Down Expand Up @@ -75,7 +77,7 @@ export class AvailableLinterActivator implements IAvailableLinterActivator {
];

// tslint:disable-next-line:messages-must-be-localized
const pick = await this.appShell.showInformationMessage(`Linter ${linterInfo.id} is available but not enabled.`, ...optButtons);
const pick = await this.appShell.showInformationMessage(Linters.installedButNotEnabled().format(linterInfo.id), ...optButtons);
if (pick) {
await linterInfo.enableAsync(pick.enabled);
return true;
Expand Down
4 changes: 2 additions & 2 deletions src/test/interpreters/interpreterService.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -512,9 +512,9 @@ suite('Interpreters service', () => {
}

function setupLocators(wks: PythonInterpreter[], pipenv: PythonInterpreter[]) {
pipenvLocator.setup(x => x.getInterpreters(TypeMoq.It.isAny())).returns(() => Promise.resolve(pipenv));
pipenvLocator.setup(x => x.getInterpreters(TypeMoq.It.isAny(), TypeMoq.It.isValue(true))).returns(() => Promise.resolve(pipenv));
serviceManager.addSingletonInstance<IInterpreterLocatorService>(IInterpreterLocatorService, pipenvLocator.object, PIPENV_SERVICE);
wksLocator.setup(x => x.getInterpreters(TypeMoq.It.isAny())).returns(() => Promise.resolve(wks));
wksLocator.setup(x => x.getInterpreters(TypeMoq.It.isAny(), TypeMoq.It.isValue(true))).returns(() => Promise.resolve(wks));
serviceManager.addSingletonInstance<IInterpreterLocatorService>(IInterpreterLocatorService, wksLocator.object, WORKSPACE_VIRTUAL_ENV_SERVICE);

}
Expand Down
4 changes: 2 additions & 2 deletions src/test/linters/linter.availability.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ suite('Linter Availability Provider tests', () => {
}(Product.pylint, 'pylint', configServiceMock.object, ['.pylintrc', 'pylintrc']);

appShellMock.setup(ap => ap.showInformationMessage(
TypeMoq.It.isValue(`Linter ${linterInfo.id} is available but not enabled.`),
TypeMoq.It.isValue(`Linter ${linterInfo.id} is installed but not enabled.`),
TypeMoq.It.isAny(),
TypeMoq.It.isAny())
)
Expand Down Expand Up @@ -207,7 +207,7 @@ suite('Linter Availability Provider tests', () => {
// arrange
const [appShellMock, installerMock, workspaceServiceMock, configServiceMock, linterInfo] = getDependenciesForAvailabilityTests();
appShellMock.setup(ap => ap.showInformationMessage(
TypeMoq.It.isValue(`Linter ${linterInfo.id} is available but not enabled.`),
TypeMoq.It.isValue(`Linter ${linterInfo.id} is installed but not enabled.`),
TypeMoq.It.isAny(),
TypeMoq.It.isAny())
)
Expand Down

0 comments on commit 7e30acb

Please sign in to comment.