Skip to content

Commit

Permalink
Use ruff.interpreter from workspace settings (#553)
Browse files Browse the repository at this point in the history
## Summary

This PR updates the scope of `ruff.interpreter` setting from `window` to
`resource` in order to resolve the variables such as
`${workspaceFolder}` if present.

In #551, what happens is that the Python extension fails to resolve the
environment corresponding to that interpreter which causes the hang. The
bug present in #551 was there for a very long time but it got visible
because of
e665ec7.
Before that commit, the extension would just move ahead and use the
interpreter from the Python extension but now we explicitly stop moving
ahead if it fails to resolve the environment corresponding to the
interpreter.

For reference, the `vscode-black-formatter` extension also updated the
scope of `ruff.interpreter` to be `resource` in
microsoft/vscode-black-formatter@5f2fc61.

fixes: #551

## Test Plan

### Relative paths are unresolved

```json
{
  "ruff.importStrategy": "fromEnvironment",
  "ruff.interpreter": [".venv/bin/python"]
}
```

Logs:

```
2024-07-24 08:50:37.182 [info] Using interpreter: .venv/bin/python
2024-07-24 08:50:39.240 [error] Unable to find any Python environment for the interpreter path: .venv/bin/python
```

Preview for the status bar:

<img width="1728" alt="Screenshot 2024-07-24 at 08 51 15"
src="https://github.com/user-attachments/assets/e58d2691-cd94-4e03-8ec5-bbc65d1bf46f">

### Using VS Code specific variables

```json
{
  "ruff.importStrategy": "fromEnvironment",
  "ruff.interpreter": ["${workspaceFolder}/.venv/bin/python"]
}
```

Logs:
```
2024-07-24 08:51:43.272 [info] Using interpreter: /Users/dhruv/playground/ruff/.venv/bin/python
2024-07-24 08:51:43.306 [info] Using the Ruff binary: /Users/dhruv/playground/ruff/.venv/bin/ruff
2024-07-24 08:51:43.310 [info] Resolved 'ruff.nativeServer: auto' to use the native server
2024-07-24 08:51:43.313 [info] Found Ruff 0.5.4 at /Users/dhruv/playground/ruff/.venv/bin/ruff
2024-07-24 08:51:43.313 [info] Server run command: /Users/dhruv/playground/ruff/.venv/bin/ruff server
2024-07-24 08:51:43.313 [info] Server: Start requested.
```
  • Loading branch information
dhruvmanila authored Jul 24, 2024
1 parent 9672002 commit c089b5a
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 62 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@
"ruff.interpreter": {
"default": [],
"markdownDescription": "Path to a Python interpreter to use to run the LSP server.",
"scope": "window",
"scope": "resource",
"items": {
"type": "string"
},
Expand Down
6 changes: 3 additions & 3 deletions src/common/python.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,13 @@ export async function runPythonExtensionCommand(command: string, ...rest: any[])
return await commands.executeCommand(command, ...rest);
}

export function checkVersion(resolved: ResolvedEnvironment | undefined): boolean {
const version = resolved?.version;
export function checkVersion(resolved: ResolvedEnvironment): boolean {
const version = resolved.version;
if (version?.major === 3 && version?.minor >= 7) {
return true;
}
traceError(`Python version ${version?.major}.${version?.minor} is not supported.`);
traceError(`Selected python path: ${resolved?.executable.uri?.fsPath}`);
traceError(`Selected python path: ${resolved.executable.uri?.fsPath}`);
traceError("Supported versions are 3.7 and above.");
return false;
}
7 changes: 2 additions & 5 deletions src/common/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import {
getGlobalSettings,
getUserSetLegacyServerSettings,
getUserSetNativeServerSettings,
getWorkspaceSettings,
ISettings,
} from "./settings";
import {
Expand All @@ -36,7 +35,6 @@ import {
NATIVE_SERVER_STABLE_VERSION,
} from "./version";
import { updateServerKind, updateStatus } from "./status";
import { getProjectRoot } from "./utilities";
import { isVirtualWorkspace } from "./vscodeapi";
import { execFile } from "child_process";
import which = require("which");
Expand Down Expand Up @@ -412,6 +410,8 @@ async function createServer(

let _disposables: Disposable[] = [];
export async function restartServer(
projectRoot: vscode.WorkspaceFolder,
workspaceSettings: ISettings,
serverId: string,
serverName: string,
outputChannel: LogOutputChannel,
Expand All @@ -426,9 +426,6 @@ export async function restartServer(

updateStatus(undefined, LanguageStatusSeverity.Information, true);

const projectRoot = await getProjectRoot();
const workspaceSettings = await getWorkspaceSettings(serverId, projectRoot);

const extensionSettings = await getExtensionSettings(serverId);
const globalSettings = await getGlobalSettings(serverId);

Expand Down
10 changes: 7 additions & 3 deletions src/common/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,12 @@ export async function getWorkspaceSettings(
const config = getConfiguration(namespace, workspace.uri);

let interpreter: string[] = getInterpreterFromSetting(namespace, workspace) ?? [];
if (interpreter.length === 0 && vscode.workspace.isTrusted) {
interpreter = (await getInterpreterDetails(workspace.uri)).path ?? [];
if (interpreter.length === 0) {
if (vscode.workspace.isTrusted) {
interpreter = (await getInterpreterDetails(workspace.uri)).path ?? [];
}
} else {
interpreter = resolveVariables(interpreter, workspace);
}

let configuration = config.get<string>("configuration") ?? null;
Expand All @@ -136,7 +140,7 @@ export async function getWorkspaceSettings(
workspace: workspace.uri.toString(),
path: resolveVariables(config.get<string[]>("path") ?? [], workspace),
ignoreStandardLibrary: config.get<boolean>("ignoreStandardLibrary") ?? true,
interpreter: resolveVariables(interpreter, workspace),
interpreter,
configuration,
importStrategy: config.get<ImportStrategy>("importStrategy") ?? "fromEnvironment",
codeAction: config.get<CodeAction>("codeAction") ?? {},
Expand Down
97 changes: 47 additions & 50 deletions src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { ExecuteCommandRequest, LanguageClient } from "vscode-languageclient/nod
import { registerLogger, traceError, traceLog, traceVerbose } from "./common/log/logging";
import {
checkVersion,
getInterpreterDetails,
initializePython,
onDidChangePythonInterpreter,
resolveInterpreter,
Expand All @@ -12,6 +11,7 @@ import { restartServer } from "./common/server";
import {
checkIfConfigurationChanged,
getInterpreterFromSetting,
getWorkspaceSettings,
ISettings,
} from "./common/settings";
import { loadServerDefaults } from "./common/setup";
Expand All @@ -23,6 +23,7 @@ import {
onDidGrantWorkspaceTrust,
registerCommand,
} from "./common/vscodeapi";
import { getProjectRoot } from "./common/utilities";

const issueTracker = "https://github.com/astral-sh/ruff/issues";

Expand Down Expand Up @@ -87,63 +88,59 @@ export async function activate(context: vscode.ExtensionContext): Promise<void>

restartInProgress = true;

if (!vscode.workspace.isTrusted) {
lsClient = await restartServer(serverId, serverName, outputChannel, lsClient);
const projectRoot = await getProjectRoot();
const workspaceSettings = await getWorkspaceSettings(serverId, projectRoot);

restartInProgress = false;
if (restartQueued) {
restartQueued = false;
await runServer();
}

return;
}

const interpreter = getInterpreterFromSetting(serverId);
if (interpreter !== undefined && interpreter.length > 0) {
if (checkVersion(await resolveInterpreter(interpreter))) {
traceVerbose(
`Using interpreter from ${serverInfo.module}.interpreter: ${interpreter.join(" ")}`,
if (vscode.workspace.isTrusted) {
if (workspaceSettings.interpreter.length === 0) {
updateStatus(
vscode.l10n.t("Please select a Python interpreter."),
vscode.LanguageStatusSeverity.Error,
);
traceError(
"Python interpreter missing:\r\n" +
"[Option 1] Select Python interpreter using the ms-python.python.\r\n" +
`[Option 2] Set an interpreter using "${serverId}.interpreter" setting.\r\n` +
"Please use Python 3.7 or greater.",
);
lsClient = await restartServer(serverId, serverName, outputChannel, lsClient);

restartInProgress = false;
if (restartQueued) {
restartQueued = false;
await runServer();
}
return;
}

restartInProgress = false;
return;
}

const interpreterDetails = await getInterpreterDetails();
if (interpreterDetails.path) {
traceVerbose(`Using interpreter from Python extension: ${interpreterDetails.path.join(" ")}`);
lsClient = await restartServer(serverId, serverName, outputChannel, lsClient);

restartInProgress = false;
if (restartQueued) {
restartQueued = false;
await runServer();
traceLog(`Using interpreter: ${workspaceSettings.interpreter.join(" ")}`);
const resolvedEnvironment = await resolveInterpreter(workspaceSettings.interpreter);
if (resolvedEnvironment === undefined) {
updateStatus(
vscode.l10n.t("Python interpreter not found."),
vscode.LanguageStatusSeverity.Error,
);
traceError(
"Unable to find any Python environment for the interpreter path:",
workspaceSettings.interpreter.join(" "),
);
restartInProgress = false;
return;
} else if (!checkVersion(resolvedEnvironment)) {
restartInProgress = false;
return;
}

return;
}

restartInProgress = false;

updateStatus(
vscode.l10n.t("Please select a Python interpreter."),
vscode.LanguageStatusSeverity.Error,
);
traceError(
"Python interpreter missing:\r\n" +
"[Option 1] Select Python interpreter using the ms-python.python.\r\n" +
`[Option 2] Set an interpreter using "${serverId}.interpreter" setting.\r\n` +
"Please use Python 3.7 or greater.",
lsClient = await restartServer(
projectRoot,
workspaceSettings,
serverId,
serverName,
outputChannel,
lsClient,
);

restartInProgress = false;
if (restartQueued) {
restartQueued = false;
await runServer();
}
};

context.subscriptions.push(
Expand Down Expand Up @@ -263,10 +260,10 @@ export async function activate(context: vscode.ExtensionContext): Promise<void>
traceLog(`Python extension loading`);
await initializePython(context.subscriptions);
traceLog(`Python extension loaded`);
return; // The `onDidChangePythonInterpreter` event will trigger the server start.
}
} else {
await runServer();
}
await runServer();
});
}

Expand Down

0 comments on commit c089b5a

Please sign in to comment.