echasnovski / mini.nvim

Library of 40+ independent Lua modules improving overall Neovim (version 0.8 and higher) experience with minimal effort
MIT License
4.74k stars 179 forks source link

mini.completion auto import not working with vtsls or tsserver #822

Closed mikebcbc closed 4 months ago

mikebcbc commented 4 months ago

Contributing guidelines

Module(s)

mini.completion

Description

I'm using vtsls and auto import seems to not work with mini.completion. I did notice https://github.com/echasnovski/mini.nvim/issues/61 but just cant seem to recreate. Im not sure if its something that I am doing wrong, but it seems to be that both vtsls supports auto import using additionalTextEdits and mini.completion should as well.

I have tried with tsserver as well and cannot for the life of me get auto imports working.

Any assistance would be amazing. Thanks so much.

LSP Config Mini Config

Neovim version

nightly

Steps to reproduce

  1. Setup vtsls LSP
  2. Open a TS project
  3. Start typing a func from another library or file
  4. C-y to confirm
  5. Not imported until running the actual code action to import.

Expected behavior

Should add import

Actual behavior

Completes, but does not add import.

echasnovski commented 4 months ago

Thanks for the issue!

Steps to reproduce

  1. Setup vtsls LSP
  2. Open a TS project
  3. Start typing a func from another library or file
  4. C-y to confirm
  5. Not imported until running the actual code action to import.

I am afraid I have to ask for a more detailed steps so that we are on the same page. In particular:

mikebcbc commented 4 months ago

Thanks for the issue!

Steps to reproduce

  1. Setup vtsls LSP
  2. Open a TS project
  3. Start typing a func from another library or file
  4. C-y to confirm
  5. Not imported until running the actual code action to import.

I am afraid I have to ask for a more detailed steps so that we are on the same page. In particular:

  • The exact code you use to setup vtsls language server (I assume it is the one from LSP setup, but still).
  • The exact TS project, file to open, text to type, etc.

for sure! sorry about that!

The linked LSP config is the exact code that i am using to setup vtsls. When i install tsserver, im just using mason. Both of these do not work for auto importing with mini completion but do with nvim cmp. Here are the exact lines containing my vtsls config - very barebones.

As for the typescript project, its happening to all TS projects for me, which is what is leading me to believe i have something misconfigured - whats weird is that nvim cmp works perfectly for me. As an example, I can reproduce on this project: https://github.com/microsoft/TypeScript-Node-Starter

  1. Clone project and install either vtsls or tsserver
  2. Go to src/controllers/contact.ts and start typing getApi to import from adjacent api.ts file
  3. Confirm selection and notice that it does not import it.
  4. Follow same steps with nvim-cmp base install (also in linked repo, commented out in cmp.lua) and auto import works.

Thanks so much for your time and fast response :)

echasnovski commented 4 months ago

Thanks for the feedback!

I'll try to reproduce it in the near future and see where the problem is.

echasnovski commented 4 months ago

So, I've tried to reproduce this and had only partial success. TL;DR:

Here are responses I got from each server:

vtsls ``` -- textDocument/completion { command = { arguments = { { cacheId = 1, index = 930, providerId = 2 } }, command = "_vtsls.completionCacheCommand", title = "" }, commitCharacters = { ".", ",", ";", "(" }, data = { cacheId = 1, index = 930, providerId = 2 }, insertTextFormat = 1, kind = 6, label = "getApi", labelDetails = { description = "./api" }, sortText = "16", textEdit = { newText = "getApi", range = { ["end"] = { character = 3, line = 4 }, start = { character = 0, line = 4 } } } } -- completionItem/resolve { command = { arguments = { { cacheId = 1, index = 930, providerId = 2 } }, command = "_vtsls.completionCacheCommand", title = "" }, commitCharacters = { ".", ",", ";", "(" }, data = { cacheId = 1, index = 930, providerId = 2 }, detail = 'Add import from "./api"\n\nconst getApi: (req: Request, res: Response) => void', documentation = { kind = "markdown", value = "List of API examples.\n\n*@route* — GET /api" }, insertTextFormat = 1, kind = 6, label = "getApi", labelDetails = { description = "./api" }, sortText = "16", textEdit = { newText = "getApi", range = { ["end"] = { character = 1, line = 4 }, start = { character = 0, line = 4 } } } } ```
tsserver ``` -- textDocument/completion { data = { cacheId = 926 }, detail = "./api", kind = 6, label = "getApi", sortText = "￿16" } -- completionItem/resolve { additionalTextEdits = { { newText = 'import { getApi } from "./api";\n', range = { ["end"] = { character = 0, line = 3 }, start = { character = 0, line = 3 } } } }, data = { entryNames = { { data = { exportMapKey = "6 1967 getApi ", exportName = "getApi", fileName = "/home/user/repos/TypeScript-Node-Starter/src/controllers/api.ts", moduleSpecifier = "./api" }, name = "getApi", source = "./api" } }, file = "/home/user/repos/TypeScript-Node-Starter/src/controllers/contact.ts", line = 5, offset = 2 }, detail = "Auto import from './api'\nconst getApi: (req: Request, res: Response) => void", documentation = { kind = "markdown", value = "List of API examples.\n\n*@route* — GET /api" }, kind = 6, label = "getApi", sortText = "￿16" } ```

So in general to make additionalTextEdits to take effect: actually select an item, wait for a bit until info window is shown, press <Space> or <C-y>. This is an expected 'mini.completion' behavior.

Closing as "it seems to wrok as expected".

mikebcbc commented 4 months ago

So, I've tried to reproduce this and had only partial success. TL;DR:

  • vtsls did not work for me. It seems because it does not include additionalTextEdits in its response so there is nowhere 'mini.completion' can get that information from.
  • tsserver includes additionalTextEdits but not in its initial response, but when there is an info window shown (i.e. in "completionItem/resolve" response). This is expected and documented in 'NOTES'.

Here are responses I got from each server:

vtsls tsserver So in general to make additionalTextEdits to take effect: actually select an item, wait for a bit until info window is shown, press <Space> or <C-y>. This is an expected 'mini.completion' behavior.

Closing as "it seems to wrok as expected".

This is really interesting. I was indeed not waiting for the window when using tsserver - it works as expected with that note. What is weird is that vtsls works with nvim-cmp so i wonder if something other than addtionalTextEdits is being used there.

Either way, I appreciate your time! Ill look more into this in the context of vtsls and see whats up. Thanks so much again!

mikebcbc commented 4 months ago

linking this here for anyone who stumbles on this in the future: vtsls requires the client to execute a command from the client for additionaltextedits, which is why nvim-cmp works but mini.completion does not.

https://github.com/yioneko/vtsls/issues/156#issuecomment-2067826553

@echasnovski sorry to resurrect this, but curious if it is at all feasible to have mini.completion correctly handle the command field as described in the above issue?

echasnovski commented 4 months ago

linking this here for anyone who stumbles on this in the future: vtsls requires the client to execute a command from the client for additionaltextedits, which is why nvim-cmp works but mini.completion does not.

yioneko/vtsls#156 (comment)

Thanks for clarifying.

@echasnovski sorry to resurrect this, but curious if it is at all feasible to have mini.completion correctly handle the command field as described in the above issue?

Here are my thoughts:

So TL;DR: 'mini.completion' follows only official LSP specification; if it contains a properly documented way of making vtsls work in its current form, I'll consider adding it (if it is not overly complicated); otherwise nothing will be done here.

yioneko commented 4 months ago

@echasnovski There is no off-spec implementation involved here. The client just needs to send a workspace/execuateCommand with the provided command in completion item, and the server will take charge of sending back workspace/applyEdits request to client, which is already handled by nvim builtin client.

echasnovski commented 4 months ago

@echasnovski There is no off-spec implementation involved here. The client just needs to send a workspace/execuateCommand with the provided command in completion item, and the server will take charge of sending back workspace/applyEdits request to client, which is already handled by nvim builtin client.

For me, there are both in-spec and off-spec implementations:

yioneko commented 4 months ago

Yes, the server should prefer using additionalTextEdits as the spec suggested, I choose to use command just because of maintainability while I'm free to change that if necessary.

And from the client's perspective, there is no off-spec implemention required: execuate the carrying command by sending it back to server according to spec, and I see no harm here.

mikebcbc commented 4 months ago

Chiming in here as an outlier, it seems that as @echasnovski noted, this could be considered both in spec and out of spec but stands as two separate issues.

  1. It feels like mini.completion should be able to handle command field sent from the server. Even though it may not be the correct way to trigger additionalTextEdits, what's "in-spec" here seems to be the overall handling of that field.

  2. It also feels like vtsls should not be relying on "command" to trigger "additionalTextEdits". From what I'm reading, the server should prefer otherwise.

Not sure if you both agree but this seems like it could be two separate issues for each repo. Curious on thoughts.

Thanks so much for the time and having the convo here.

echasnovski commented 4 months ago

Chiming in here as an outlier, it seems that as @echasnovski noted, this could be considered both in spec and out of spec but stands as two separate issues.

Yes, that was what I meant. Did not yet have time to add support for command.

mikebcbc commented 4 months ago

Chiming in here as an outlier, it seems that as @echasnovski noted, this could be considered both in spec and out of spec but stands as two separate issues.

Yes, that was what I meant. Did not yet have time to add support for command.

Okay awesome :) should this issue be reopened?

echasnovski commented 3 months ago

Yes, that was what I meant. Did not yet have time to add support for command.

Sorry for taking so long to get back to this. As vtsls seems to already add additionalTextEdits field, I would like to wait for another use case of where executing command is the only way to make something (reasonable) work. This is mostly because 'mini.completion' has one of the most complex designs among all 'mini.nvim' modules (significantly due to LSP spec itself), so I'd like to touch it as rarely as possible.

So, that said, if anyone from the future has a good use case for support command and ends up reading this, feel free to open a separate issue.