Skip to content

Expose unfixable as an LSP setting#12674

Closed
charliermarsh wants to merge 1 commit intomainfrom
charlie/unfix
Closed

Expose unfixable as an LSP setting#12674
charliermarsh wants to merge 1 commit intomainfrom
charlie/unfix

Conversation

@charliermarsh
Copy link
Copy Markdown
Member

Summary

It's common for users to want to disable F401 fixes in the editor. Otherwise, imports you just added get removed on-save. I think we should add this as a first-class editor setting.

@charliermarsh charliermarsh added the server Related to the LSP server label Aug 5, 2024
@charliermarsh
Copy link
Copy Markdown
Member Author

@dhruvmanila -- I feel like, ideally, the Quick Fix actions would still be available here. What do you think?

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Aug 5, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@dhruvmanila
Copy link
Copy Markdown
Member

I feel like, ideally, the Quick Fix actions would still be available here. What do you think?

Yeah, I think that's correct but we need to make sure that it's only available as a Quick Fix actions and not Source code actions. So, the source.fixAll.ruff action shouldn't fix this. I can take that up as a follow-up if you want.

Comment thread docs/editors/settings.md
@charliermarsh
Copy link
Copy Markdown
Member Author

@dhruvmanila - Do you think that should apply always for --unfixable, or only when it's set through the editor like this?

@dhruvmanila
Copy link
Copy Markdown
Member

@dhruvmanila - Do you think that should apply always for --unfixable, or only when it's set through the editor like this?

By --unfixable, do you mean whether, in the CLI, we should display the fixes but not apply them?

This actually reminds me of eslint.codeActionsOnSave.rules which is provided by the Eslint VS Code extension and is a list of rules that should be fixed on save which is similar to fixable option.

It seems that the ES Lint extension doesn't apply the code actions if it's negated in the above list but it still shows the diagnostics and provides code actions to fix it manually. This manual application also includes the "Fix all" code action. So, it's only the code actions on save where it's excluded.

I think this is an ideal behavior from an editor perspective and the "manual" / "automatic" invocation of code actions can be known by the server as it's encoded in the request payload.

@charliermarsh
Copy link
Copy Markdown
Member Author

Yeah, I think we want something like codeActionsOnSave but for rules. So maybe what I have in this PR isn't correct.

@dhruvmanila
Copy link
Copy Markdown
Member

I guess that's right, we cannot use the existing fixable / unfixable option without affecting the CLI. We'd want an server (and editor) specific setting and perform the filter at the server level instead.

@charliermarsh
Copy link
Copy Markdown
Member Author

Yeah, sounds right to me. So we'd need both a setting in ruff-server and then in the VS Code extension to connect it together. I can just close this then.

@dhruvmanila
Copy link
Copy Markdown
Member

Opened #12709 to keep track of this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

server Related to the LSP server

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants