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

LSP function completion does not insert placeholder for function arguments. #553

Closed
2 tasks done
AnhQuanTrl opened this issue Dec 14, 2024 · 7 comments
Closed
2 tasks done
Labels
bug Something isn't working

Comments

@AnhQuanTrl
Copy link

Make sure you have done the following

  • I have updated to the latest version of blink.cmp
  • 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 AnhQuanTrl added the bug Something isn't working label Dec 14, 2024
@Saghen
Copy link
Owner

Saghen commented Dec 14, 2024

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

@AnhQuanTrl
Copy link
Author

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
      }
    }
  }
}

@the-fuckin-nobody
Copy link

Duplicate of #479 ?

@AnhQuanTrl
Copy link
Author

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.

@the-fuckin-nobody
Copy link

Oh my bad then. Sorry

@AnhQuanTrl
Copy link
Author

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.

@Saghen
Copy link
Owner

Saghen commented Dec 16, 2024

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 Saghen closed this as completed in 7a83acf Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants