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

Notify user about LSP errors from editor actions #23011

Merged
merged 1 commit into from
Jan 11, 2025

Conversation

mgsloan
Copy link
Contributor

@mgsloan mgsloan commented Jan 11, 2025

Closes #22976

Release Notes:

  • Improved visibility of errors from language servers by reporting them in the UI when the user invokes an LSP action.

Closes #22976

Release Notes:

* Improved visibility of errors from language servers by reporting them in the UI when the user invokes an LSP action.
@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Jan 11, 2025
@zed-industries-bot
Copy link

Warnings
⚠️

This PR is missing release notes.

Please add a "Release Notes" section that describes the change:

Release Notes:

- Added/Fixed/Improved ...

If your change is not user-facing, you can use "N/A" for the entry:

Release Notes:

- N/A

Generated by 🚫 dangerJS against d1d0c9d

@mgsloan mgsloan enabled auto-merge January 11, 2025 21:40
@mgsloan mgsloan added this pull request to the merge queue Jan 11, 2025
Merged via the queue into main with commit 65c38f2 Jan 11, 2025
13 checks passed
@mgsloan mgsloan deleted the notify-user-of-lsp-errors-from-editor-actions branch January 11, 2025 22:09
github-merge-queue bot pushed a commit that referenced this pull request Jan 11, 2025
#23011 adds display of errors
in the UI so it's now more important to contextualize these.

Release Notes:

- N/A
mgsloan added a commit that referenced this pull request Jan 12, 2025
I added these notifies in #23011, but in practive have found them to be overly disruptive. It would definitely be good to do something better than logging here, but having a sticky error notification is worse. I think it is still good to notify on mutation failures, so left those in

In particular with rust-analyzer, "Go to definition" and "Find references" frequently fail with "Content modified" quite a while after sending the request.
github-merge-queue bot pushed a commit that referenced this pull request Jan 12, 2025
…23040)

I added these notifies in #23011, but in practive have found them to be
overly disruptive. It would definitely be good to do something better
than logging here, but having a sticky error notification is worse. I
think it is still good to notify on mutation failures, so left those in

In particular with rust-analyzer, "Go to definition" and "Find
references" frequently fail with "Content modified" quite a while after
sending the request. Since users are probably used to these operations
being finicky it doesn't seem useful to have a prominent display of
errors for them.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Rename Symbol" does not report language server errors
2 participants