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

Add deprecation warning #520

Merged
merged 4 commits into from
Feb 6, 2025
Merged

Add deprecation warning #520

merged 4 commits into from
Feb 6, 2025

Conversation

dhruvmanila
Copy link
Member

@dhruvmanila dhruvmanila commented Jan 31, 2025

Summary

This PR adds a deprecation warning during the initialization request if the ruff executable found is >= 0.3.5.

Refer to #520 (comment) for the discussion.

Preview

VS Code

It'll provide a warning notification along with logging the message:

Screenshot 2025-02-03 at 11 21 29 AM

Neovim

It seems like Neovim has already deprecated ruff_lsp if the user is using nvim-lspconfig (which should be most of the users):

Screenshot 2025-02-03 at 11 31 50 AM

But, regardless, the users should receive the warning if they've configured it to do so:

Screenshot 2025-02-03 at 11 30 54 AM

@dhruvmanila dhruvmanila marked this pull request as ready for review February 3, 2025 06:11
Copy link
Member

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

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

I'm a bit worried by the churn the deprecation creates. Should we provide an opt-out option for now (setting) to silence the warning, then remove the option in a follow up release (0.10.x) and remove support entirely in 0.11.x?

README.md Outdated Show resolved Hide resolved
ruff_lsp/server.py Outdated Show resolved Hide resolved
@dhruvmanila
Copy link
Member Author

I'm a bit worried by the churn the deprecation creates. Should we provide an opt-out option for now (setting) to silence the warning, then remove the option in a follow up release (0.10.x) and remove support entirely in 0.11.x?

Hmm, I see. Does this mean that by default there will be no warning but if the user de-select this option, they'll receive this warning?

@MichaReiser
Copy link
Member

Hmm, I see. Does this mean that by default there will be no warning but if the user de-select this option, they'll receive this warning?

I'd enable it by default, but give users an option to opt-out. But I'm not sure if it is worth it. I'm just trying to think trough approaches on how we could de-risk the deprecation by reducing possible churn. On the other hand, showing the deprecation only on start doesn't seem too bad.

@dhruvmanila
Copy link
Member Author

I'd enable it by default, but give users an option to opt-out. But I'm not sure if it is worth it. I'm just trying to think trough approaches on how we could de-risk the deprecation by reducing possible churn. On the other hand, showing the deprecation only on start doesn't seem too bad.

Yeah, I'm not sure if it's worth it. There's the issue that the option itself would need to go through a deprecation period of the sorts.

@dhruvmanila
Copy link
Member Author

Re #520 (comment), here's how I tested it:

Installed Ruff 0.3.4 via uv tool install [email protected] and got no deprecation warning and the following logs:

Client logs:

025-02-06 15:22:41.619 [info] Using the Ruff binary: /Users/dhruv/.local/bin/ruff
2025-02-06 15:22:41.662 [info] Stable version of the native server requires Ruff 0.5.3, but found 0.3.4 at /Users/dhruv/.local/bin/ruff instead
2025-02-06 15:22:41.662 [info] Resolved 'ruff.nativeServer: auto' to use the legacy (ruff-lsp) server
2025-02-06 15:22:41.665 [info] Server run command: /Users/dhruv/playground/ruff/.venv/bin/python /Users/dhruv/work/astral/ruff-vscode/bundled/tool/server.py
2025-02-06 15:22:41.665 [info] Server: Start requested.

Server logs:

Interpreter executable (/Users/dhruv/playground/ruff/.venv/bin/ruff) not found
Using environment executable: /Users/dhruv/.local/bin/ruff
Inferred version 0.3.4 for: /Users/dhruv/.local/bin/ruff
Found ruff 0.3.4 at /Users/dhruv/.local/bin/ruff
Interpreter executable (/Users/dhruv/playground/ruff/.venv/bin/ruff) not found
Using environment executable: /Users/dhruv/.local/bin/ruff
Found ruff 0.3.4 at /Users/dhruv/.local/bin/ruff
Running Ruff with: /Users/dhruv/.local/bin/ruff ['check', '--force-exclude', '--no-cache', '--no-fix', '--quiet', '--output-format', 'json', '-', '--stdin-filename', '/Users/dhruv/playground/ruff/lsp/play.py']

And, upgraded the Ruff version to make sure I still get the warnings.

@dhruvmanila dhruvmanila merged commit c973b94 into main Feb 6, 2025
20 checks passed
@dhruvmanila dhruvmanila deleted the dhruv/deprecate branch February 6, 2025 14:41
dhruvmanila added a commit to astral-sh/ruff-vscode that referenced this pull request Feb 6, 2025
## Summary

This PR adds the deprecation message to `ruff-lsp` specific settings and
update the README section with a [warning
message](https://github.com/astral-sh/ruff-vscode/blob/dhruv/deprecate/README.md#python-based-language-server-ruff-lsp).

There doesn't seem to be any way to show this to the user as the
settings are now not visible in the settings UI although they're visible
in the JSON viewer:

<img width="1097" alt="Screenshot 2025-02-03 at 12 02 50 PM"
src="https://github.com/user-attachments/assets/b7406bdd-eafe-4a7f-9897-27246d0064b4"
/>

But, it's not visible in the hover window of the settings:

<img width="947" alt="Screenshot 2025-02-03 at 12 04 05 PM"
src="https://github.com/user-attachments/assets/5706e10e-9c47-4830-b9bf-1df14745c2d6"
/>

### Deprecation warning

Additionally, the extension will provide a deprecation warning when Ruff
version if < 0.3.5. For >= 0.3.5, `ruff-lsp` will provide the warning
(astral-sh/ruff-lsp#520).

Preview of this warning:
 
<img width="2184" alt="Screenshot 2025-02-06 at 5 30 27 PM"
src="https://github.com/user-attachments/assets/54edd643-fcff-43a5-8680-5bf4ae6b493c"
/>
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.

2 participants