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

Completion items are sent back with a lot of duplicates #4919

Open
rchiodo opened this issue Oct 4, 2023 · 4 comments
Open

Completion items are sent back with a lot of duplicates #4919

rchiodo opened this issue Oct 4, 2023 · 4 comments
Assignees
Labels
enhancement New feature or request

Comments

@rchiodo
Copy link
Contributor

rchiodo commented Oct 4, 2023

Since 3.17, LSP has a way to send duplicate data on completions only once instead of in each completion item.

This here:

https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#completionItem

Doing so could make Pylance faster (and make sync server closer to async server)

@github-actions github-actions bot added the needs repro Issue has not been reproduced yet label Oct 4, 2023
@debonte debonte assigned rchiodo and unassigned debonte Oct 11, 2023
@debonte debonte added enhancement New feature or request and removed needs repro Issue has not been reproduced yet labels Oct 11, 2023
rchiodo added a commit to microsoft/vscode-python that referenced this issue Oct 11, 2023
Dirk added this feature here:

microsoft/vscode-languageserver-node@0b7acc1

We want to use this in Pylance in order to speedup completions. For the
degenerate case, this can speedup completion results by 30%.

See microsoft/pyrx#4113 and
microsoft/pylance-release#4919
@rchiodo
Copy link
Contributor Author

rchiodo commented Oct 11, 2023

Okay this is blocked on possibly changing the LSP client code to combine the data differently:
microsoft/vscode-languageserver-node#1338

@rchiodo
Copy link
Contributor Author

rchiodo commented Oct 11, 2023

Changes taking advantage of this are here:
https://github.com/rchiodo/pyrx/tree/rchiodo/completion_dupes

@debonte
Copy link
Contributor

debonte commented Jun 13, 2024

Another possible size optimization would we to eliminate the textEdit field when it's not necessary. I did a quick test where I typed print and looked through the response. There were 735 items that included textEdit and about 50% of those appeared to be unnecessary -- the range was the range of the identifier we were completing and newText matched the label. I checked this quickly by eye, so I might have missed something.

The other half were functions that needed the textEdit to insert the parens (because I had python.analysis.completeFunctionParens enabled) and move the cursor between them.

HeeJae agrees that this could make the response smaller, but is concerned about the perf impact of deciding when textEdit is needed or not.

Here's a typical textEdit. It's not small...

            "textEdit": {
                "range": {
                    "start": {
                        "line": 13,
                        "character": 0
                    },
                    "end": {
                        "line": 13,
                        "character": 1
                    }
                },
                "newText": "_P"
            },

@heejaechang
Copy link
Contributor

heejaechang commented Jun 13, 2024

a lot of range is same since it refer to the same range, if completion has a way to internalize or share range between items, that would save a lot of repeated text (text of LSP message)

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

3 participants