Merge 0.7 branch (rebased to master)

[ranjithshegde]

Since the 0.7 branch had no one taking initiative, this is essentially #1838 rebased to master.

I have been running the branch of the PR since it was created, have zero issues. Seeing as 0.7.2 is released and master already requires nvim >=0.7, it seems like the ideal time to revisit and merge this.

All credits go to the excellent work of mjlbach, who I wont tag to respect his privacy.

[justinmk]

I have been running the branch of the PR since it was created, have zero issues.

Thank you! That is valuable to know.

Why again was commands renamed to user_commands ? Doesn't look like there was a technical reason for it, nor is it an aesthetic improvement. It implies a distinction between "non user commands", which doesn't serve any purpose. It's just an interface to allow specifying commands, there is no risk of confusion, so making users type out user_commands instead of a simpler name is just pedantry.

[ranjithshegde]

I have been running the branch of the PR since it was created, have zero issues.

Thank you! That is valuable to know.

Why again was commands renamed to user_commands ?

This is straight from the comments on the main PR

"rename commands -> user commands to match API and free up commands namespace"

On the one hand, the api resemblance is nice. But do you think its too confusing for the users? as these commands are essentially wrappers around unique lsp/commands?

Edit: after reading your edits, do you say it just be changed back to commands?

[justinmk]

"API resemblance" here means "it literally matches nvim_create_user_command ? This is questionable because:

1. nvim_create_user_command itself is probably over-specific and should have been named nvim_create_command (if ever it becomes theoretically possible to define non-user commands, why would we want a separate interface?! and, if it never becomes possible, what's the point of mentioning it over and over? Not even :command itself mentions this, it's literaly named :command.)
2. this is not a strong enough motivation to ask users to perform a migration

[ranjithshegde]

"API resemblance" here means "it literally matches nvim_create_user_command ? This is questionable because:

1. nvim_create_user_command itself is probably over-specific and should have been named nvim_create_command (if ever it becomes theoretically possible to define non-user commands, why would we want a separate interface?! and, if it never becomes possible, what's the point of mentioning it over and over? Not even :command itself mentions this, it's literaly named :command.)
2. this is not a strong enough motivation to ask users to perform a migration

Will change it back to command

[justinmk]

Looking closer, and reviewing https://github.com/neovim/nvim-lspconfig/pull/1838 , I see that a migration will be required anyway, because the command structure changed.

I wonder if instead we should maintain the old interface. And deprecate it. Because it's not really needed, people can just call nvim_create_user_comand from on_init or on_attach.

[ranjithshegde]

Looking closer, and reviewing #1838 , I see that a migration will be required anyway, because the command structure changed.

I wonder if instead we should maintain the old interface. And deprecate it. Because it's not really needed, people can just call nvim_create_user_comand from on_init or on_attach.

Perhaps we remove the interface for user to define the commands, seeing as they could just use nvim_create_user_command where appropriate. But many internally defined commands (ClangdSwitchSourceHeader as an example) rely on this mechanism.

Not sure if any language specific plugins make use of them either.

So perhaps just remove it from user_space?

EDIT: Defining the commands in the setup/config of the language_server has the advantage of being able to access the server specific functions with little code.

An example from the help.txt of this PR

  commands = {
     {
      name = "TexlabBuild"
      command = function()
        buf_build(0)
      end,
      opts = {
        desc = "Build the current buffer",
      }
    },
  },

---The `configs.__newindex` metamethod consumes the config definition and returns
---an object with a `setup()` method, to be invoked by users:

    require'lspconfig'.SERVER_NAME.setup{}

---After you set `configs.SERVER_NAME` you can add arbitrary language-specific
---functions to it if necessary.

---Example
    configs.texlab.buf_build = buf_build

[justinmk]

Perhaps we remove the interface for user to define the commands, seeing as they could just use nvim_create_user_command where appropriate.

I'm in favor of that. We would need to provide a simple example of using nvim_create_user_command in on_attach.

EDIT: Defining the commands in the setup/config of the language_server has the advantage of being able to access the server specific functions with little code.

An example from the help.txt of this PR

I don't see the advantage. The nvim_create_user_command call could just reference require'lspconfig'.texlab.buf_build.

[ranjithshegde]

I don't see the advantage. The nvim_create_user_command call could just reference require'lspconfig'.texlab.buf_build.

Many methods from this repo (chiefly lspconfig/utils like vim.fs) are already in core, along with the addition of au-events like LspAttach. Maybe after the release of nvim 0.8 + an appropriate "settle-in" period, this repo could be stripped down to a growing collection of lsp-configs with strong defaults?

With that in mind, this PR could supply a deprecation warning concerning commands, with some examples of how to set the nvim_create_user_commands in an on_attach function? Removing it now might be too radical for some users just yet!

[ranjithshegde]

I have added a deprecation notice for commands as discussed. There is also an example in lspconfig.txt to set them up in on_attach function.

Still not entirely sure this is ideal, having commands in individual LSP setups seems cleaner than having them in a common on_attach function where the commands are filtered by client.name.

@justinmk Would appreciate your review and thoughts on this matter

[ranjithshegde]

@justinmk Gentle ping Requesting your review

[ranjithshegde]

Not sure who the maintainers are now to request review from. @williamboman?

[justinmk]

When I opened this PR (still 95% his work), other maintainers first suggested it be just called commands instead, which I changed, then it was discussed it was best to deprecate it entirely.

Right now it reflect all of those changes, in a single PR!. What is the suggestion? Keep it as the original PR by mjlbach and then deprecate it in the next nvim release cycle? Or something else?

@ranjithshegde this is my fault, sorry. Let's see if we can make progress here without too much work. Here's what I suggest:

1. soft deprecate (show warning/error but continue to work) the commands API without a breaking change (so don't need to change the code)
2. add the documentation examples for nvim_create_user_command
3. remove the examples for commands
4. update the existing lspconfigs to avoid commands

[ranjithshegde]

Is this good to go then?

[ranjithshegde]

Is there a way to see on which file and line codespell action fails at?

[glepnir]

[ranjithshegde]

Squashed all the changes into single commit!

[justinmk]

Will merge this now. Can someone open a PR that removes stuff for 0.8? It won't be merged until 0.8 is released, but a PR will help us figure out what changes are expected.

[ranjithshegde]

Will merge this now. Can someone open a PR that removes stuff for 0.8? It won't be merged until 0.8 is released, but a PR will help us figure out what changes are expected.

Will start with an issue. Maybe deprecation is requried as the first step. Many functions in util are available upstream. A strategy could be to redirect those calls to upstream with a deprecation warning

[mike-lloyd03]

I almost just opened an issue for this but did a bit more digging first. I see that this commit introduces a breaking change. But I cannot tell from the conversation here what needs to be changed in my config to accommodate this change. Or if it's a problem with another plugin that depends on lspconfig and I should roll back to a previous version of this plug until other plug developers can update. Is there recommended guidance somewhere I should be looking at?

Thanks and sorry for this annoying message. I did my research first but came up short.

[justinmk]

@ranjithshegde I wasn't expecting breaking changes here per https://github.com/neovim/nvim-lspconfig/pull/1984#issuecomment-1221625288 :

soft deprecate (show warning/error but continue to work) the commands API without a breaking change (so don't need to change the code)

What did we break?

[ii14]

@justinmk looks like require("lspconfig").available_servers is now require("lspconfig.util").available_servers.

[ii14]

If this is a problem, it could just be added back:

diff --git a/lua/lspconfig.lua b/lua/lspconfig.lua
index 1178202..7838f53 100644
--- a/lua/lspconfig.lua
+++ b/lua/lspconfig.lua
@@ -4,6 +4,11 @@ local M = {
   util = require 'lspconfig.util',
 }
 
+function M.available_servers()
+  vim.deprecate('lspconfig.available_servers', 'lspconfig.util.available_servers', '0.1.4', 'lspconfig')
+  return M.util.available_servers()
+end
+
 local mt = {}
 function mt:__index(k)
   if configs[k] == nil then

edit: added vim.deprecate

[glepnir]

@justinmk like i said in #2078 I had search in github some configs and plugins use it .but it move to utils.lua