Skip to content
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.

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

Use ruff.interpreter from workspace settings #553

Merged
merged 1 commit into from
Jul 24, 2024
Merged

Conversation

dhruvmanila
Copy link
Member

@dhruvmanila dhruvmanila commented Jul 23, 2024

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

{
  "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:

Screenshot 2024-07-24 at 08 51 15

Using VS Code specific variables

{
  "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.

@dhruvmanila dhruvmanila force-pushed the dhruv/interpreter branch 2 times, most recently from 6b70a82 to 78dd0ae Compare July 24, 2024 03:14
Comment on lines 268 to 267
traceLog(`Python extension loaded`);
return; // The `onDidChangePythonInterpreter` event will trigger the server start.
}
} else {
await runServer();
}
await runServer();
});
Copy link
Member Author

@dhruvmanila dhruvmanila Jul 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a bug that would hang the extension when ruff.interpreter is provided and the workspace is trusted. We would never call the runServer in that case.

@dhruvmanila dhruvmanila force-pushed the dhruv/interpreter branch 2 times, most recently from 7e25f69 to dec965c Compare July 24, 2024 03:19
@dhruvmanila dhruvmanila marked this pull request as ready for review July 24, 2024 03:23
@charliermarsh
Copy link
Member

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.

Do we need to fall back to the interpreter from the Python extension?

@dhruvmanila
Copy link
Member Author

dhruvmanila commented Jul 24, 2024

Do we need to fall back to the interpreter from the Python extension?

We do fall back to that, it's just moved to settings.ts when we get the workspace settings: https://github.com/astral-sh/ruff-vscode/pull/553/files#diff-3a5b47a3d2e56af22ee3eeb11a7ac02b69ac7b6bf52f9b48a870acd00da9f39a (getInterpreterDetails)

Base automatically changed from dhruv/reduce-api to main July 24, 2024 04:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

VS Code setting ruff.interpreter does no longer accept relative paths
2 participants