-
Notifications
You must be signed in to change notification settings - Fork 53
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
Support auto
enablement of Ruff server
#518
Conversation
6cbb64b
to
a9de921
Compare
a9de921
to
482f517
Compare
I'm a bit confused about what the values of the new settings are. The PR summary describes that the values are |
src/common/server.ts
Outdated
let nativeServerSettings = getUserSetNativeServerSettings(serverId, workspace); | ||
if (nativeServerSettings.length > 0) { | ||
traceInfo(`Native server settings found: ${JSON.stringify(nativeServerSettings)}`); | ||
} | ||
useNativeServer = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should at least show a warning in this csae to let users know that one of their settings was ignored.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we provide a warning for the other case as well where the Ruff version supports the stable server but the user has set legacy server settings?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a warning with a suggestion for this case:
2024-07-08 15:59:18.068 [info] Using the Ruff binary: /Users/dhruv/.local/bin/ruff
2024-07-08 15:59:18.075 [info] Stable version of the native server requires Ruff 0.5.0, but found 0.4.10 at /Users/dhruv/.local/bin/ruff instead
2024-07-08 15:59:21.067 [warning] The following settings are not supported with the legacy server (ruff-lsp): ["ruff.configuration"]
2024-07-08 15:59:21.067 [warning] Please remove these settings or set 'nativeServer' to 'on' to use the native server
2024-07-08 15:59:21.067 [info] Resolved 'ruff.nativeServer: auto' to use the legacy (ruff-lsp) server
af924b0
to
e048d71
Compare
1680e05
to
464ff9b
Compare
Sorry, this was copy-pasted from my notes where I initially started off with |
464ff9b
to
2c45c51
Compare
2c45c51
to
e77124c
Compare
Summary
Superseds #505
This PR updates the
nativeServer
setting with new valueson | off | auto
based on the suggestion at #505 (comment).For backwards compatibility,
on
andoff
are equivalent totrue
andfalse
respectively.Logic
"on"
/true
"off"
/false
ruff-lsp
"auto"
ruff
version is <0.5.2
ruff-lsp
.ruff
version is ≥0.5.2
ruff-lsp
ruff server
Settings reference:
Global
Here, the scope represents the following:
ruff-lsp
enable
fixAll
organizeImports
importStrategy
showNotifications
showSyntaxErrors
configuration
configurationPreference
exclude
lineLength
logLevel
logFile
path
interpreter
ignoreStandardLibrary
codeAction
disableRuleComment.enable
fixViolation.enable
lint
enable
preview
select
extendSelect
ignore
extendIgnore
args
run
format
preview
args
Settings Preview
Preview of how the description of
nativeServer
setting renders:Auto-completion
Hover on "auto"
Hover on "on"
Hover on "off"
Dropdown
Screen.Recording.2024-07-05.at.10.46.33.mov
Test Plan
Native server
on
with a unsupported settingsLogs:
Notification Preview:
Native server
off
with unsupported settingsLogs:
Notification Preview:
Native server "auto" with non-stable version:
Logs:
Native server "auto" with non-stable version and native server settings:
Logs:
Native server "auto" with stable version and using native server settings:
Logs:
Native server "auto" with stable version and using legacy server settings:
Logs:
Native server "auto" with stable version and using both native and legacy server settings:
Logs: