Problems with executing LSP commands

[drzbida]

Make sure you have done the following

• Updated to the latest version of blink.cmp
• Searched for existing issues and documentation (try <C-k> on https://cmp.saghen.dev)

Bug Description

Related to the recently added #1130

Context:

There are some LSPs that give you a completion item with a command to execute for the completion to work, same story as #1148 (comment) :). Well, that's actually not even the full story. In this exact case mentioned before, the server actually gives you the name of the client command that needs to be executed and is implemented by the ..client. This can be done in neovim by setting vim.lsp.commands[COMMAND_NAME] like in this example. The vscode implementation needs to be replicated in roslyn.nvim for override completions to work.

Problem 1:

Neovim does not check every workspace/executeCommand request for a client implementation, but only those originating from specific parts of the core code. I wrote more about this in a neovim issue, but only the new neovim 0.11 completion was fixed and I think it's up for each completion plugin to check for client implementations before calling executeCommand themselves. I've started working on adding this functionality in blink.cmp, but I got stuck with the following issue

Problem 2: (FIXED & Tested that the resolved command arrives)

The item that arrives in lsp:execute is incomplete. In my case, the item that I have after completionItem/resolve contains the command parameter, while the item I get in lsp:execute does not. This is a problem even without considering implementing the previous issue. Example:

This is my transformed_item which contains the command:

{
  client_id = 1,
  client_name = "roslyn",
  command = {
    arguments = { {
        uri = "redacted"
      }, {
        newText = "rocess(double delta)\r\n    {\r\n        base._Process(delta);\r\n    }",
        range = {
          ["end"] = {
            character = 27,
            line = 30
          },
          start = {
            character = 27,
            line = 30
          }
        }
      }, false, 1001 },
    command = "roslyn.client.completionComplexEdit",
    title = "CompleteComplexEditCommand"
  },
  commitCharacters = { "(" },
  cursor_column = 27,
  data = {
    ResultId = 17,
    TextDocument = {
      uri = "redacted"
    }
  },
  documentation = {
    kind = "markdown",
    value = "```csharp\r\nvoid Node._Process(double delta)\r\n```\r\n  \r\nCalled during the processing step of the main loop\\. Processing happens at every frame and as fast as possible, so the delta time since the previous frame is not constant\\. delta is in seconds\\.  \r\n  \r\nIt is only called if processing is enabled, which is done automatically if this method is overridden, and can be toggled with Node\\.SetProcess\\(bool\\)\\.  \r\n  \r\nCorresponds to the Node\\.NotificationProcess notification in GodotObject\\.\\_Notification\\(int\\)\\.  \r\n  \r\n**Note:** This method is only called if the node is present in the scene tree \\(i\\.e\\. if it's not an orphan\\)\\."
  },
  exact = false,
  filterText = "_Process",
  insertTextFormat = 1,
  insertTextMode = 1,
  kind = 2,
  label = "_Process(double delta)",
  score = 26,
  score_offset = 0,
  sortText = "_Process",
  source_id = "lsp",
  source_name = "LSP",
  textEdit = {
    newText = "_P",
    range = {
      ["end"] = {
        character = 27,
        line = 30
      },
      start = {
        character = 25,
        line = 30
      }
    }
  },
  textEditText = "_P"
}

This is my item which does not contain the command

{
  client_id = 1,
  client_name = "roslyn",
  commitCharacters = { "(" },
  cursor_column = 27,
  data = {
    ResultId = 17,
    TextDocument = {
      uri = "redacted"
    }
  },
  exact = false,
  filterText = "_Process",
  insertTextFormat = 1,
  insertTextMode = 1,
  kind = 2,
  label = "_Process(double delta)",
  score = 26,
  score_offset = 0,
  sortText = "_Process",
  source_id = "lsp",
  source_name = "LSP",
  textEdit = {
    newText = "_P",
    range = {
      ["end"] = {
        character = 27,
        line = 30
      },
      start = {
        character = 25,
        line = 30
      }
    }
  },
  textEditText = "_P"
}

P.S: I've just migrated from a "bloated" nvim-cmp configuration and I can't believe how much faster it feels and how fast it was to migrate everything :). Thank you for your work.

Relevant configuration

neovim version

NVIM v0.11.0-dev-1756+gc091bc3b9a

blink.cmp version

v0.11.0

[Saghen]

I'm really happy it's working well for you :) I've just pushed a fix that should pass the resolved item to the execute

[drzbida]

Thank you for the help :)

I've added the call to custom client commands and skipping brackets for items with a command that needs to be executed (as either the custom command or the LSP command will complete the item), but I still have a problem with the resolved item. It seems that the item is still not complete (misses the command) only for the first item that has a command. This happens for the first accepted item with a command like 4 out of 5 times. After that, the item always contains the command and works. It still works after changing buffers. I've checked that the item is indeed resolved in general and contains the command (the transformed_item has it). Do you have any hunch as to why that might be the case?

edit: I've also increased the resolve timeout ms to a huge value thinking that maybe the server is somehow slow to resolve the first item with a command, but that was not the case

edit2: Actually I've increased it even further (to 10 seconds) and now it seems to work. I think that the problem is on the server side and it's just painfully slow on the first resolve with a command. I'll investigate a bit more just to be sure, sorry for bothering you :)

Showcase example in case it's not clear what I mean:

2025-02-15_16-24-01.mp4

[Saghen]

Rather than skipping the auto brackets if the item contains a command, we should allow sources to opt-out of auto brackets, and add a lsp/hacks/your-lsp.lua which disables it for this LSP. Generally speaking, LSPs add auto brackets as part of resolve or return it in the original get_completions response, so this should be a pretty rare case

Would also be good to add a note to the code about dropping the client implementation check on the command when we switch to supporting only v0.11+

[drzbida]

Would also be good to add a note to the code about dropping the client implementation check on the command when we switch to supporting only v0.11+

It seems we can just use exec_cmd from lsp client instead of manually sending the lsp request. This one does the check. It was available in <0.11 as a "private" method (although it was already used in other parts of core neovim as a public api). Execute could look something like this if it's acceptable, I've seen there are other places in the codebase which check the neovim version.

function lsp:execute(source, item, callback)
  local client = vim.lsp.get_client_by_id(item.client_id)
  local command_data = item.command

  if not (client and command_data) then
    callback()
    return
  end

  if vim.fn.has('nvim-0.11') == 1 then
    client:exec_cmd(command_data, { bufnr = source.bufnr }, function() callback() end)
  else
    -- TODO: remove this once 0.11 is the minimum version
    client:_exec_cmd(command_data, { bufnr = source.bufnr }, function() callback() end)
  end
end

Rather than skipping the auto brackets if the item contains a command, we should allow sources to opt-out of auto brackets, and add a lsp/hacks/your-lsp.lua which disables it for this LSP. Generally speaking, LSPs add auto brackets as part of resolve or return it in the original get_completions response, so this should be a pretty rare case

I'm not sure what you mean, do you mean disabling the brackets for the LSP in general? Because in the case for Roslyn the server doesn't actually complete the brackets. This was needed because when using autobrackets and you want to accept the a completion like:

public override void _Dro // accept _DropData(Vector2 atPosition, Variant data)

The command executes and it becomes

public override void _DropData(Vector2 atPosition, Variant data)
{
    base._DropData(atPosition, data);
}() // extra brackets here

For normal scenarios, the autobrackets added are needed. The LSP specs indeed says additional modifications to the current document should be described with the additionalTextEdits-property, so what happens here is out of spec. I don't honestly know any other lsp which uses commands in completion to see what they do with it, so we can definitely move the hack for the brackets in the roslyn plugin as to not bloat this.

[disrupted]

Is this related? neovim/neovim@f20335a This feature was just merged into Neovim core.
edit: nvm, just saw that the original issue is linked here