-
-
Notifications
You must be signed in to change notification settings - Fork 504
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): register formatter dynamically if possible #1590
Conversation
✅ Deploy Preview for biomejs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks great! Can you add a changelog line before merging?
I've added the changelog entry under a new |
Cool, you'll have to run |
Thanks for the assist, much appreciated! |
Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>
Summary
This PR changes the way the formatting providers are registered by the LSP to prevent registering a formatter twice.
The problem
Some LSP clients do not support dynamic registration for formatting providers, so in PR #1042 , we explicitly enabled static registration of the formatters. Unfortunately, this caused clients that support dynamic registration to show two formatters (the statically registered one, and the dynamically registered one).
The solution
This PR adds logic to detect whether the clients supports dynamic registration of formatters, and skips the static registration when they do.
Test Plan
Format Document with...
menu.Fixes #1584