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

All completion items are shown as snippets #992

Closed
yoshi1123 opened this issue Mar 18, 2022 · 12 comments
Closed

All completion items are shown as snippets #992

yoshi1123 opened this issue Mar 18, 2022 · 12 comments
Labels
enhancement New feature or request

Comments

@yoshi1123
Copy link
Contributor

yoshi1123 commented Mar 18, 2022

Describe the bug
The language server seems to send "insertTextFormat": 2 for all completion items, in a response to 'textDocument/completion' requests.

For example,

    "response": {
        "id": 20,
        "jsonrpc": "2.0",
        "result": {
            "items": [
                {
                    "label": "true",
                    "sortText": "0002",
                    "kind": 14,
                    "insertTextFormat": 2
                },

insertTextFormat should be set to '1' because '2' is for snippets (https://microsoft.github.io/language-server-protocol/specifications/specification-3-17/#insertTextFormat). Thus the client might display all completions as snippets.

@yoshi1123 yoshi1123 changed the title insertTextFormat set to 2 for all completion items All completion items are shown as snippets Mar 18, 2022
@sumneko
Copy link
Collaborator

sumneko commented Mar 18, 2022

The real reason is that I am lazy.
In my understanding, the role of this field is to tell whether the client needs to resolve this text (just used to set the cursor location), and the true attribute of this text is independent.
For example, the text returned by the hover request, I can select it by param that if it is plain text or markdown. But I was lazy to set it all for markdown.

@yoshi1123
Copy link
Contributor Author

yoshi1123 commented Mar 19, 2022

@sumneko Are you saying that only some completion items should be marked with insertTextFormat set to 2, but you just set every completion item's insertTextFormat to 2, instead of only the items that are actually snippets, because its easier?

@yoshi1123
Copy link
Contributor Author

These changes seemed to fix it:

--- script/core/completion/completion.lua
+++ script/core/completion/completion.lua
@@ -229,7 +229,7 @@
     if snipType == 'Both' or snipType == 'Replace' then
         local snipData = util.deepCopy(data)
 
-        snipData.kind             = define.CompletionItemKind.Snippet
+        -- snipData.kind             = define.CompletionItemKind.Snippet
         snipData.insertText       = buildFunctionSnip(source, value, oop)
         snipData.insertTextFormat = 2
         snipData.command          = {

and

--- script/provider/provider.lua
+++ script/provider/provider.lua
@@ -560,6 +560,11 @@
         local easy = false
         local items = {}
         for i, res in ipairs(result) do
+            local itf = 1
+            if res.kind == define.CompletionItemKind.Method
+                or res.kind == define.CompletionItemKind.Function
+                or res.kind == define.CompletionItemKind.Snippet
+            then itf = 2 end
             local item = {
                 label            = res.label,
                 kind             = res.kind,
@@ -568,7 +573,7 @@
                 sortText         = res.sortText or ('%04d'):format(i),
                 filterText       = res.filterText,
                 insertText       = res.insertText,
-                insertTextFormat = 2,
+                insertTextFormat = itf,
                 commitCharacters = res.commitCharacters,
                 command          = res.command,
                 textEdit         = res.textEdit and {

@yoshi1123
Copy link
Contributor Author

Or, for provider.lua, this is even better:

--- script/provider/provider.lua
+++ script/provider/provider.lua
@@ -560,6 +560,9 @@
         local easy = false
         local items = {}
         for i, res in ipairs(result) do
+            if res.insertTextFormat == nil then
+                res.insertTextFormat = 1
+            end
             local item = {
                 label            = res.label,
                 kind             = res.kind,
@@ -568,7 +571,7 @@
                 sortText         = res.sortText or ('%04d'):format(i),
                 filterText       = res.filterText,
                 insertText       = res.insertText,
-                insertTextFormat = 2,
+                insertTextFormat = res.insertTextFormat,
                 commitCharacters = res.commitCharacters,
                 command          = res.command,
                 textEdit         = res.textEdit and {

@sumneko
Copy link
Collaborator

sumneko commented Mar 19, 2022

@sumneko Are you saying that only some completion items should be marked with insertTextFormat set to 2, but you just set every completion item's insertTextFormat to 2, instead of only the items that are actually snippets, because its easier?

Yes, because I assume that if the text does not have any $1, the client's behavior will be consistent with plain text.

@sumneko
Copy link
Collaborator

sumneko commented Mar 19, 2022

-        snipData.kind             = define.CompletionItemKind.Snippet
+        -- snipData.kind             = define.CompletionItemKind.Snippet

Why modify this line?

@yoshi1123
Copy link
Contributor Author

yoshi1123 commented Mar 19, 2022

That line, if not commented, will have the completion popup show the item as a snippet, rather than, say, a function. At least with https://github.com/prabirshrestha/asyncomplete.vim. The items have a '~' after them to indicate they are a snippet, and it will show, to the right of the item in the completion popup, "function", "snippet", "method", etc. In other words, with that patch, instead of the functions in asyncomplete showing up as snippets, they will show up as functions, but a '~' will indicate anything that is expandable. I am not sure how this change will effect other lsp clients.

I didn't look into the code too much, so you'll have to verify these changes.

@yoshi1123
Copy link
Contributor Author

yoshi1123 commented Mar 19, 2022

Actually, perhaps that should not be commented.

The problem is that if 'Lua.completion.callSnippet' is set to 'Replace', then it shows as a snippet, which is correct behavior. But there is no way of knowing if it is a function, method, etc, as it will show up as just "snippet".

If setting it to "Both", then it shows the function and the snippet, which works, but seems like a lot of extra items.

I found a solution. With the following, the kind will only be set to define.CompletionItemKind.Snippet if snipType == "Both". That way, if it is set to "Replace", then the client will know its original completion kind (e.g., function, method), but insertTextFormat is set to 2, so the client also knows that it is expandable.
Here it is:

--- script/core/completion/completion.lua
+++ script/core/completion/completion.lua
@@ -229,7 +229,9 @@
     if snipType == 'Both' or snipType == 'Replace' then
         local snipData = util.deepCopy(data)
 
-        snipData.kind             = define.CompletionItemKind.Snippet
+        if snipType == 'Both' then
+            snipData.kind             = define.CompletionItemKind.Snippet
+        end
         snipData.insertText       = buildFunctionSnip(source, value, oop)
         snipData.insertTextFormat = 2
         snipData.command          = {

For example, with asyncomplete,

Completion popup for original script/core/completion/completion.lua:

callSnippet = "Replace" (notice that there is no information showing ipairs(t) is a function):

if .. then~       snippet
ipairs(t)~        snippet

callSnippet = "Both":

if .. then~       snippet
ipairs(t)         function
ipairs(t)~        snippet

Completion popup with the above patch for script/core/completion/completion.lua:

callSnippet = "Replace" (notice the '~', meaning expandable, is shown for 'function'):

if .. then~       snippet
ipairs(t)~        function

callSnippet = "Both":

if .. then~       snippet
ipairs(t)         function
ipairs(t)~        snippet

@yoshi1123
Copy link
Contributor Author

Let me know if you want me to make a pull request.

@sumneko
Copy link
Collaborator

sumneko commented Mar 21, 2022

I found a solution. With the following, the kind will only be set to define.CompletionItemKind.Snippet if snipType == "Both". That way, if it is set to "Replace", then the client will know its original completion kind (e.g., function, method), but insertTextFormat is set to 2, so the client also knows that it is expandable.

This sounds good, you can make a PR about it, thank you!

About InsertTextFormat, I think it should only indicate how the client resolves this text (and how to control the cursor), and cannot change the attribute of this item.
This is the client code I imagined:

...
writeText(getCursorPos(), getItemText())
setCursorPos(getCursorPos() + #getItemText())
if insertTextFormat == 2 then
    local cursorMap = resolveSnipPos()
    if cursorMap then
        setCursorPosByMap(cursorMap)
    end
end
...

@suguruwataru
Copy link

suguruwataru commented Mar 28, 2022

@yoshi1123 Hi, please pardon me for asking something that might be unrelated to the topic of the thread. I'm also using https://github.com/prabirshrestha/asyncomplete.vim. Everything from this LS is working, except for that no completion ever shows up. It seems that you got this combination working (and then saw the wrong completion item kind). Did you do anything special to make completion show up?
Found out that you fixed it in #995. Thanks!

@sumneko sumneko added the enhancement New feature or request label Apr 4, 2022
sumneko added a commit that referenced this issue Apr 4, 2022
using `Snippet` only when `callSnippet` is `Both`
@sumneko sumneko closed this as completed Jun 30, 2022
@andrewbraxton
Copy link
Contributor

The issue in the title was resolved, but the issue brought up in the initial post's body text was not. I have a PR here: #3005

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants