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

fix(lsp): don't apply unsafe quick fixes on-save #62

Merged
merged 1 commit into from
Aug 25, 2023

Conversation

Conaclos
Copy link
Member

Summary

This PR avoids applying unsafe fixes when quickfixes are triggered on-save.
Unsafe quickfixes are still available in the IDE via direct invocation.

Test Plan

Manually tested.

@Conaclos Conaclos temporarily deployed to Website deployment August 24, 2023 16:22 — with GitHub Actions Inactive
@github-actions github-actions bot added A-Project Area: project A-LSP Area: language server protocol labels Aug 24, 2023
@Conaclos Conaclos force-pushed the conaclos/lsp-no-unsafe-quickfixes-on-save branch from 1f47f73 to 934a52b Compare August 24, 2023 16:30
@Conaclos Conaclos temporarily deployed to Website deployment August 24, 2023 16:30 — with GitHub Actions Inactive
Copy link
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

Any chance we can create a test?

CHANGELOG.md Outdated Show resolved Hide resolved
@ematipico ematipico changed the title refactor(lsp): don't apply unsafe quick fixes on-save fix(lsp): don't apply unsafe quick fixes on-save Aug 24, 2023
@Conaclos Conaclos force-pushed the conaclos/lsp-no-unsafe-quickfixes-on-save branch from 934a52b to 189b1c9 Compare August 24, 2023 20:50
@Conaclos Conaclos temporarily deployed to Website deployment August 24, 2023 20:50 — with GitHub Actions Inactive
@github-actions github-actions bot added the A-Website Area: website label Aug 24, 2023
@Conaclos
Copy link
Member Author

Any chance we can create a test?

I added two tests.

I found the LSP code a bit complex. I wonder if there is room for simplification...

It's certainly not ideal that extracting quickfixes only results in the extraction of safe quickfixes.
However, the only command that pulls quick fixes seems to be the quickfix on-save command.

Quickfixes on tooltips are part of an unfiltered query.

@ematipico ematipico merged commit 4dbd78a into main Aug 25, 2023
17 checks passed
@ematipico ematipico deleted the conaclos/lsp-no-unsafe-quickfixes-on-save branch August 25, 2023 07:02
@Conaclos
Copy link
Member Author

This PR should also fix rome#4748.

By the way, this brings a question: what is the difference between source.fixAll.biome and quickfix.biome?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Changelog Area: changelog A-LSP Area: language server protocol A-Project Area: project A-Website Area: website
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants