Add showSyntaxErrors server setting
#454
Conversation
| @@ -750,6 +756,7 @@ def _get_severity(code: str) -> DiagnosticSeverity: | |||
| "F821", # undefined name `name` | |||
| "E902", # `IOError` | |||
| "E999", # `SyntaxError` | |||
| None, # `SyntaxError` as of Ruff v0.5.0 | |||
There was a problem hiding this comment.
This is actually required after v0.5.0.
There was a problem hiding this comment.
If you say it is required, does it mean that Ruff-LSP crashes or is it because it otherwise shows syntax errors as warnings?
I guess there's not much we can do about it other than keeping to emit E999 in the JSON emitter for syntax errors. It might be worth to do so for a while.
There was a problem hiding this comment.
No, I meant that the code field in JSON would be either:
E999for all Ruff versions before v0.5Nonefor Ruff version v0.5 and later
Both are included in this list.
There was a problem hiding this comment.
If you say it is required, does it mean that Ruff-LSP crashes or is it because it otherwise shows syntax errors as warnings?
Oh, I think I read your question wrong but yes otherwise it'll emit syntax errors as warnings. This is why I'm including None in this list so that it's emitted as error.
There was a problem hiding this comment.
I guess there's not much we can do about it other than keeping to emit
E999in the JSON emitter for syntax errors. It might be worth to do so for a while.
I'm not sure about that. If we do keep E999 in the JSON emitter, it should probably be the same for other emitters as well otherwise users might find it confusing.
The current change will only break if there are any other diagnostics emitted by Ruff which doesn't have a rule code and is suppose to be a warning instead. I think it'll be a quite some time before this happens and by that time I think ruff-lsp will be deprecated.
There was a problem hiding this comment.
Makes sense. Let's ship as is. I expect that most users are using the most recent extension anyway because they auto-update.
dhruvmanila
force-pushed
the
dhruv/show-syntax-errors
branch
from
eb2e6ad to
062f956
Compare
dhruvmanila
force-pushed
the
dhruv/show-syntax-errors
branch
from
062f956 to
c7e51b7
Compare
| @@ -614,7 +614,11 @@ async def _lint_document_impl( | |||
| show_error(f"Ruff: Lint failed ({result.stderr.decode('utf-8')})") | |||
| return [] | |||
|
|
|||
| return _parse_output(result.stdout) if result.stdout else [] | |||
| return ( | |||
| _parse_output(result.stdout, settings.get("showSyntaxErrors", True)) | |||
There was a problem hiding this comment.
We use settings.get so this change is backwards compatible i.e., the new ruff-lsp version can be used with an old VS Code extension version.
Summary
This PR adds a new
showSyntaxErrorsserver setting for astral-sh/ruff#12059 inruff-lsp.Test Plan
VS Code
Requires: astral-sh/ruff-vscode#504
Verified that the VS Code extension is using the bundled
ruff-lspand the debug build ofruff:Using the following VS Code config:
{ "ruff.nativeServer": false, "ruff.path": ["/Users/dhruv/work/astral/ruff/target/debug/ruff"], "ruff.showSyntaxErrors": false }First, set
ruff.showSyntaxErrorstotrue:And then set it to
false:Neovim
Using the following Ruff server config:
First, set
showSyntaxErrorstotrue:And then set it to
false: