Skip to content

Fix no interpreter error message #57

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Jul 25, 2023
Merged

Fix no interpreter error message #57

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
66ec06b
Add verification for interpreter in debugAdapter
paulacamargo25 Jun 30, 2023
a7cd940
Fix tests
paulacamargo25 Jul 24, 2023
cb02bda
Merge branch 'main' into Fix-no-interpreter-error-message
paulacamargo25 Jul 25, 2023
57242ab
Add error
paulacamargo25 Jul 24, 2023
dc90877
Fix unused import
paulacamargo25 Jul 25, 2023
124813c
Fix tests
paulacamargo25 Jul 25, 2023
c210045
fix import error
paulacamargo25 Jul 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 13 additions & 13 deletions src/extension/common/python.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ interface IExtensionApi {
known: Environment[];
getActiveEnvironmentPath(resource?: Resource): EnvironmentPath;
resolveEnvironment(
environment: Environment | EnvironmentPath | string,
environment: Environment | EnvironmentPath | string | undefined,
): Promise<ResolvedEnvironment | undefined>;
readonly onDidChangeActiveEnvironmentPath: Event<ActiveEnvironmentPathChangeEvent>;
getEnvironmentVariables(resource?: Resource): EnvironmentVariables;
Expand Down Expand Up @@ -80,17 +80,6 @@ export async function initializePython(disposables: Disposable[]): Promise<void>
}
}

export async function getInterpreterDetails(resource?: Uri): Promise<IInterpreterDetails> {
const api = await getPythonExtensionAPI();
const environment = await api?.environments.resolveEnvironment(
api?.environments.getActiveEnvironmentPath(resource),
);
if (environment?.executable.uri) {
return { path: [environment?.executable.uri.fsPath], resource };
}
return { path: undefined, resource };
}

export async function getDebuggerPath(): Promise<string | undefined> {
const api = await getPythonExtensionAPI();
return api?.debug.getDebuggerPackagePath();
Expand All @@ -111,7 +100,7 @@ export async function getEnvironmentVariables(resource?: Resource) {
return api?.environments.getEnvironmentVariables(resource);
}

export async function resolveEnvironment(env: Environment | EnvironmentPath | string) {
export async function resolveEnvironment(env: Environment | EnvironmentPath | string | undefined) {
const api = await getPythonExtensionAPI();
return api?.environments.resolveEnvironment(env);
}
Expand All @@ -121,6 +110,17 @@ export async function getActiveEnvironmentPath(resource?: Resource) {
return api?.environments.getActiveEnvironmentPath(resource);
}

export async function getInterpreterDetails(resource?: Uri): Promise<IInterpreterDetails> {
const api = await getPythonExtensionAPI();
const environment = await api?.environments.resolveEnvironment(
api?.environments.getActiveEnvironmentPath(resource),
);
if (environment?.executable.uri) {
return { path: [environment?.executable.uri.fsPath], resource };
}
return { path: undefined, resource };
}

export async function hasInterpreters() {
const api = await getPythonExtensionAPI();
if (api) {
Expand Down
39 changes: 1 addition & 38 deletions src/extension/common/utils/localize.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export namespace AttachProcess {
}

export namespace DebugConfigStrings {
export const debugStopped = l10n.t('Debug Stopped');
export const selectConfiguration = {
title: l10n.t('Select a debug configuration'),
placeholder: l10n.t('Debug Configuration'),
Expand Down Expand Up @@ -142,44 +143,6 @@ export namespace DebugConfigStrings {
}
}

export namespace Diagnostics {
export const warnSourceMaps = l10n.t(
'Source map support is enabled in the Python Extension, this will adversely impact performance of the extension.',
);
export const disableSourceMaps = l10n.t('Disable Source Map Support');
export const warnBeforeEnablingSourceMaps = l10n.t(
'Enabling source map support in the Python Extension will adversely impact performance of the extension.',
);
export const enableSourceMapsAndReloadVSC = l10n.t('Enable and reload Window.');
export const lsNotSupported = l10n.t(
'Your operating system does not meet the minimum requirements of the Python Language Server. Reverting to the alternative autocompletion provider, Jedi.',
);
export const invalidPythonPathInDebuggerSettings = l10n.t(
'You need to select a Python interpreter before you start debugging.\n\nTip: click on "Select Interpreter" in the status bar.',
);
export const invalidPythonPathInDebuggerLaunch = l10n.t('The Python path in your debug configuration is invalid.');
export const invalidDebuggerTypeDiagnostic = l10n.t(
'Your launch.json file needs to be updated to change the "pythonExperimental" debug configurations to use the "python" debugger type, otherwise Python debugging may not work. Would you like to automatically update your launch.json file now?',
);
export const consoleTypeDiagnostic = l10n.t(
'Your launch.json file needs to be updated to change the console type string from "none" to "internalConsole", otherwise Python debugging may not work. Would you like to automatically update your launch.json file now?',
);
export const justMyCodeDiagnostic = l10n.t(
'Configuration "debugStdLib" in launch.json is no longer supported. It\'s recommended to replace it with "justMyCode", which is the exact opposite of using "debugStdLib". Would you like to automatically update your launch.json file to do that?',
);
export const yesUpdateLaunch = l10n.t('Yes, update launch.json');
export const invalidTestSettings = l10n.t(
'Your settings needs to be updated to change the setting "python.unitTest." to "python.testing.", otherwise testing Python code using the extension may not work. Would you like to automatically update your settings now?',
);
export const updateSettings = l10n.t('Yes, update settings');
export const checkIsort5UpgradeGuide = l10n.t(
'We found outdated configuration for sorting imports in this workspace. Check the [isort upgrade guide](https://aka.ms/AA9j5x4) to update your settings.',
);
export const pylanceDefaultMessage = l10n.t(
"The Python extension now includes Pylance to improve completions, code navigation, overall performance and much more! You can learn more about the update and learn how to change your language server [here](https://aka.ms/new-python-bundle).\n\nRead Pylance's license [here](https://marketplace.visualstudio.com/items/ms-python.vscode-pylance/license).",
);
}

export namespace Common {
export const loadingExtension = l10n.t('Python Debugger extension loading...');
export const doNotShowAgain = l10n.t('Do not show again');
Expand Down
57 changes: 20 additions & 37 deletions src/extension/debugger/adapter/factory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,15 @@ import {
} from 'vscode';
import { AttachRequestArguments, LaunchRequestArguments } from '../../types';
import { IDebugAdapterDescriptorFactory } from '../types';
import { showErrorMessage } from '../../common/vscodeapi';
import { executeCommand, showErrorMessage } from '../../common/vscodeapi';
import { traceLog, traceVerbose } from '../../common/log/logging';
import { EventName } from '../../telemetry/constants';
import { sendTelemetryEvent } from '../../telemetry';
import {
getActiveEnvironmentPath,
getInterpreters,
hasInterpreters,
resolveEnvironment,
runPythonExtensionCommand,
} from '../../common/python';
import { getActiveEnvironmentPath, resolveEnvironment, runPythonExtensionCommand } from '../../common/python';
import { Commands, EXTENSION_ROOT_DIR } from '../../common/constants';
import { Common, Interpreters } from '../../common/utils/localize';
import { Common, DebugConfigStrings, Interpreters } from '../../common/utils/localize';
import { IPersistentStateFactory } from '../../common/types';
import { Environment } from '../../common/pythonTypes';
import { ignoreErrors } from '../../common/promiseUtils';

// persistent state names, exported to make use of in testing
export enum debugStateKeys {
Expand All @@ -45,7 +38,7 @@ export class DebugAdapterDescriptorFactory implements IDebugAdapterDescriptorFac
public async createDebugAdapterDescriptor(
session: DebugSession,
_executable: DebugAdapterExecutable | undefined,
): Promise<DebugAdapterDescriptor> {
): Promise<DebugAdapterDescriptor | undefined> {
const configuration = session.configuration as LaunchRequestArguments | AttachRequestArguments;

// There are four distinct scenarios here:
Expand Down Expand Up @@ -98,16 +91,14 @@ export class DebugAdapterDescriptorFactory implements IDebugAdapterDescriptorFac
traceLog(`DAP Server launched with command: ${executable} ${args.join(' ')}`);
sendTelemetryEvent(EventName.DEBUG_ADAPTER_USING_WHEELS_PATH, undefined, { usingWheels: true });
return new DebugAdapterExecutable(executable, args);
} else {
throw new Error(DebugConfigStrings.debugStopped);
}

// Unlikely scenario.
throw new Error('Debug Adapter Executable not provided');
}

// FIX THIS
/**
* Get the python executable used to launch the Python Debug Adapter.
* In the case of `attach` scenarios, just use the workspace interpreter, else first available one.
* In the case of `attach` scenarios, just use the workspace interpreter.
* It is unlike user won't have a Python interpreter
*
* @private
Expand All @@ -128,21 +119,25 @@ export class DebugAdapterDescriptorFactory implements IDebugAdapterDescriptorFac

const resourceUri = workspaceFolder ? workspaceFolder.uri : undefined;
const interpreterPath = await getActiveEnvironmentPath(resourceUri);
const interpreter = await resolveEnvironment(interpreterPath);

if (interpreterPath) {
if (interpreter?.path) {
traceVerbose(`Selecting active interpreter as Python Executable for DA '${interpreterPath}'`);
return this.getExecutableCommand(await resolveEnvironment(interpreterPath));
}

await hasInterpreters(); // Wait until we know whether we have an interpreter
const interpreters = await getInterpreters();
if (interpreters.length === 0) {
ignoreErrors(this.notifySelectInterpreter());
return [];
const prompts = [Interpreters.changePythonInterpreter];
const selection = await showErrorMessage(
l10n.t(
'You need to select a Python interpreter before you start debugging.\n\nTip: click on "Select Interpreter" in the status bar.',
),
{ modal: true },
...prompts,
);
if (selection === Interpreters.changePythonInterpreter) {
await executeCommand(Commands.Set_Interpreter);
}

traceVerbose(`Picking first available interpreter to launch the DA '${interpreters[0].path}'`);
return this.getExecutableCommand(interpreters[0]);
return [];
}

private async showDeprecatedPythonMessage() {
Expand Down Expand Up @@ -185,16 +180,4 @@ export class DebugAdapterDescriptorFactory implements IDebugAdapterDescriptorFac
}
return [];
}

/**
* Notify user about the requirement for Python.
* Unlikely scenario, as ex expect users to have Python in order to use the extension.
* However it is possible to ignore the warnings and continue using the extension.
*
* @private
* @memberof DebugAdapterDescriptorFactory
*/
private async notifySelectInterpreter() {
await showErrorMessage(l10n.t('Please install Python or select a Python Interpreter to use the debugger.'));
}
}
41 changes: 25 additions & 16 deletions src/test/unittest/adapter/factory.unit.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import * as vscodeApi from '../../../extension/common/vscodeapi';
import { EXTENSION_ROOT_DIR } from '../../../extension/common/constants';
import { Architecture } from '../../../extension/common/platform';
import * as pythonApi from '../../../extension/common/python';
import { DebugConfigStrings } from '../../../extension/common/utils/localize';

use(chaiAsPromised);

Expand Down Expand Up @@ -125,21 +126,14 @@ suite('Debugging - Adapter Factory', () => {
assert.deepStrictEqual(descriptor, debugExecutable);
});

test('Return the path of the first available interpreter as the current python path, configuration.pythonPath is not defined and there is no active interpreter', async () => {
const session = createSession({});
const debugExecutable = new DebugAdapterExecutable(pythonPath, [debugAdapterPath]);

const descriptor = await factory.createDebugAdapterDescriptor(session, nodeExecutable);

assert.deepStrictEqual(descriptor, debugExecutable);
});

test('Display a message if no python interpreter is set', async () => {
getInterpretersStub.returns([]);
test.only('Display a message if no python interpreter is set', async () => {
getActiveEnvironmentPathStub.resolves(undefined);
const session = createSession({});
const promise = factory.createDebugAdapterDescriptor(session, nodeExecutable);

await expect(promise).to.eventually.be.rejectedWith('Debug Adapter Executable not provided');
await expect(promise).to.eventually.be.rejectedWith(DebugConfigStrings.debugStopped);

//check error message
sinon.assert.calledOnce(showErrorMessageStub);
});

Expand Down Expand Up @@ -191,11 +185,10 @@ suite('Debugging - Adapter Factory', () => {
test('Return Debug Adapter executable if request is "attach", and listen is specified', async () => {
const session = createSession({ request: 'attach', listen: { port: 5678, host: 'localhost' } });
const debugExecutable = new DebugAdapterExecutable(pythonPath, [debugAdapterPath]);

getActiveEnvironmentPathStub.resolves(interpreter.path);
resolveEnvironmentStub.resolves(interpreter);

const descriptor = await factory.createDebugAdapterDescriptor(session, nodeExecutable);

assert.deepStrictEqual(descriptor, debugExecutable);
});

Expand Down Expand Up @@ -223,6 +216,9 @@ suite('Debugging - Adapter Factory', () => {
EXTENSION_ROOT_DIR,
]);

getActiveEnvironmentPathStub.resolves(interpreter.path);
resolveEnvironmentStub.withArgs(interpreter.path).resolves(interpreter);

const descriptor = await factory.createDebugAdapterDescriptor(session, nodeExecutable);

assert.deepStrictEqual(descriptor, debugExecutable);
Expand All @@ -232,6 +228,9 @@ suite('Debugging - Adapter Factory', () => {
const session = createSession({});
const debugExecutable = new DebugAdapterExecutable(pythonPath, [debugAdapterPath]);

getActiveEnvironmentPathStub.resolves(interpreter.path);
resolveEnvironmentStub.withArgs(interpreter.path).resolves(interpreter);

const descriptor = await factory.createDebugAdapterDescriptor(session, nodeExecutable);

assert.deepStrictEqual(descriptor, debugExecutable);
Expand All @@ -241,20 +240,28 @@ suite('Debugging - Adapter Factory', () => {
const session = createSession({ logToFile: false });
const debugExecutable = new DebugAdapterExecutable(pythonPath, [debugAdapterPath]);

getActiveEnvironmentPathStub.resolves(interpreter.path);
resolveEnvironmentStub.withArgs(interpreter.path).resolves(interpreter);

const descriptor = await factory.createDebugAdapterDescriptor(session, nodeExecutable);

assert.deepStrictEqual(descriptor, debugExecutable);
});

test('Send attach to local process telemetry if attaching to a local process', async () => {
const session = createSession({ request: 'attach', processId: 1234 });
getActiveEnvironmentPathStub.resolves(interpreter.path);
resolveEnvironmentStub.withArgs(interpreter.path).resolves(interpreter);

await factory.createDebugAdapterDescriptor(session, nodeExecutable);

assert.ok(Reporter.eventNames.includes(EventName.DEBUGGER_ATTACH_TO_LOCAL_PROCESS));
});

test("Don't send any telemetry if not attaching to a local process", async () => {
const session = createSession({});
getActiveEnvironmentPathStub.resolves(interpreter.path);
resolveEnvironmentStub.withArgs(interpreter.path).resolves(interpreter);

await factory.createDebugAdapterDescriptor(session, nodeExecutable);

Expand All @@ -265,7 +272,8 @@ suite('Debugging - Adapter Factory', () => {
const customAdapterPath = 'custom/debug/adapter/path';
const session = createSession({ debugAdapterPath: customAdapterPath });
const debugExecutable = new DebugAdapterExecutable(pythonPath, [customAdapterPath]);

getActiveEnvironmentPathStub.resolves(interpreter.path);
resolveEnvironmentStub.withArgs(interpreter.path).resolves(interpreter);
const descriptor = await factory.createDebugAdapterDescriptor(session, nodeExecutable);

assert.deepStrictEqual(descriptor, debugExecutable);
Expand All @@ -292,7 +300,8 @@ suite('Debugging - Adapter Factory', () => {
test('Do not use "python" to spawn the debug adapter', async () => {
const session = createSession({ python: '/bin/custompy' });
const debugExecutable = new DebugAdapterExecutable(pythonPath, [debugAdapterPath]);

getActiveEnvironmentPathStub.resolves(interpreter.path);
resolveEnvironmentStub.withArgs(interpreter.path).resolves(interpreter);
const descriptor = await factory.createDebugAdapterDescriptor(session, nodeExecutable);

assert.deepStrictEqual(descriptor, debugExecutable);
Expand Down