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

Better error message for unsupported Ruff version #512

Merged
merged 1 commit into from
Jul 4, 2024

Conversation

dhruvmanila
Copy link
Member

@dhruvmanila dhruvmanila commented Jul 3, 2024

Summary

This PR adds back the message to suggest the user that the current Ruff binary doesn't support the native server.

This was removed from the Python script in #511.

fixes: #476

Test Plan

Install an older Ruff version to test this out using pipx install --force "ruff==0.3.3".

Screenshot 2024-07-03 at 14 50 41

And, we've another log message which displays the Ruff version similar to ruff-lsp:

2024-07-03 16:00:03.089 [info] Using the Ruff binary: /Users/dhruv/work/astral/ruff-lsp/.venv/bin/ruff
2024-07-03 16:00:03.094 [info] Found Ruff 0.4.10 at /Users/dhruv/work/astral/ruff-lsp/.venv/bin/ruff
2024-07-03 16:00:03.095 [info] Server run command: /Users/dhruv/work/astral/ruff-lsp/.venv/bin/ruff server --preview

Comment on lines +65 to +70
/**
* The minimum version of the Ruff executable that supports the native server.
*/
const MINIMUM_RUFF_SERVER_VERSION: VersionInfo = { major: 0, minor: 3, patch: 5 };
Copy link
Member Author

Choose a reason for hiding this comment

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

This comes from #443 although the minimum supported version if actually 0.3.3 but I guess this is the version we moved away from using the bundled version.

Copy link

Choose a reason for hiding this comment

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

As explained in #508 there are also commands/features that unsupported by old version of nativeServer and/or ruff-lsp. should we declare version constant for them? e.g. ruff.printDebugInformation which was added in astral-sh/ruff#11831 for nativeServer

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. I intend to implement the warning part for the settings in #505.

ruff.printDebugInformation which was added in astral-sh/ruff#11831 for nativeServer

Regarding this command, I think we should only register a VS Code command only if it's available in the server capabilities (astral-sh/ruff#11850). This way we don't need to check the Ruff version for at least the server capabilities.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've opened #513 to keep track of it.

@dhruvmanila dhruvmanila marked this pull request as draft July 3, 2024 09:30
@dhruvmanila dhruvmanila marked this pull request as ready for review July 3, 2024 10:31
Base automatically changed from dhruv/reduce-python to main July 4, 2024 03:49
@dhruvmanila dhruvmanila merged commit fb6ec2f into main Jul 4, 2024
6 checks passed
@dhruvmanila dhruvmanila deleted the dhruv/better-error-message branch July 4, 2024 03:53
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.

Better notification when used Ruff not compatible to extension.
4 participants