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.

Sign up for GitHub

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

Jump to bottom

Add showSyntaxErrors extension setting #504

Merged
merged 3 commits into from
Jun 27, 2024
Merged

Add showSyntaxErrors extension setting #504

merged 3 commits into from
Jun 27, 2024

Conversation

dhruvmanila
Copy link
Member

@dhruvmanila dhruvmanila commented Jun 27, 2024 •
edited
Loading

Summary

This PR adds a ruff.showSyntaxErrors config option for astral-sh/ruff#12059.

Test Plan

Refer to the test plan in astral-sh/ruff#12059.

@dhruvmanila dhruvmanila mentioned this pull request Jun 27, 2024
Merged
@dhruvmanila dhruvmanila marked this pull request as ready for review June 27, 2024 03:09
@dhruvmanila dhruvmanila mentioned this pull request Jun 27, 2024
Merged
MichaReiser
MichaReiser approved these changes Jun 27, 2024
View reviewed changes
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.

LGTM.

We should add the new setting to the README and maybe mention that the setting is only supported with Ruff 0.5.

package.json Outdated Show resolved Hide resolved
package.json
@@ -269,6 +269,12 @@
"additionalProperties": false,
"markdownDescription": "Whether to display Quick Fix actions to disable rules via `noqa` suppression comments."
},
"ruff.showSyntaxErrors": {
"default": true,
Copy link
Member

Choose a reason for hiding this comment

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

What was the default before. Did ruff always filter out syntax error diagnostics? I'm mainly asking to understand if this is a "breaking" change in the sense that the default changes

Copy link
Member Author

@dhruvmanila dhruvmanila Jun 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

No, Ruff would always show syntax error diagnostics. I don't think this is a breaking change.

Copy link
Member

Choose a reason for hiding this comment

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

Good :) Unless someone used ignore=E999 but that "breaking" change is already handled by Ruff's changelog

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I assume you mean ruff.ignore server setting. Ruff would emit a warning in that case but it won't be displayed to the user, it'll be in the logs:

2024-06-27 12:34:00.546 [info] warning: Rule `E999` is deprecated and will be removed in a future release. Syntax errors will always be shown regardless of whether this rule is selected or not.

But, this won't work for ignore because it doesn't work in Ruff itself.

dhruvmanila added a commit to astral-sh/ruff that referenced this pull request Jun 27, 2024
@dhruvmanila
Add server config to filter out syntax error diagnostics (#12059)
a824faf 
## Summary

Follow-up from #11901 

This PR adds a new server setting to show / hide syntax errors.

## Test Plan

### VS Code

Using astral-sh/ruff-vscode#504 with the
following config:

```json
{
  "ruff.nativeServer": true,
  "ruff.path": ["/Users/dhruv/work/astral/ruff/target/debug/ruff"],
  "ruff.showSyntaxErrors": true
}
```

First, set `ruff.showSyntaxErrors` to `true`:
<img width="1177" alt="Screenshot 2024-06-27 at 08 34 58"
src="https://github.com/astral-sh/ruff/assets/67177269/5d77547a-a908-4a00-8714-7c00784e8679">

And then set it to `false`:
<img width="1185" alt="Screenshot 2024-06-27 at 08 35 19"
src="https://github.com/astral-sh/ruff/assets/67177269/9720f089-f10c-420b-a2c1-2bbb2245be35">

### Neovim

Using the following Ruff server config:

```lua
require('lspconfig').ruff.setup {
  init_options = {
    settings = {
      showSyntaxErrors = false,
    },
  },
}
```

First, set `showSyntaxErrors` to `true`:
<img width="1279" alt="Screenshot 2024-06-27 at 08 28 03"
src="https://github.com/astral-sh/ruff/assets/67177269/e694e231-91ba-47f8-8e8a-ad2e82b85a45">

And then set it to `false`:
<img width="1284" alt="Screenshot 2024-06-27 at 08 28 20"
src="https://github.com/astral-sh/ruff/assets/67177269/25b86a57-02b1-44f7-9f65-cf5fdde93b0c">
dhruvmanila added a commit to astral-sh/ruff-lsp that referenced this pull request Jun 27, 2024
@dhruvmanila
Add showSyntaxErrors server setting (#454)
3b6166b 
## Summary

This PR adds a new `showSyntaxErrors` server setting for
astral-sh/ruff#12059 in `ruff-lsp`.

## Test Plan

### VS Code

Requires: astral-sh/ruff-vscode#504

Verified that the VS Code extension is using the bundled `ruff-lsp` and
the debug build of `ruff`:
```
2024-06-27 08:47:49.567 [info] Server run command: /Users/dhruv/work/astral/ruff-vscode/.venv/bin/python /Users/dhruv/work/astral/ruff-vscode/bundled/tool/server.py
2024-06-27 08:47:49.820 [info] Using 'path' setting: /Users/dhruv/work/astral/ruff/target/debug/ruff
2024-06-27 08:47:49.827 [info] Inferred version 0.4.10 for: /Users/dhruv/work/astral/ruff/target/debug/ruff
2024-06-27 08:47:49.828 [info] Found ruff 0.4.10 at /Users/dhruv/work/astral/ruff/target/debug/ruff
```

Using the following VS Code config:

```json
{
  "ruff.nativeServer": false,
  "ruff.path": ["/Users/dhruv/work/astral/ruff/target/debug/ruff"],
  "ruff.showSyntaxErrors": false
}
```

First, set `ruff.showSyntaxErrors` to `true`:
<img width="1177" alt="Screenshot 2024-06-27 at 08 34 58"
src="https://github.com/astral-sh/ruff/assets/67177269/5d77547a-a908-4a00-8714-7c00784e8679">

And then set it to `false`:
<img width="1185" alt="Screenshot 2024-06-27 at 08 35 19"
src="https://github.com/astral-sh/ruff/assets/67177269/9720f089-f10c-420b-a2c1-2bbb2245be35">

### Neovim

Using the following Ruff server config:

```lua
require('lspconfig').ruff_lsp.setup {
  cmd = { '/Users/dhruv/work/astral/ruff-lsp/.venv/bin/ruff-lsp' },
  init_options = {
    settings = {
      path = { '/Users/dhruv/work/astral/ruff/target/debug/ruff' },
      showSyntaxErrors = true,
    },
  },
}
```

First, set `showSyntaxErrors` to `true`:
<img width="1279" alt="Screenshot 2024-06-27 at 08 28 03"
src="https://github.com/astral-sh/ruff/assets/67177269/e694e231-91ba-47f8-8e8a-ad2e82b85a45">

And then set it to `false`:
<img width="1284" alt="Screenshot 2024-06-27 at 08 28 20"
src="https://github.com/astral-sh/ruff/assets/67177269/25b86a57-02b1-44f7-9f65-cf5fdde93b0c">
@dhruvmanila dhruvmanila merged commit eafae85 into main Jun 27, 2024
6 checks passed
@dhruvmanila dhruvmanila deleted the dhruv/show-syntax-errors branch June 27, 2024 07:14
MichaReiser pushed a commit to astral-sh/ruff that referenced this pull request Jun 27, 2024
@dhruvmanila @MichaReiser
Add server config to filter out syntax error diagnostics (#12059)
7362c1a 
## Summary

Follow-up from #11901 

This PR adds a new server setting to show / hide syntax errors.

## Test Plan

### VS Code

Using astral-sh/ruff-vscode#504 with the
following config:

```json
{
  "ruff.nativeServer": true,
  "ruff.path": ["/Users/dhruv/work/astral/ruff/target/debug/ruff"],
  "ruff.showSyntaxErrors": true
}
```

First, set `ruff.showSyntaxErrors` to `true`:
<img width="1177" alt="Screenshot 2024-06-27 at 08 34 58"
src="https://github.com/astral-sh/ruff/assets/67177269/5d77547a-a908-4a00-8714-7c00784e8679">

And then set it to `false`:
<img width="1185" alt="Screenshot 2024-06-27 at 08 35 19"
src="https://github.com/astral-sh/ruff/assets/67177269/9720f089-f10c-420b-a2c1-2bbb2245be35">

### Neovim

Using the following Ruff server config:

```lua
require('lspconfig').ruff.setup {
  init_options = {
    settings = {
      showSyntaxErrors = false,
    },
  },
}
```

First, set `showSyntaxErrors` to `true`:
<img width="1279" alt="Screenshot 2024-06-27 at 08 28 03"
src="https://github.com/astral-sh/ruff/assets/67177269/e694e231-91ba-47f8-8e8a-ad2e82b85a45">

And then set it to `false`:
<img width="1284" alt="Screenshot 2024-06-27 at 08 28 20"
src="https://github.com/astral-sh/ruff/assets/67177269/25b86a57-02b1-44f7-9f65-cf5fdde93b0c">
MichaReiser pushed a commit to astral-sh/ruff that referenced this pull request Jun 27, 2024
@dhruvmanila @MichaReiser
Add server config to filter out syntax error diagnostics (#12059)
22cebdf 
## Summary

Follow-up from #11901 

This PR adds a new server setting to show / hide syntax errors.

## Test Plan

### VS Code

Using astral-sh/ruff-vscode#504 with the
following config:

```json
{
  "ruff.nativeServer": true,
  "ruff.path": ["/Users/dhruv/work/astral/ruff/target/debug/ruff"],
  "ruff.showSyntaxErrors": true
}
```

First, set `ruff.showSyntaxErrors` to `true`:
<img width="1177" alt="Screenshot 2024-06-27 at 08 34 58"
src="https://github.com/astral-sh/ruff/assets/67177269/5d77547a-a908-4a00-8714-7c00784e8679">

And then set it to `false`:
<img width="1185" alt="Screenshot 2024-06-27 at 08 35 19"
src="https://github.com/astral-sh/ruff/assets/67177269/9720f089-f10c-420b-a2c1-2bbb2245be35">

### Neovim

Using the following Ruff server config:

```lua
require('lspconfig').ruff.setup {
  init_options = {
    settings = {
      showSyntaxErrors = false,
    },
  },
}
```

First, set `showSyntaxErrors` to `true`:
<img width="1279" alt="Screenshot 2024-06-27 at 08 28 03"
src="https://github.com/astral-sh/ruff/assets/67177269/e694e231-91ba-47f8-8e8a-ad2e82b85a45">

And then set it to `false`:
<img width="1284" alt="Screenshot 2024-06-27 at 08 28 20"
src="https://github.com/astral-sh/ruff/assets/67177269/25b86a57-02b1-44f7-9f65-cf5fdde93b0c">
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MichaReiser MichaReiser MichaReiser approved these changes

@snowsignal snowsignal snowsignal approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dhruvmanila @MichaReiser @snowsignal