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

add documentFormattingProvider to server capabilities response #1307

Closed

Conversation

williamboman
Copy link

Hey! Seeing as this LSP does support the textDocument/formatting command - I figured we could add it to the server capabilities response. Currently struggling to get neovim's LSP client to recognize the fact that the ESLint LSP server does in fact support formatting - this ought to fix it.

@dbaeumer
Copy link
Member

dbaeumer commented Aug 6, 2021

The LSP server does register a document formatter provider but in a dynamic way. This ensures that if the formatter gets disabled ESLint formatter will not show up in the list of default formatters. Does NeoVim not support dynamic registration?

@dbaeumer dbaeumer added the info-needed Issue requires more information from poster label Aug 6, 2021
@sassanh
Copy link

sassanh commented Aug 8, 2021

Related: neovim/neovim#13634

@mjlbach
Copy link

mjlbach commented Aug 8, 2021

We have not implemented dynamic registration in neovim. Most of the time users are setting up options declaratively in their init.vim/init.lua, so it hasn't yet made sense to enable dynamic registration for anything.

@sassanh
Copy link

sassanh commented Aug 8, 2021

Thanks! For those who don't know how to enable/disable features in their init.vim/init.lua, I did it by adding something like this to on_attach callback provided in server:setup:

local on_attach = function(client, bufnr)
  if client.name == 'tsserver' then
    -- Disable formatting for typescript
    client.resolved_capabilities.document_formatting = false
    client.resolved_capabilities.document_range_formatting = false
  elseif client.name == 'eslintls' then
    -- Enable formatting for eslintls
    client.resolved_capabilities.document_formatting = true
  end
  ...

It is working as expected, I just learnt this imperative way, I don't know about the declarative approach yet.

@dbaeumer
Copy link
Member

dbaeumer commented Aug 9, 2021

Go, great to hear that this exist. I will close the PR then since the servers recommended way is to have dynamic registration.

@dbaeumer dbaeumer closed this Aug 9, 2021
@williamboman
Copy link
Author

williamboman commented Aug 10, 2021

Thanks everyone for jumping in! Apologies if I'm continuing the discussion in the wrong repo, but @mjlbach would you say the snippets @sassanh provided is the recommended way to "toggle" the registered capabilities of LSP clients (e.g., disable document formatting for client #1, but enable it for client #2)? If yes, do you recommend doing so as @sassanh has done it, in the on_attach callback and reading the client.name property?

@williamboman
Copy link
Author

williamboman commented Aug 10, 2021

Also a question for @dbaeumer: For ESLint LSP - would you say that the source.fixAll{.eslint} code action request is fully synonymous with the textDocument/formatting request, and both can safely be used interchangeably? Or do you see these potentially drifting apart in the future?

@dbaeumer
Copy link
Member

source.fixAll{.eslint} and textDocument/formatting might already today not produce the same result. So they are not interchangeably.

iovis added a commit to iovis/dotfiles that referenced this pull request Feb 1, 2024
Apparently we have to tell neovim that eslint can format
microsoft/vscode-eslint#1307 (comment)
bpinto added a commit to bpinto/dotfiles that referenced this pull request Apr 11, 2024
Enable formatting capabilities to eslint LSP as neovim is currently
incapable of handling dynamic capabilities from LSP server.

Ref: microsoft/vscode-eslint#1307
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
info-needed Issue requires more information from poster
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants