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

Multicursor autocomplete with LS does not respect additionalTextEdits option #113551

Closed
nmay231 opened this issue Dec 29, 2020 · 10 comments
Closed
Assignees
Labels
suggest IntelliSense, Auto Complete under-discussion Issue is under discussion for relevance, priority, approach

Comments

@nmay231
Copy link

nmay231 commented Dec 29, 2020

  • VSCode Version: 1.52.1 and confirmed 1.53.0-insiders
  • OS Version: Ubuntu 20.04

Info

For language extensions that provide completions with the additionalTextEdits option, multicursor mode only applies the edit to the first selection and not subsequent ones. To give an example, the cpp extension allows autocompleting indirect struct attributes using periods. If my_struct is a pointer to a struct, you can type my_struct.at and then tab-complete to get my_struct->attribute. However, using multicursor autocomplete, gives unexpected results:

struct my_struct {
  int attr;
} *a;

a.at<cursor>;
a.at<cursor>;
a.at<cursor>;

Pressing tab will autocomplete to the following.

struct my_struct {
  int attr;
} *a;

a->attr; // This is correct
a.>attr; // Notice ".>" instead of "->"
a.>attr;

Also, see attached screenshot.

vscode_c_extension_autocomplete

Copied/moved from microsoft/vscode-cpptools#6720

@sean-mcmanus
Copy link
Contributor

The "root" issue is that our C/C++ extension receives a single textDocument/completion request via the LSP, and we respond with a CompletionItem that is only valid on one of the cursor locations, but VS Code is replicating the insertText to all cursor locations, i.e. in other cases the variable to the left of the cursor may be a different type and have different valid completion results, so I think correct fix would be to send a completion request for every cursor, although the UI experience might be odd with multiple popups per cursor. The additonalTextEdits we send to replace the '.' with '-' is only valid for the first cursor, since it's an edit of a specific line/character, so it seems unlikely that VS Code would want to try to do something to try to replicate the additionalTextEdits for the other cursor locations like it's doing for the insertText.

@nmay231 nmay231 changed the title Multicursor autocomplete with language extensions does not respect additionalEdit option Multicursor autocomplete with LS does not respect additionalTextEdits option Dec 30, 2020
@sean-mcmanus
Copy link
Contributor

I also tried using a range (and a inserting/replacing range) that is before the dot, but VS Code won't allow the range to go before the invocation position (the ".").

@jrieken
Copy link
Member

jrieken commented Jan 4, 2021

but VS Code won't allow the range to go before the invocation position (the ".").

I am pretty sure it does.

@jrieken
Copy link
Member

jrieken commented Jan 4, 2021

Since additional text edits are "precise", e.g. naming a range and text, we cannot multiply them with each cursor location. However, the main edit of a completion will be applied for each cursor. So, ideally the c++ extension doesn't need additional text edits for this.

@jrieken jrieken added suggest IntelliSense, Auto Complete under-discussion Issue is under discussion for relevance, priority, approach labels Jan 4, 2021
@sean-mcmanus
Copy link
Contributor

@jrieken Here are the details:
We receive:

{
    "jsonrpc": "2.0",
    "id": 68,
    "method": "textDocument/completion",
    "params": {
        "textDocument": {
            "uri": "file:///test.cpp"
        },
        "position": {
            "line": 8,
            "character": 6
        },
        "context": {
            "triggerKind": 1
        }
    }
}

and send back


{
    "jsonrpc": "2.0",
    "id": 68,
    "result": {
        "isIncomplete": false,
        "items": [
            {
                "label": "attr",
                "kind": 5,
                "detail": "int my_struct::attr",
                "documentation": {
                    "value": "**File:** test.cpp",
                    "kind": "markdown"
                },
                "sortText": "1attr_",
                "textEdit": {
                    "range": {
                        "start": {
                            "line": 8,
                            "character": 5
                        },
                        "end": {
                            "line": 8,
                            "character": 6
                        }
                    },
                    "newText": "->attr"
                },
                "data": {
                    "file": "TEST.CPP",
                    "symbolPosition": {
                        "line": 1,
                        "character": 8
                    },
                    "hasOverloads": false
                }
            }
        ]
    }
}

but VS Code shows "no results" -- changing textEdit.start.character from 5 to 6 gives results (but that ends up giving var.->member instead of var->member due to the range issue. So that seems like a VS Code issue, right? Let us know if you have any advice on how we can debug/investigate the issue further.

@sean-mcmanus
Copy link
Contributor

sean-mcmanus commented Jan 4, 2021

@jrieken We're not using InsertReplaceEdit with LSP 3.16, but LSP 3.14.1 (vscode-languageclient 5.2.1.). Let us know if you think upgrading to a new version is expected to fix this issue.

@jrieken
Copy link
Member

jrieken commented Jan 4, 2021

When the range of the text edit select the . of a. than you must have a filter text property that matches that

@sean-mcmanus
Copy link
Contributor

@jrieken Cool, thanks a lot for the help -- I got it to work via changing filterText to "." + <label>. So you can close this issue.

We were never using "filterText" and didn't understand what the docs meant by "A string that should be used when filtering a set of completion items. " (adding an example might help others understand its effect).

@nmay231 Our next release should have a fix.

@nmay231
Copy link
Author

nmay231 commented Jan 4, 2021

Awesome! Thank you both 👍

@nmay231 nmay231 closed this as completed Jan 4, 2021
@jrieken
Copy link
Member

jrieken commented Jan 5, 2021

@sean-mcmanus 👏 very cool. Yeah, the filterText is really under-documented and something that's hard to grasp. I'll make sure our extension samples and/or docs get better

@github-actions github-actions bot locked and limited conversation to collaborators Feb 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
suggest IntelliSense, Auto Complete under-discussion Issue is under discussion for relevance, priority, approach
Projects
None yet
Development

No branches or pull requests

3 participants