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

Try all path, consider useBundled import strategy #509

Merged
merged 2 commits into from
Jul 4, 2024
Merged

Conversation

dhruvmanila
Copy link
Member

@dhruvmanila dhruvmanila commented Jul 2, 2024

Summary

This PR does two things:

  1. As ruff.path is an array, we need to check each entry and only proceed with the first valid path (ruff-lsp reference)
  2. Consider useBundled import strategy

fixes: #507

Test Plan

1

One valid path:

{
  "ruff.nativeServer": true,
  "ruff.importStrategy": "useBundled",
  "ruff.path": ["/Users/dhruv/work/astral/ruff/target/debug/ruff"]
}

Logs:

2024-07-02 15:43:01.550 [info] Using 'path' setting: /Users/dhruv/work/astral/ruff/target/debug/ruff
2024-07-02 15:43:01.550 [info] Server run command: /Users/dhruv/work/astral/ruff/target/debug/ruff server --preview
2024-07-02 15:43:01.550 [info] Server: Start requested.

2

Multiple valid paths:

{
  "ruff.nativeServer": true,
  "ruff.importStrategy": "useBundled",
  "ruff.path": [
    "/Users/dhruv/.local/bin/ruff",
    "/Users/dhruv/work/astral/ruff/target/debug/ruff"
  ]
}

Logs:

2024-07-02 15:44:12.698 [info] Using 'path' setting: /Users/dhruv/.local/bin/ruff
2024-07-02 15:44:12.698 [info] Server run command: /Users/dhruv/.local/bin/ruff server --preview
2024-07-02 15:44:12.698 [info] Server: Start requested.

3

First is an invalid path, second is a valid path:

{
  "ruff.nativeServer": true,
  "ruff.importStrategy": "useBundled",
  "ruff.path": [
    "/Users/dhruv/random/ruff",
    "/Users/dhruv/work/astral/ruff/target/debug/ruff"
  ]
}

Logs:

2024-07-02 15:44:47.816 [info] Using 'path' setting: /Users/dhruv/work/astral/ruff/target/debug/ruff
2024-07-02 15:44:47.816 [info] Server run command: /Users/dhruv/work/astral/ruff/target/debug/ruff server --preview
2024-07-02 15:44:47.816 [info] Server: Start requested.

4

All invalid paths, should fallback to using bundled executable:

{
  "ruff.nativeServer": true,
  "ruff.importStrategy": "useBundled",
  "ruff.path": [
    "/Users/dhruv/random/ruff",
    "/Users/dhruv/another/random/ruff"
  ]
}

Logs:

2024-07-02 15:59:44.215 [info] Could not find executable in 'path': /Users/dhruv/random/ruff, /Users/dhruv/another/random/ruff
2024-07-02 15:59:44.215 [info] Using bundled executable: /Users/dhruv/work/astral/ruff-vscode/bundled/libs/bin/ruff
2024-07-02 15:59:44.215 [info] Server run command: /Users/dhruv/work/astral/ruff-vscode/bundled/libs/bin/ruff server --preview
2024-07-02 15:59:44.215 [info] Server: Start requested.

5

No paths given, should fallback to using bundled executable:

{
  "ruff.nativeServer": true,
  "ruff.importStrategy": "useBundled",
  "ruff.path": []
}

Logs:

2024-07-02 15:45:34.349 [info] Using bundled executable: /Users/dhruv/work/astral/ruff-vscode/bundled/libs/bin/ruff
2024-07-02 15:45:34.349 [info] Server run command: /Users/dhruv/work/astral/ruff-vscode/bundled/libs/bin/ruff server --preview
2024-07-02 15:45:34.349 [info] Server: Start requested.

6

No paths, not using bundled executable, let the Python script decide on which executable to use:

{
  "ruff.nativeServer": true,
  "ruff.path": []
}

Logs:

2024-07-02 15:50:13.326 [info] Server run command: /Users/dhruv/work/astral/ruff-vscode/.venv/bin/python /Users/dhruv/work/astral/ruff-vscode/bundled/tool/ruff_server.py server --preview
2024-07-02 15:50:13.326 [info] Server: Start requested.

@dhruvmanila dhruvmanila force-pushed the dhruv/bundled branch 2 times, most recently from 12cd6de to 9a2654b Compare July 2, 2024 10:19
@dhruvmanila dhruvmanila marked this pull request as ready for review July 2, 2024 10:22
src/common/server.ts Outdated Show resolved Hide resolved
src/common/server.ts Outdated Show resolved Hide resolved
src/common/server.ts Outdated Show resolved Hide resolved
@dhruvmanila dhruvmanila merged commit e34ffba into main Jul 4, 2024
6 checks passed
@dhruvmanila dhruvmanila deleted the dhruv/bundled branch July 4, 2024 03:05
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.

Enabling ruff.nativeServer doesn't respect ruff.importStrategy: useBundled
4 participants