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

LSP: Do not compute text edits in source actions on resolve call. #6990

Merged
merged 1 commit into from
Jan 26, 2024

Conversation

dbalek
Copy link
Contributor

@dbalek dbalek commented Jan 24, 2024

Modified to follow change in VSCode behavior when codeAction/resolve LSP call is invoked also on mouse hover and not only on source action selection (see microsoft/vscode#175008). The source actions that require user interaction to compute their workspace edits should no more do it on codeAction/resolve.

@dbalek dbalek added LSP [ci] enable Language Server Protocol tests VSCode Extension [ci] enable VSCode Extension tests labels Jan 24, 2024
@dbalek dbalek added this to the NB22 milestone Jan 24, 2024
@dbalek dbalek requested review from sdedic and lahodaj January 24, 2024 11:54
@dbalek dbalek self-assigned this Jan 24, 2024
@dbalek dbalek requested a review from MartinBalin January 24, 2024 11:54
@MartinBalin
Copy link
Contributor

This has to go to NB21 as when using mouse, like those of us who don't remember 3keys shortcut on mac then this always triggers the action when hoovering over menu item. Ugly behaviour now in VSCode.

@dbalek
Copy link
Contributor Author

dbalek commented Jan 26, 2024

Unfortunately, simple rebasing would bring a lot of unwanted commits to delivery. Merging to master.

@dbalek dbalek merged commit 1c19e60 into apache:master Jan 26, 2024
35 checks passed
@dbalek dbalek deleted the dbalek/lsp-source-actions-update branch January 26, 2024 09:24
@neilcsmith-net
Copy link
Member

@dbalek so you're saying this change cannot go into NB21?

@MartinBalin
Copy link
Contributor

Exactly @neilcsmith-net , too difficult to make it to NB21.

@neilcsmith-net
Copy link
Member

Thanks @MartinBalin There are two PRs (this and #6996 ) that you've said must go in NB21 and then have been merged to master without confirmation from you in review. Just checking we're not expecting anything related to either to hit delivery?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LSP [ci] enable Language Server Protocol tests VSCode Extension [ci] enable VSCode Extension tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants