blink.cmp icon indicating copy to clipboard operation
blink.cmp copied to clipboard

LSP function completion does not insert placeholder for function arguments.

Open AnhQuanTrl opened this issue 11 months ago • 7 comments

Make sure you have done the following

  • [X] I have updated to the latest version of blink.cmp
  • [X] I have read the README

Bug Description

Screenshot 2024-12-14 at 10 30 34 Currently function argument placeholders are not inserted to the completion even though the language server do return a response that tell client to insert a snippet.

Here is response of the language server (I got this by adding a vim.print(resolved_item) in accept/init.lua:

{
data = {
  entryNames = { "fetchTenant" },
  file = ".........",
  line = 55,
  offset = 17
},
detail = "(method) RecommendationJobBase.fetchTenant(tenantId: string): Promise<Tenant>",
insertText = "fetchTenant(${1:tenantId})$0",
insertTextFormat = 2,
kind = 2,
label = "fetchTenant",
sortText = "11",
textEdit = {
  insert = {
    ["end"] = {
      character = 16,
      line = 54
    },
    start = {
      character = 15,
      line = 54
    }
  },
  newText = "fetchTenant(${1:tenantId})$0",
  replace = {
    ["end"] = {
      character = 16,
      line = 54
    },
    start = {
      character = 15,
      line = 54
    }
  }
}

The LSP did return insertTextFormat = 2 (Snippet) and also the insertText field contains the snippet expression.

Below is the switch~ snippet resolved_item for comparison:

{
cursor_column = 5,
description = "Switch Statement",
detail = "switch (key) {\n\tcase value:\n\t\t\n\t\tbreak;\n\n\tdefault:\n\t\tbreak;\n}",
documentation = {
  kind = "markdown",
  value = "Switch Statement"
},
insertText = "switch (${1:key}) {\n\tcase ${2:value}:\n\t\t$0\n\t\tbreak;\n\n\tdefault:\n\t\tbreak;\n}",
insertTextFormat = 2,
kind = 15,
label = "switch",
score_offset = -3,
source_id = "snippets",
source_name = "Snippets"
}
Screenshot 2024-12-14 at 10 37 29

It seems like there is no difference between when it comes to resolved_item in both cases but the latter shows the placeholder while the former does not.

Relevant configuration

opts = {
      appearance = {
        use_nvim_cmp_as_default = false,
        nerd_font_variant = 'mono',
      },
      completion = {
        accept = {
          auto_brackets = {
            enabled = true,
          },
        },
        menu = {
          draw = {
            treesitter = true,
          },
        },
        documentation = {
          auto_show = true,
          auto_show_delay_ms = 200,
        },
        -- ghost_text = {
        --   enabled = vim.g.ai_cmp,
        -- },
      },

      sources = {
        default = { 'lsp', 'path', 'snippets', 'buffer' },
      },

      keymap = { preset = 'default' },
    },

neovim version

v0.10.1

blink.cmp version: branch, tag, or commit

v0.7.6

AnhQuanTrl avatar Dec 14 '24 03:12 AnhQuanTrl

What's the output of vim.print(item)?

saghen avatar Dec 14 '24 03:12 saghen

I need to insert vim.print(item) here right? Screenshot 2024-12-14 at 11 06 49

Output is below (for function call case)

{
  client_id = 1,
  cursor_column = 16,
  data = {
    cacheId = 8
  },
  insertTextFormat = 2,
  kind = 2,
  label = "fetchTenant",
  score_offset = 0,
  sortText = "11",
  source_id = "lsp",
  source_name = "LSP",
  textEdit = {
    insert = {
      ["end"] = {
        character = 16,
        line = 44
      },
      start = {
        character = 15,
        line = 44
      }
    },
    newText = "fetchTenant",
    replace = {
      ["end"] = {
        character = 16,
        line = 44
      },
      start = {
        character = 15,
        line = 44
      }
    }
  }
}

AnhQuanTrl avatar Dec 14 '24 04:12 AnhQuanTrl

Duplicate of #479 ?

voidstar-null avatar Dec 14 '24 10:12 voidstar-null

I don't think this is a duplicate since in this scenario the resolved_item did contain the snippet placeholder in insertText.

In this case, I am testing with tsserver, which do support function completion with argument expansion unlike lua_ls in that issue.

AnhQuanTrl avatar Dec 14 '24 11:12 AnhQuanTrl

Oh my bad then. Sorry

voidstar-null avatar Dec 14 '24 15:12 voidstar-null

I'm trying to diagnose this issue with my limited knowledge of LSP. https://github.com/typescript-language-server/typescript-language-server/blob/184c60de3308621380469d6632bdff2e10f672fd/docs/configuration.md?plain=1#L187

It seems like the Typescript Language Server expect the client to be able to handle either insertText and textEdits inside the response from completionItem/resolve for function arguments expansion to work properly. It does not return the full snippet of the item in the first textDocument/completion response.

/**
 * Complete functions with their parameter signature.
 *
 * This functionality relies on LSP client resolving the completion using the `completionItem/resolve` call. If the
 * client can't do that before inserting the completion then it's not safe to enable it as it will result in some
 * completions having a snippet type without actually being snippets, which can then cause problems when inserting them.
 *
 * @default false
 */
completions.completeFunctionCalls: boolean;

I'm not sure if this is out-of-spec or not. Looking at the LSP spec there is no restriction to which property in the CompletionItem returned from LSP the client can handle. My theory as to how nvim-cmp can handle this case is that it prioritize the textEdits field from completionItem/resolve over one from textDocument/completion.

Should blink.cmp handle the scenario this way as well? I can create a PR if this idea is good.

AnhQuanTrl avatar Dec 16 '24 04:12 AnhQuanTrl

It's out of spec technically but I still think we should merge the two items together, prioritizing the resolved item, for compatibility. See end of this paragraph about not changing values not marked as resolvable: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_completion

PR is welcome!

saghen avatar Dec 16 '24 04:12 saghen