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

Support client/registerCapability LSP requests for formatting methods #12661

Closed
1 task done
luckydye opened this issue Jun 4, 2024 · 3 comments · Fixed by #14478
Closed
1 task done

Support client/registerCapability LSP requests for formatting methods #12661

luckydye opened this issue Jun 4, 2024 · 3 comments · Fixed by #14478
Labels
enhancement [core label] language server An umbrella label for all language servers setting Feedback for preferences, configuration, etc

Comments

@luckydye
Copy link
Contributor

luckydye commented Jun 4, 2024

Check for existing issues

  • Completed

Describe the feature

Currently client/registerCapability Language Server requests for formatting methods are ignored, which causes certain language-servers to not format files even though they could.

Specifically textDocument/rangeFormatting, textDocument/onTypeFormatting and textDocument/formatting.

Related to biomejs/biome-zed#5.

If applicable, add mockups / screenshots to help present your vision of the feature

No response

@luckydye luckydye added admin read Pending admin review enhancement [core label] triage Maintainer needs to classify the issue labels Jun 4, 2024
@JosephTLyons JosephTLyons added setting Feedback for preferences, configuration, etc language server An umbrella label for all language servers and removed triage Maintainer needs to classify the issue admin read Pending admin review labels Jun 5, 2024
@SomeoneToIgnore
Copy link
Contributor

@luckydye , I have drafted a branch with the change: https://github.com/zed-industries/zed/compare/kb/format-capabilities?expand=1 but failed to format via biome.
In the logs, I see that it starts and sends back diagnostics and actions successfully, but never gets to the formatting one, even after https://github.com/biomejs/biome-zed/tree/06154375a1e1af532bfcef55a4bc12ee6812568a?tab=readme-ov-file#configuration was applied.

Can you help me with testing it?
Also, curious to hear if you have considered ways to make biome the default formatter for Zed's settings.json and other files without moving the whole functionality into Zed itself.

@luckydye
Copy link
Contributor Author

@SomeoneToIgnore Awesome! I tried it myself once too, but im too much of a Rust noob still 🥲

So it actually works for me. Try this config:

"language_servers": [ "biome" ],

The only problem now is, that we can't choose the "primary" language server used for the formatting. (#11288)
With a setting for this, it would also then be easily possible to just set biome to format Zed settings files for example, since it does not require a config file to function.

@SomeoneToIgnore
Copy link
Contributor

Hmm, that almost works for me now, thank you.

With a setting for this, it would also then be easily possible to just set biome to format Zed settings files for example

I fear, we also would need to add more logic for plugin retrieval in this case, but one step at a time I guess.

I am able to format standalone (no package.json and node_modules around) json files like tasks.json, but standalone .ts and .js files formatting behaves odd.
E.g. if I open Zed project, open https://github.com/zed-industries/zed/blob/main/crates/prettier/src/prettier_server.js in it and try to format, I either see no textDocument/formatting requests sent at all (they get routed into prettier still for some odd reason) or I see them in other projects but get null in response.
For some cases, I see numerours re-registrations of the formatting capabilities..

All happens with the same

"format_on_save": "on",
"formatter": "language_server",
"language_servers": ["biome"],

config.

I do not have much bandwidth to explore all those things, but seems that a bit more work is needed on that front.
Maybe #11288 is the key to this also.

I'll merge the PR and close this issue as it seems fixed.
Cannot promise any fast steps on #11288 and other things, but generally interested to see biome as a default formatter.

SomeoneToIgnore added a commit that referenced this issue Jul 15, 2024
Closes #12661

Release Notes:

- Added dynamic [un]registration for LSP formatting capabilities
([#12661](#12661))
SomeoneToIgnore added a commit that referenced this issue Jul 15, 2024
Closes #12661

Release Notes:

- Added dynamic [un]registration for LSP formatting capabilities
([#12661](#12661))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement [core label] language server An umbrella label for all language servers setting Feedback for preferences, configuration, etc
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants