fix: remove the config.commands #2092

glepnir · 2022-08-26T09:43:25Z

now we can use vim.api.nvim_create_user_command in on_attach field to create commands just work for this server. so
there is no need this field config.commands .

Example:

  local function organize_imports()
    local params = {
      command = 'pyright.organizeimports',
      arguments = { vim.uri_from_bufnr(0) },
    }
    vim.lsp.buf.execute_command(params)
  end

  local on_attach = function(client, bufnr)
    if client.name == "pyright" then
      vim.api.nvim_create_user_command("PyrightOrganizeImports", organize_imports, {desc = 'Organize Imports'})
    end
  end

  require("lspconfig")['pyright'].setup({
    on_attach = on_attach
  })

Fix #1750

ii14 · 2022-08-26T11:00:55Z

doc/lspconfig.txt


-Warning: Commands is deprecated and will be removed in future releases.
-It is recommended to use `vim.api.nvim_create_user_command()` instead in an `on_attach` function.
+Custom commands only work for servers


Suggested change

Custom commands only work for servers

Custom commands only works for servers

I think that entire section about custom commands is not needed anymore?

I think it can help some people so keep it . like some new vimer this can give them a tip

I doubt new people are going to define custom commands. This is an advanced thing to do, and for that they should refer to neovim LSP documentation.

got it. remove

ii14 · 2022-08-26T11:01:00Z

lua/lspconfig/util.lua

  for command_name, def in pairs(commands) do
-    local opts = M._parse_user_command_options(def)
+    local _, opts = unpack(def)


Suggested change

local _, opts = unpack(def)

local opts = def[2]

ii14 · 2022-08-26T11:40:32Z

lua/lspconfig/util.lua

-  return opts
-end
-
+---@deprecated
 function M.create_module_commands(module_name, commands)


As far as I can tell, this function was only used in configs.lua and was not documented. Can't we just get rid of it?

yup. But I'm not sure if this function is used by anyone else. Because he is derived. so i didn't remove it

Took a quick search on github, and seems like there are only lspconfig forks. I think it's okay to get rid of it entirely

ranjithshegde · 2022-08-26T13:03:26Z

now we can use vim.api.nvim_create_user_command in on_attach field to create commands just work for this server. so there is no need this field config.commands .

Example:

  local function organize_imports()
    local params = {
      command = 'pyright.organizeimports',
      arguments = { vim.uri_from_bufnr(0) },
    }
    vim.lsp.buf.execute_command(params)
  end

  local on_attach = function(client, bufnr)
    if client.name == "pyright" then
      vim.api.nvim_create_user_command("PyrightOrganizeImports", organize_imports, {desc = 'Organize Imports'})
    end
  end

  require("lspconfig")['pyright'].setup({
    on_attach = on_attach
  })

Fix #1750

I am a bit too late for this. But this breaks all the internal commands like texlab-build.
The above example only works if the client provides workspace/executeCommands that dont require params field.
In the case of clangdSwitchSourceheader and all texlab commands, you need to write this manually like this

local texlab_build_status = vim.tbl_add_reverse_lookup {
  Success = 0,
  Error = 1,
  Failure = 2,
  Cancelled = 3,
}

local texlab_forward_status = vim.tbl_add_reverse_lookup {
  Success = 0,
  Error = 1,
  Failure = 2,
  Unconfigured = 3,
}

local function buf_build(bufnr)
  bufnr = util.validate_bufnr(bufnr)
  local texlab_client = util.get_active_client_by_name(bufnr, 'texlab')
  local params = {
    textDocument = { uri = vim.uri_from_bufnr(bufnr) },
  }
  if texlab_client then
    texlab_client.request('textDocument/build', params, function(err, result)
      if err then
        error(tostring(err))
      end
      print('Build ' .. texlab_build_status[result.status])
    end, bufnr)
  else
    print 'method textDocument/build is not supported by any servers active on the current buffer'
  end
end


--- and only then 
vim.api.nvim_create_autocmd(--[[stuff]])

And with configs.command removed, none of the internals defined in lspconfig/server_configurations/[sever].lua work anymore

glepnir · 2022-08-26T13:06:22Z

it should be rewrite I think follow this in doc. notice lspconfig only provide the basic lsp server settting .

Warning: Commands is deprecated and will be removed in future releases.
It is recommended to use `vim.api.nvim_create_user_command()` instead in an `on_attach` function.

ranjithshegde · 2022-08-26T13:08:17Z

it should be rewrite I think follow this in doc. notice lspconfig only provide the basic lsp server settting .
Warning: Commands is deprecated and will be removed in future releases.
It is recommended to use `vim.api.nvim_create_user_command()` instead in an `on_attach` function.

Yes but the plan was to soft deprecate it over future releases. At least thats what was discussed during #1984

I added that messaage in lspconfig.txt and it was only merged 3 days ago. too little warning. Should it not be at least a release cycle?

And as far as I can tell, there are no plugins out there that do this for texlab. This breaks config of anyone who uses texlab.
This breaks it with no alternatives for users to use.
The only people who were aware of this removal were people who participated in PR review

ii14 · 2022-08-26T13:11:10Z

Oh, server configurations were not migrated before this? Sorry, I should've checked that. I think this has to be reverted for now then

glepnir · 2022-08-26T13:12:20Z

future releases so future is what time. now we have api can do this. I don't understand why this field was buggy in the past. So do you think there are any commands available. Because of this field, there are constantly some prs that are deeply bound to the server config. It's weird. And justin has removed part of it. I think should be provide a new api like a function to do that. not use this commands field

ranjithshegde · 2022-08-26T13:14:07Z

future releases so future is what time. now we have api can do this. I don't understand why this field was buggy in the past. So do you think there are any commands available. Because of this field, there are constantly some prs that are deeply bound to the server. It's weird. And jusmtin has removed part of it. I think should be provide a new api like a function to do that. not use this commands field

The plan discussed with justin was that we soft deprecate it, and no longer allow users to define it in their config, but keep the internals that are part of server_configurations/[server].lua

Also 3 days is by no means a sufficient warning to remove something with 0 alternattives

ii14 · 2022-08-26T13:14:40Z

But it's still used in default configurations. I don't think we can just set a default on_attach, because it can get overridden by the user, right? So it's not a trivial change.

ranjithshegde · 2022-08-26T13:15:13Z

But it's still used in default configurations. I don't think we can just set a default on_attach, because it can get overridden by the user, right?

With this PR, the commands as part of default configuration no longer work

ii14 · 2022-08-26T13:15:50Z

Yes, this is exactly what I mean.

glepnir · 2022-08-26T13:18:03Z

maybe the design of this field commands was a mistake from the start. Deep bundling brings new PRs and issues

This reverts commit 99e0dc9.

ranjithshegde · 2022-08-26T13:19:09Z

maybe the design of this field commands was a mistake from the start. Deep bundling brings new PRs and issues

I fully agree, 100%. But as it stands, this is part of so many (at least 8 that I know of) LSPs, and most users are accustomed to having it. It needs a bit of slow deprecation.

glepnir · 2022-08-26T13:20:37Z

need redesign and provide a new api to do .then they can implement what they want in local not in this project.

ranjithshegde · 2022-08-26T13:22:26Z

need redesign and provide a new api to do .then they can implement what they want in local not in this project.

This has to be done. But is not trivial. Some servers like pyright just need workspace/executeCommand and then its simple. Others like texlab require textDocument/myFancyOffspecCmd which need params and ctx handling.
Cant make a generalized function.

For some servers there are dedicated plugin. zk has one, clangd has one, I wrote one a few days ago for ccls. So just pointing to dedicated plugins wont work for many other servers

glepnir · 2022-08-26T13:24:25Z

Some issues need to be looked at carefully. what do they need. Provide a function or something to bring some of the information they need.

This reverts commit 99e0dc9.

glepnir · 2022-08-26T13:26:58Z

revert. I think there is a bug in commands so no one uses it. Then I remove something. Sorry for the inconvenience

ranjithshegde · 2022-08-26T13:40:46Z

revert. I think there is a bug in commands so no one uses it. Then I remove something. Sorry for the inconvenience

Removal is the correct and long term idea. Maybe after 0.14 we remove this from userspace and only keep the existing defaults in server_configurations/

glepnir added 2 commits August 26, 2022 17:40

fix: remove the config.commands

b09ab53

fix: format by stylua and remove comamnds test

c44ffb9

ii14 reviewed Aug 26, 2022

View reviewed changes

fix: remove commands from doc

0d505ff

ii14 reviewed Aug 26, 2022

View reviewed changes

fix: remove unused function

88c7cd0

glepnir merged commit 99e0dc9 into neovim:master Aug 26, 2022

glepnir added a commit that referenced this pull request Aug 26, 2022

Revert "fix: remove the config.commands (#2092)"

1ff0b86

This reverts commit 99e0dc9.

glepnir mentioned this pull request Aug 26, 2022

Revert "fix: remove the config.commands" #2093

Merged

glepnir added a commit that referenced this pull request Aug 26, 2022

Revert "fix: remove the config.commands (#2092)" (#2093)

b89530f

This reverts commit 99e0dc9.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: remove the config.commands #2092

fix: remove the config.commands #2092

glepnir commented Aug 26, 2022 •

edited

Loading

ii14 Aug 26, 2022

glepnir Aug 26, 2022

ii14 Aug 26, 2022

glepnir Aug 26, 2022

ii14 Aug 26, 2022

glepnir Aug 26, 2022

ii14 Aug 26, 2022

glepnir Aug 26, 2022

ii14 Aug 26, 2022

glepnir Aug 26, 2022

ranjithshegde commented Aug 26, 2022 •

edited

Loading

glepnir commented Aug 26, 2022

ranjithshegde commented Aug 26, 2022 •

edited

Loading

ii14 commented Aug 26, 2022

glepnir commented Aug 26, 2022 •

edited

Loading

ranjithshegde commented Aug 26, 2022

ii14 commented Aug 26, 2022 •

edited

Loading

ranjithshegde commented Aug 26, 2022

ii14 commented Aug 26, 2022

glepnir commented Aug 26, 2022

ranjithshegde commented Aug 26, 2022

glepnir commented Aug 26, 2022

ranjithshegde commented Aug 26, 2022 •

edited

Loading

glepnir commented Aug 26, 2022

glepnir commented Aug 26, 2022

ranjithshegde commented Aug 26, 2022

	Custom commands only work for servers
	Custom commands only works for servers

fix: remove the config.commands #2092

fix: remove the config.commands #2092

Conversation

glepnir commented Aug 26, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ranjithshegde commented Aug 26, 2022 • edited Loading

glepnir commented Aug 26, 2022

ranjithshegde commented Aug 26, 2022 • edited Loading

ii14 commented Aug 26, 2022

glepnir commented Aug 26, 2022 • edited Loading

ranjithshegde commented Aug 26, 2022

ii14 commented Aug 26, 2022 • edited Loading

ranjithshegde commented Aug 26, 2022

ii14 commented Aug 26, 2022

glepnir commented Aug 26, 2022

ranjithshegde commented Aug 26, 2022

glepnir commented Aug 26, 2022

ranjithshegde commented Aug 26, 2022 • edited Loading

glepnir commented Aug 26, 2022

glepnir commented Aug 26, 2022

ranjithshegde commented Aug 26, 2022

glepnir commented Aug 26, 2022 •

edited

Loading

ranjithshegde commented Aug 26, 2022 •

edited

Loading

ranjithshegde commented Aug 26, 2022 •

edited

Loading

glepnir commented Aug 26, 2022 •

edited

Loading

ii14 commented Aug 26, 2022 •

edited

Loading

ranjithshegde commented Aug 26, 2022 •

edited

Loading