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

Extend completion to support language servers which requires an additional "completion item resolve" request #1313

Closed
Gabrielbdd opened this issue Dec 20, 2021 · 4 comments · Fixed by #1315
Labels
A-language-server Area: Language server client C-enhancement Category: Improvements

Comments

@Gabrielbdd
Copy link
Contributor

There are some language servers that does not include a "textEdit" nor a "additionalTextEdits" on the response of the "textDocument/completion" request, and because of that, some features like auto importing an unqualified type does not work.

An example of that is the TypeScript language server.

Reading the LSP documentation, the way of doing that is issuing a "completion item resolve" request with the "completion item" selected by the user.

If that helps, the TypeScript language server replies the "documentText/completion" with (simplified, extracted from the logs):

{
    "jsonrpc": "2.0",
    "id": 4,
    "isIncomplete": false,
    "result": {
        "items": [
            {
                "label": "Z_BLOCK",
                "kind": 21,
                "sortText": "�24",
                "commitCharacters": [
                    ".",
                    ",",
                    "("
                ],
                "data": {
                    "file": "/home/example/index.ts",
                    "line": 4,
                    "offset": 3,
                    "entryNames": [
                        {
                            "name": "Z_BLOCK",
                            "source": "zlib",
                            "data": {
                                "exportName": "Z_BLOCK",
                                "moduleSpecifier": "zlib",
                                "ambientModuleName": "zlib"
                            }
                        }
                    ]
                },
                "detail": "zlib",
                "tags": [
                    1
                ]
            }
        ]
    }
}
@Gabrielbdd Gabrielbdd added the C-enhancement Category: Improvements label Dec 20, 2021
@Gabrielbdd
Copy link
Contributor Author

Gabrielbdd commented Dec 20, 2021

I'm pretty new to many things here (Helix, Rust, LSP 😄) but I'm willing (and eager) to help with that.

I looked at how the current implementation works, and right here it applies the "additional edits" on the document.

I'm not sure if that's fine to make the completionItem/resolve request to the language server there, as this file appears to be concerned only with UI logic.

@matoous
Copy link
Contributor

matoous commented Dec 20, 2021

I think this would actually happen in the code_action command around here ~https://github.com/helix-editor/helix/blob/master/helix-term/src/commands.rs#L3277. You would send the codeAction along with the data field to the server codeAction/resolve route to resolve the actions1 and then apply them as it is done now.

But rather wait for someone experienced.

Also, this overlaps with: #183


Footnotes

  1. https://microsoft.github.io/language-server-protocol/specifications/specification-current/#textDocument_codeAction

@kirawi kirawi added the A-helix-term Area: Helix term improvements label Dec 20, 2021
@Gabrielbdd
Copy link
Contributor Author

Hi @matoous , I'm not sure if I understood your idea. I agree that the "codeAction/resolve" could be done right where you pointed out. But to solve the problem that I mentioned, the right request to do is the "completionItem/resolve", as said in the docs. And it looks like its not possible to do that at code_action. (But sure, this one also need to be done, I'm sure there are languages that depends on it).

About the solution, "archseer" gave me some direction on the Matrix community about how to do that using "block_on", I will try it and see if that works.

@matoous
Copy link
Contributor

matoous commented Dec 20, 2021

🙈 yes, I just mis-read. You are absolutely right.

@kirawi kirawi added A-language-server Area: Language server client and removed A-helix-term Area: Helix term improvements labels Dec 20, 2021
@sudormrfbin sudormrfbin linked a pull request Dec 23, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-language-server Area: Language server client C-enhancement Category: Improvements
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants